languages/dart-sdk
languages/dart-sdk
1
0
Fork 0
mirror of https://github.com/dart-lang/sdk synced 2024-10-06 13:08:01 +00:00
Code Issues Packages Projects Releases Wiki Activity

Revert "[vm/compiler] Check def/use relation for PushArguments in environment."

Browse source 
This reverts commit 88e94994b1 as it breaks flutter:

```
[ +192 ms] executing: $FH/engine/src/out/ios_profile_arm/clang_x64/gen_snapshot --causal_async_stacks
--deterministic --snapshot_kind=app-aot-elf --elf=build/aot/app.so --strip --no-sim-use-hardfp
--no-use-integer-division build/aot/app.dill
[+12798 ms] ===== CRASH =====
[   +1 ms] si_signo=Segmentation fault: 11(11), si_code=1, si_addr=0x28
[        ] version=2.6.0-dev.0.0.flutter-88e94994b1 (Wed Sep 11 12:00:07 2019 +0000) on "macos_simarm"
[        ] thread=775, isolate=isolate(0x7fe116011200)
[        ]   pc 0x000000010afd1970 fp 0x00007ffee4e6cc30

dart::AotCallSpecializer::TryExpandCallThroughGetter(dart::Class const&, dart::InstanceCallInstr*)
[        ]   pc 0x000000010afd0ee0 fp 0x00007ffee4e6ccf0
dart::AotCallSpecializer::VisitInstanceCall(dart::InstanceCallInstr*)
[        ]   pc 0x000000010b047199 fp 0x00007ffee4e6cd30 dart::FlowGraphVisitor::VisitBlocks()
[        ]   pc 0x000000010b0f9e32 fp 0x00007ffee4e6cd50
dart::CompilerPass_ApplyICData::DoBody(dart::CompilerPassState*) const
[        ]   pc 0x000000010b0f8d84 fp 0x00007ffee4e6cde0 dart::CompilerPass::Run(dart::CompilerPassState*) const
[        ]   pc 0x000000010b0f8fc1 fp 0x00007ffee4e6ce00
dart::CompilerPass::RunInliningPipeline(dart::CompilerPass::PipelineMode, dart::CompilerPassState*)
[        ]   pc 0x000000010b0a3280 fp 0x00007ffee4e6d2f0 dart::CallSiteInliner::TryInlining(dart::Function const&,
dart::Array const&, dart::InlinedCallData*, bool)
[        ]   pc 0x000000010b0af278 fp 0x00007ffee4e6d3d0 dart::CallSiteInliner::InlineStaticCalls()
[        ]   pc 0x000000010b0aa115 fp 0x00007ffee4e6d4e0 dart::CallSiteInliner::InlineCalls()
[        ]   pc 0x000000010b0a9d01 fp 0x00007ffee4e6d5d0 dart::FlowGraphInliner::Inline()
[        ]   pc 0x000000010b0f9fdf fp 0x00007ffee4e6d640
dart::CompilerPass_Inlining::DoBody(dart::CompilerPassState*) const
[        ]   pc 0x000000010b0f8d84 fp 0x00007ffee4e6d6d0 dart::CompilerPass::Run(dart::CompilerPassState*) const
[        ]   pc 0x000000010b0f9271 fp 0x00007ffee4e6d700
dart::CompilerPass::RunPipeline(dart::CompilerPass::PipelineMode, dart::CompilerPassState*)
[        ]   pc 0x000000010afde14e fp 0x00007ffee4e6dd00
dart::PrecompileParsedFunctionHelper::Compile(dart::CompilationPipeline*)
[        ]   pc 0x000000010afdf725 fp 0x00007ffee4e6ded0 dart::PrecompileFunctionHelper(dart::Precompiler*,
dart::CompilationPipeline*, dart::Function const&, bool)
[        ]   pc 0x000000010afdb95c fp 0x00007ffee4e6df80 dart::Precompiler::CompileFunction(dart::Precompiler*,
dart::Thread*, dart::Zone*, dart::Function const&)
[        ]   pc 0x000000010afda83d fp 0x00007ffee4e6dfe0 dart::Precompiler::ProcessFunction(dart::Function const&)
[        ]   pc 0x000000010afd7294 fp 0x00007ffee4e6e010 dart::Precompiler::Iterate()
[        ]   pc 0x000000010afd5274 fp 0x00007ffee4e6e1c0 dart::Precompiler::DoCompileAll()
[        ]   pc 0x000000010afd4ddc fp 0x00007ffee4e6e570 dart::Precompiler::CompileAll()
[        ]   pc 0x000000010b200102 fp 0x00007ffee4e6e630 Dart_Precompile
[        ]   pc 0x000000010ad939cc fp 0x00007ffee4e6e7d0 dart::bin::main(int, char**)
[        ]   pc 0x00007fff5ae163d5 fp 0x00007ffee4e6e7e0 start
[        ] -- End of DumpStackTrace
[   +1 ms] Dart snapshot generator failed with exit code -6
```
Change-Id: I609493026eeed1fbe68adc9aa52a5b872d08bca0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/116888
Reviewed-by: Alexander Aprelev <aam@google.com>
Commit-Queue: Alexander Aprelev <aam@google.com>
This commit is contained in:
Alexander Aprelev 2019-09-12 01:03:36 +00:00 committed by commit-bot@chromium.org
parent 596a4ab14d
commit dabfda5770
6 changed files with 19 additions and 225 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

70
runtime/tests/vm/dart/getter_closure_call_generic_test.dart
View file

@ -1,70 +0,0 @@
// Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
//
// This regression test targets AotCallSpecializer::TryExpandCallThroughGetter,
// which previously would create bad references in environments to either
// push arguments that came later than the environment's instruction or, worse,
// references to push arguments whose values had not already been calculated at
// the environment's instruction.
final states = <int>[];
main() {
final x = kTrue ? Sub1() : Sub2();
barbar(kTrue, x);
print('states = $states');
}
@pragma('vm:never-inline')
void barbar(bool kTrue, Base base) {
var a = foo();
var b = foo2();
var c = foo3();
var d = foo4();
try {
// rotate
final t = a;
if (kTrue) a = b;
if (kTrue) b = c;
if (kTrue) c = d;
if (kTrue) d = t;
base.barGetter(
a = addState(c), b = addState(d), c = addState(a), d = addState(b));
} catch (e) {
print('got $e');
print('$a $b $c $d');
}
}
final bool kTrue = int.parse('1') == 1;
@pragma('vm:never-inline')
int foo() => kTrue ? 1 : 2;
@pragma('vm:never-inline')
int foo2() => kTrue ? 2 : 3;
@pragma('vm:never-inline')
int foo3() => kTrue ? 3 : 4;
@pragma('vm:never-inline')
int foo4() => kTrue ? 4 : 5;
@pragma('vm:never-inline')
int addState(int i) {
states.add(i);
return i;
}
@pragma('vm:never-inline')
void doit<A>(A x, A x2, A x3, A x4) => throw 'a';
class Base {
void Function<A>(A, A, A, A) get barGetter => doit;
}
class Sub1 extends Base {}
class Sub2 extends Base {}

51
runtime/tests/vm/dart/getter_closure_call_nongeneric_test.dart
View file

@ -1,51 +0,0 @@
// Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
//
// This regression test targets AotCallSpecializer::TryExpandCallThroughGetter,
// which previously would create bad references in environments to either
// push arguments that came later than the environment's instruction or, worse,
// references to push arguments whose values had not already been calculated at
// the environment's instruction.
import 'getter_closure_call_generic_test.dart' as test;
main() {
final x = test.kTrue ? Sub1() : Sub2();
barbar(test.kTrue, x);
print('states = ${test.states}');
}
@pragma('vm:never-inline')
barbar(bool kTrue, Base base) {
var a = test.foo();
var b = test.foo2();
var c = test.foo3();
var d = test.foo4();
try {
// rotate
final t = a;
if (kTrue) a = b;
if (kTrue) b = c;
if (kTrue) c = d;
if (kTrue) d = t;
if (!kTrue) b = test.foo();
base.barGetter(a = test.addState(1), b = test.addState(2),
c = test.addState(3), d = test.addState(4));
} catch (e) {
print('got $e');
print('$a $b $c $d');
}
}
class Base {
FT get barGetter => test.doit;
}
class Sub1 extends Base {}
class Sub2 extends Base {}
typedef dynamic FT(dynamic a, dynamic b, dynamic c, dynamic d);

107
runtime/vm/compiler/aot/aot_call_specializer.cc
View file

@ -1058,37 +1058,6 @@ void AotCallSpecializer::VisitStaticCall(StaticCallInstr* instr) {
CallSpecializer::VisitStaticCall(instr);
}
// Replaces any PushArguments for [call] appearing in [env] with the definition
// of the PushArgument's value. If [idx] is given, then only PushArguments at
// that index or later are replaced. If [env] is nullptr or [idx] is greater
// than the number of arguments to [call], then this function is a no-op.
static void ReplacePushesInEnvWithValueDefinition(Environment* env,
InstanceCallInstr* call,
intptr_t idx = 0) {
ASSERT(call != nullptr);
if (env == nullptr) return;
if (idx >= call->ArgumentCount()) return;
auto const first_push = call->PushArgumentAt(idx);
Environment::DeepIterator it(env);
// First, look in the iterator for [first_push].
for (; !it.Done(); it.Advance()) {
if (it.CurrentValue()->definition() == first_push) break;
}
if (it.Done()) return;
// If we found [first_push], then any other references to later push
// arguments should come immediately in the environment, so iterate
// and replace as described above. If we hit the end, a non-push argument or
// a push argument unrelated to the original call, then stop.
for (intptr_t i = idx; i < call->ArgumentCount(); i++) {
if (it.Done()) return;
auto const value = it.CurrentValue();
auto const push = value->definition()->AsPushArgument();
if (push != call->PushArgumentAt(i)) break;
value->BindToEnvironment(push->value()->definition());
it.Advance();
}
}
bool AotCallSpecializer::TryExpandCallThroughGetter(const Class& receiver_class,
InstanceCallInstr* call) {
// If it's an accessor call it can't be a call through getter.
@ -1157,76 +1126,26 @@ bool AotCallSpecializer::TryExpandCallThroughGetter(const Class& receiver_class,
/*checked_argument_count=*/1,
thread()->compiler_state().GetNextDeoptId());
// For any instructions with environments between the old receiver push and
// the original call, replace any appearances of the post-receiver
// PushArguments from the call in the environment with the underlying value.
// This will allow us to put the new PushArguments in different places than
// the old ones.
for (auto inst = call->PushArgumentAt(receiver_idx)->next(); inst != call;
inst = inst->next()) {
ASSERT(inst != nullptr);
ReplacePushesInEnvWithValueDefinition(inst->env(), call, receiver_idx + 1);
// Insert all new instructions, except .call() invocation into the
// graph.
for (intptr_t i = 0; i < invoke_get->ArgumentCount(); i++) {
InsertBefore(call, invoke_get->PushArgumentAt(i), NULL, FlowGraph::kEffect);
}
// Insert all the new instructions into the graph.
// First, insert the [invoke_get] instruction, then directly replace the old
// TypeArguments push (if any) as well as the old receiver push with the
// receiver push for [invoke_get].
//
// We must insert [invoke_get] right before the old call because local
// variables may have been redefined during the evaluation of the arguments to
// the old call. If they were, then there may be references to the results of
// those evaluations in the environment.
InsertBefore(call, invoke_get, call->env(), FlowGraph::kValue);
ReplacePushesInEnvWithValueDefinition(invoke_get->env(), call,
receiver_idx + 1);
if (receiver_idx > 0) {
call->PushArgumentAt(0)->ReplaceWith(invoke_call->PushArgumentAt(0),
/*iterator=*/nullptr);
}
call->PushArgumentAt(receiver_idx)
->ReplaceWith(invoke_get->PushArgumentAt(0), /*iterator=*/nullptr);
// For the instructions that will replace the original call, we'll also need
// to alter their environments. For [invoke_get], we just need to change
// pushes that we'll be replacing later, and we do that now after copying
// the original call's environment as part of the InsertBefore.
ReplacePushesInEnvWithValueDefinition(invoke_get->env(), call,
receiver_idx + 1);
// Next, replace the original call with the new .call(...) invocation.
// To update the environment for [invoke_call], just change reference to the
// receiver PushArgument. All other push arguments will be changed in the
// last step.
call->ReplaceWith(invoke_call, current_iterator());
for (Environment::DeepIterator it(invoke_call->env()); !it.Done();
it.Advance()) {
auto const val = it.CurrentValue();
if (auto const push = val->definition()->AsPushArgument()) {
if (push == call->PushArgumentAt(receiver_idx)) {
val->BindToEnvironment(invoke_call->PushArgumentAt(receiver_idx));
break; // The push argument will only appear once in the environment.
}
}
}
// Finally, add all the other push arguments for [invoke_call] between
// [invoke_get] and [invoke_call]. For the non-receiver pushes, redirect uses
// of the old instruction to the new one (notably, in the environment for
// [invoke_call] copied from the original) and remove the old instruction.
// (The only other use for [invoke_call]'s receiver push, if any, is in its
// environment, which we already handled earlier.)
ASSERT(call->ArgumentCount() == invoke_call->ArgumentCount());
InsertBefore(invoke_call, invoke_call->PushArgumentAt(receiver_idx),
/*env=*/nullptr, FlowGraph::kEffect);
for (intptr_t i = receiver_idx + 1; i < call->ArgumentCount(); i++) {
InsertBefore(invoke_call, invoke_call->PushArgumentAt(i), /*env=*/nullptr,
for (intptr_t i = 0; i < invoke_call->ArgumentCount(); i++) {
InsertBefore(call, invoke_call->PushArgumentAt(i), NULL,
FlowGraph::kEffect);
}
// Replace original PushArguments in the graph (mainly env uses).
ASSERT(call->ArgumentCount() == invoke_call->ArgumentCount());
for (intptr_t i = 0; i < call->ArgumentCount(); i++) {
call->PushArgumentAt(i)->ReplaceUsesWith(invoke_call->PushArgumentAt(i));
call->PushArgumentAt(i)->RemoveFromGraph();
}
// Replace original call with .call(...) invocation.
call->ReplaceWith(invoke_call, current_iterator());
// AOT compiler expects all calls to have an ICData.
EnsureICData(Z, flow_graph()->function(), invoke_get);
EnsureICData(Z, flow_graph()->function(), invoke_call);

2
runtime/vm/compiler/backend/flow_graph_checker.cc
View file

@ -307,8 +307,6 @@ void FlowGraphChecker::VisitUseDef(Instruction* instruction,
ASSERT(DefDominatesUse(def, instruction));
ASSERT(IsInUseList(is_env ? def->env_use_list() : def->input_use_list(),
instruction));
} else if (def->IsPushArgument()) {
ASSERT(DefDominatesUse(def, instruction));
}
}

3
runtime/vm/compiler/backend/il.cc
View file

@ -1302,7 +1302,8 @@ void Value::RemoveFromUseList() {
} else if (this == def->env_use_list()) {
def->set_env_use_list(next);
if (next != NULL) next->set_previous_use(NULL);
} else if (Value* prev = previous_use()) {
} else {
Value* prev = previous_use();
prev->set_next_use(next);
if (next != NULL) next->set_previous_use(prev);
}

11
runtime/vm/compiler/backend/il_printer.cc
View file

@ -1147,16 +1147,13 @@ void ParallelMoveInstr::PrintTo(BufferFormatter* f) const {
void Environment::PrintTo(BufferFormatter* f) const {
f->Print(" env={ ");
intptr_t arg_count = 0;
int arg_count = 0;
for (intptr_t i = 0; i < values_.length(); ++i) {
auto const value = values_[i];
if (i > 0) f->Print(", ");
if (auto const push = value->definition()->AsPushArgument()) {
f->Print("a%" Pd " (", arg_count++);
push->value()->PrintTo(f);
f->Print(")");
if (values_[i]->definition()->IsPushArgument()) {
f->Print("a%d", arg_count++);
} else {
value->PrintTo(f);
values_[i]->PrintTo(f);
}
if ((locations_ != NULL) && !locations_[i].IsInvalid()) {
f->Print(" [");