diff --git a/runtime/vm/compiler.cc b/runtime/vm/compiler.cc index 1f2d1c2ac68..bae54397970 100644 --- a/runtime/vm/compiler.cc +++ b/runtime/vm/compiler.cc @@ -271,6 +271,7 @@ static bool CompileParsedFunctionHelper(ParsedFunction* parsed_function, isolate->set_long_jump_base(&bailout_jump); if (setjmp(*bailout_jump.Set()) == 0) { FlowGraph* flow_graph = NULL; + GrowableArray guarded_fields; // TimerScope needs an isolate to be properly terminated in case of a // LongJump. { @@ -293,6 +294,7 @@ static bool CompileParsedFunctionHelper(ParsedFunction* parsed_function, FlowGraphBuilder builder(parsed_function, ic_data_array, NULL, // NULL = not inlining. + &guarded_fields, osr_id); flow_graph = builder.BuildGraph(); } @@ -326,13 +328,12 @@ static bool CompileParsedFunctionHelper(ParsedFunction* parsed_function, // Collect all instance fields that are loaded in the graph and // have non-generic type feedback attached to them that can // potentially affect optimizations. - GrowableArray guarded_fields(10); if (optimized) { TimerScope timer(FLAG_compiler_stats, &CompilerStats::graphoptimizer_timer, isolate); - FlowGraphOptimizer optimizer(flow_graph, &guarded_fields); + FlowGraphOptimizer optimizer(flow_graph); optimizer.ApplyICData(); DEBUG_ASSERT(flow_graph->VerifyUseLists()); @@ -358,7 +359,7 @@ static bool CompileParsedFunctionHelper(ParsedFunction* parsed_function, optimizer.ApplyClassIds(); DEBUG_ASSERT(flow_graph->VerifyUseLists()); - FlowGraphInliner inliner(flow_graph, &guarded_fields); + FlowGraphInliner inliner(flow_graph); inliner.Inline(); // Use lists are maintained and validated by the inliner. DEBUG_ASSERT(flow_graph->VerifyUseLists()); diff --git a/runtime/vm/flow_graph.cc b/runtime/vm/flow_graph.cc index 7a7632737e3..e0a71b0ac21 100644 --- a/runtime/vm/flow_graph.cc +++ b/runtime/vm/flow_graph.cc @@ -24,6 +24,7 @@ FlowGraph::FlowGraph(const FlowGraphBuilder& builder, : parent_(), current_ssa_temp_index_(0), max_block_id_(max_block_id), + builder_(builder), parsed_function_(*builder.parsed_function()), num_copied_params_(builder.num_copied_params()), num_non_copied_params_(builder.num_non_copied_params()), diff --git a/runtime/vm/flow_graph.h b/runtime/vm/flow_graph.h index 7e75d7e8c89..7b4be08bec5 100644 --- a/runtime/vm/flow_graph.h +++ b/runtime/vm/flow_graph.h @@ -48,6 +48,10 @@ class FlowGraph : public ZoneAllocated { GraphEntryInstr* graph_entry, intptr_t max_block_id); + const FlowGraphBuilder& builder() const { + return builder_; + } + // Function properties. const ParsedFunction& parsed_function() const { return parsed_function_; @@ -246,6 +250,7 @@ class FlowGraph : public ZoneAllocated { intptr_t max_block_id_; // Flow graph fields. + const FlowGraphBuilder& builder_; const ParsedFunction& parsed_function_; const intptr_t num_copied_params_; const intptr_t num_non_copied_params_; diff --git a/runtime/vm/flow_graph_builder.cc b/runtime/vm/flow_graph_builder.cc index b152dd92313..42a4ffab5c7 100644 --- a/runtime/vm/flow_graph_builder.cc +++ b/runtime/vm/flow_graph_builder.cc @@ -45,6 +45,7 @@ static const String& PrivateCoreLibName(const String& str) { FlowGraphBuilder::FlowGraphBuilder(ParsedFunction* parsed_function, const Array& ic_data_array, InlineExitCollector* exit_collector, + GrowableArray* guarded_fields, intptr_t osr_id) : parsed_function_(parsed_function), ic_data_array_(ic_data_array), @@ -55,6 +56,7 @@ FlowGraphBuilder::FlowGraphBuilder(ParsedFunction* parsed_function, : 0), num_stack_locals_(parsed_function->num_stack_locals()), exit_collector_(exit_collector), + guarded_fields_(guarded_fields), last_used_block_id_(0), // 0 is used for the graph entry. context_level_(0), try_index_(CatchClauseNode::kInvalidTryIndex), @@ -70,6 +72,20 @@ void FlowGraphBuilder::AddCatchEntry(CatchBlockEntryInstr* entry) { } +void FlowGraphBuilder::AddToGuardedFields(const Field& field) const { + if ((field.guarded_cid() == kDynamicCid) || + (field.guarded_cid() == kIllegalCid)) { + return; + } + for (intptr_t j = 0; j < guarded_fields_->length(); j++) { + if ((*guarded_fields_)[j]->raw() == field.raw()) { + return; + } + } + guarded_fields_->Add(&field); +} + + void InlineExitCollector::PrepareGraphs(FlowGraph* callee_graph) { ASSERT(callee_graph->graph_entry()->SuccessorCount() == 1); ASSERT(callee_graph->max_block_id() > caller_graph_->max_block_id()); @@ -3039,6 +3055,17 @@ void EffectGraphVisitor::VisitLoadInstanceFieldNode( node->field().Offset(), AbstractType::ZoneHandle(node->field().type())); load->set_field(&node->field()); + if (owner()->exit_collector() != NULL) { + // While inlining into an optimized function, the field has + // to be added to the list of guarded fields of the caller. + if (node->field().guarded_cid() != kIllegalCid) { + if (!node->field().is_nullable() || + (node->field().guarded_cid() == kNullCid)) { + load->set_result_cid(node->field().guarded_cid()); + } + owner()->AddToGuardedFields(node->field()); + } + } ReturnDefinition(load); } diff --git a/runtime/vm/flow_graph_builder.h b/runtime/vm/flow_graph_builder.h index e3d01c30739..912e38691ad 100644 --- a/runtime/vm/flow_graph_builder.h +++ b/runtime/vm/flow_graph_builder.h @@ -105,6 +105,7 @@ class FlowGraphBuilder: public ValueObject { FlowGraphBuilder(ParsedFunction* parsed_function, const Array& ic_data_array, InlineExitCollector* exit_collector, + GrowableArray* guarded_fields, intptr_t osr_id); FlowGraph* BuildGraph(); @@ -147,6 +148,12 @@ class FlowGraphBuilder: public ValueObject { bool IsInlining() const { return (exit_collector_ != NULL); } InlineExitCollector* exit_collector() const { return exit_collector_; } + GrowableArray* guarded_fields() const { + return guarded_fields_; + } + + void AddToGuardedFields(const Field& field) const; + intptr_t args_pushed() const { return args_pushed_; } void add_args_pushed(intptr_t n) { args_pushed_ += n; } @@ -169,6 +176,7 @@ class FlowGraphBuilder: public ValueObject { const intptr_t num_non_copied_params_; const intptr_t num_stack_locals_; // Does not include any parameters. InlineExitCollector* const exit_collector_; + GrowableArray* guarded_fields_; intptr_t last_used_block_id_; intptr_t context_level_; diff --git a/runtime/vm/flow_graph_inliner.cc b/runtime/vm/flow_graph_inliner.cc index c62c2444b65..910e26521f2 100644 --- a/runtime/vm/flow_graph_inliner.cc +++ b/runtime/vm/flow_graph_inliner.cc @@ -382,8 +382,7 @@ class PolymorphicInliner : public ValueObject { class CallSiteInliner : public ValueObject { public: - CallSiteInliner(FlowGraph* flow_graph, - GrowableArray* guarded_fields) + explicit CallSiteInliner(FlowGraph* flow_graph) : caller_graph_(flow_graph), inlined_(false), initial_size_(flow_graph->InstructionCount()), @@ -391,8 +390,7 @@ class CallSiteInliner : public ValueObject { inlining_depth_(1), collected_call_sites_(NULL), inlining_call_sites_(NULL), - function_cache_(), - guarded_fields_(guarded_fields) { } + function_cache_() { } FlowGraph* caller_graph() const { return caller_graph_; } @@ -544,9 +542,11 @@ class CallSiteInliner : public ValueObject { // Build the callee graph. InlineExitCollector* exit_collector = new InlineExitCollector(caller_graph_, call); + GrowableArray inlined_guarded_fields; FlowGraphBuilder builder(parsed_function, ic_data_array, exit_collector, + &inlined_guarded_fields, Isolate::kNoDeoptId); builder.SetInitialBlockId(caller_graph_->max_block_id()); FlowGraph* callee_graph; @@ -607,7 +607,7 @@ class CallSiteInliner : public ValueObject { &CompilerStats::graphinliner_opt_timer, isolate); // TODO(zerny): Do more optimization passes on the callee graph. - FlowGraphOptimizer optimizer(callee_graph, guarded_fields_); + FlowGraphOptimizer optimizer(callee_graph); optimizer.ApplyICData(); DEBUG_ASSERT(callee_graph->VerifyUseLists()); } @@ -668,6 +668,13 @@ class CallSiteInliner : public ValueObject { call_data->callee_graph = callee_graph; call_data->parameter_stubs = param_stubs; call_data->exit_collector = exit_collector; + + // When inlined, we add the guarded fields of the callee to the caller's + // list of guarded fields. + for (intptr_t i = 0; i < inlined_guarded_fields.length(); ++i) { + caller_graph_->builder().AddToGuardedFields(*inlined_guarded_fields[i]); + } + TRACE_INLINING(OS::Print(" Success\n")); return true; } else { @@ -998,7 +1005,6 @@ class CallSiteInliner : public ValueObject { CallSites* collected_call_sites_; CallSites* inlining_call_sites_; GrowableArray function_cache_; - GrowableArray* guarded_fields_; DISALLOW_COPY_AND_ASSIGN(CallSiteInliner); }; @@ -1153,8 +1159,7 @@ static Instruction* AppendInstruction(Instruction* first, bool PolymorphicInliner::TryInlineRecognizedMethod(const Function& target) { - FlowGraphOptimizer optimizer(owner_->caller_graph(), - NULL); // No guarded fields needed. + FlowGraphOptimizer optimizer(owner_->caller_graph()); TargetEntryInstr* entry; Definition* last; if (optimizer.TryInlineRecognizedMethod(target, @@ -1475,7 +1480,7 @@ void FlowGraphInliner::Inline() { printer.PrintBlocks(); } - CallSiteInliner inliner(flow_graph_, guarded_fields_); + CallSiteInliner inliner(flow_graph_); inliner.InlineCalls(); if (inliner.inlined()) { diff --git a/runtime/vm/flow_graph_inliner.h b/runtime/vm/flow_graph_inliner.h index 57318e3ecf1..dfee0d5d392 100644 --- a/runtime/vm/flow_graph_inliner.h +++ b/runtime/vm/flow_graph_inliner.h @@ -15,9 +15,7 @@ template class GrowableArray; class FlowGraphInliner : ValueObject { public: - FlowGraphInliner(FlowGraph* flow_graph, - GrowableArray* guarded_fields) - : flow_graph_(flow_graph), guarded_fields_(guarded_fields) {} + explicit FlowGraphInliner(FlowGraph* flow_graph) : flow_graph_(flow_graph) { } // The flow graph is destructively updated upon inlining. void Inline(); @@ -26,7 +24,6 @@ class FlowGraphInliner : ValueObject { private: FlowGraph* flow_graph_; - GrowableArray* guarded_fields_; DISALLOW_COPY_AND_ASSIGN(FlowGraphInliner); }; diff --git a/runtime/vm/flow_graph_optimizer.cc b/runtime/vm/flow_graph_optimizer.cc index 329fe19a382..44aad69720e 100644 --- a/runtime/vm/flow_graph_optimizer.cc +++ b/runtime/vm/flow_graph_optimizer.cc @@ -1570,20 +1570,6 @@ bool FlowGraphOptimizer::MethodExtractorNeedsClassCheck( } -void FlowGraphOptimizer::AddToGuardedFields(const Field& field) { - if ((field.guarded_cid() == kDynamicCid) || - (field.guarded_cid() == kIllegalCid)) { - return; - } - for (intptr_t j = 0; j < guarded_fields_->length(); j++) { - if ((*guarded_fields_)[j]->raw() == field.raw()) { - return; - } - } - guarded_fields_->Add(&field); -} - - void FlowGraphOptimizer::InlineImplicitInstanceGetter(InstanceCallInstr* call) { ASSERT(call->HasICData()); const ICData& ic_data = *call->ic_data(); @@ -1610,7 +1596,7 @@ void FlowGraphOptimizer::InlineImplicitInstanceGetter(InstanceCallInstr* call) { if (!field.is_nullable() || (field.guarded_cid() == kNullCid)) { load->set_result_cid(field.guarded_cid()); } - AddToGuardedFields(field); + flow_graph_->builder().AddToGuardedFields(field); } // Discard the environment from the original instruction because the load diff --git a/runtime/vm/flow_graph_optimizer.h b/runtime/vm/flow_graph_optimizer.h index 0401d3f2ce6..8c9f6fe7356 100644 --- a/runtime/vm/flow_graph_optimizer.h +++ b/runtime/vm/flow_graph_optimizer.h @@ -16,11 +16,9 @@ class ParsedFunction; class FlowGraphOptimizer : public FlowGraphVisitor { public: - FlowGraphOptimizer(FlowGraph* flow_graph, - GrowableArray* guarded_fields) + explicit FlowGraphOptimizer(FlowGraph* flow_graph) : FlowGraphVisitor(flow_graph->reverse_postorder()), - flow_graph_(flow_graph), - guarded_fields_(guarded_fields) { } + flow_graph_(flow_graph) { } virtual ~FlowGraphOptimizer() {} FlowGraph* flow_graph() const { return flow_graph_; } @@ -196,10 +194,7 @@ class FlowGraphOptimizer : public FlowGraphVisitor { Definition* left_instr, Definition* right_instr); - void AddToGuardedFields(const Field& field); - FlowGraph* flow_graph_; - GrowableArray* guarded_fields_; DISALLOW_COPY_AND_ASSIGN(FlowGraphOptimizer); }; diff --git a/tests/language/vm/optimized_guarded_field_test.dart b/tests/language/vm/optimized_guarded_field_test.dart new file mode 100644 index 00000000000..2ef5dcbfc49 --- /dev/null +++ b/tests/language/vm/optimized_guarded_field_test.dart @@ -0,0 +1,35 @@ +// Copyright (c) 2013, 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. +// Test correct handling of phis with only environment uses that were inserted +// by store to load forwarding. +// VMOptions=--optimization_counter_threshold=10 + +import "package:expect/expect.dart"; + +class A { + var foo; +} + +class B { + get foo => null; +} + +test(obj) => obj.foo == null ? "null" : "other"; + +main() { + var a = new A(); + var b = new B(); + // Trigger optimization of test with a polymorphic load. + // The guarded type of foo is null. + test(a); + test(b); + for (var i = 0; i < 20; ++i) test(a); + Expect.equals("null", test(a)); + Expect.equals("null", test(b)); + + // Store a non-null object into foo to trigger deoptimization of test. + a.foo = 123; + Expect.equals("other", test(a)); + Expect.equals("null", test(b)); +}