From ab3cbf57964f0a558d2011f1b140fe68744ea01b Mon Sep 17 00:00:00 2001 From: "kmillikin@google.com" Date: Thu, 7 Feb 2013 09:42:36 +0000 Subject: [PATCH] Change CSE, LICM, and range analysis to preserve use lists. Change the common subexpression elimination, loop-invariant code motion, and range analysis passes to maintain use lists. git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart@18209 260f80e4-7a28-3924-810f-c04153c831b5 --- runtime/vm/compiler.cc | 5 +--- runtime/vm/flow_graph.cc | 13 +++++++--- runtime/vm/flow_graph_optimizer.cc | 15 ++++++++--- runtime/vm/intermediate_language.cc | 40 +++++++++++++++++++---------- runtime/vm/intermediate_language.h | 19 +++++++++----- 5 files changed, 61 insertions(+), 31 deletions(-) diff --git a/runtime/vm/compiler.cc b/runtime/vm/compiler.cc index e0324f01023..babed804a61 100644 --- a/runtime/vm/compiler.cc +++ b/runtime/vm/compiler.cc @@ -196,10 +196,8 @@ static bool CompileParsedFunctionHelper(const ParsedFunction& parsed_function, propagator.PropagateTypes(); } - // Verify that the use lists are still valid. - DEBUG_ASSERT(flow_graph->ValidateUseLists()); - // Propagate sminess from CheckSmi to phis. + flow_graph->ComputeUseLists(); optimizer.PropagateSminess(); // Use propagated class-ids to optimize further. @@ -244,7 +242,6 @@ static bool CompileParsedFunctionHelper(const ParsedFunction& parsed_function, // We have to perform range analysis after LICM because it // optimistically moves CheckSmi through phis into loop preheaders // making some phis smi. - flow_graph->ComputeUseLists(); optimizer.InferSmiRanges(); } diff --git a/runtime/vm/flow_graph.cc b/runtime/vm/flow_graph.cc index 58ae9ac02ed..ea66a5a2480 100644 --- a/runtime/vm/flow_graph.cc +++ b/runtime/vm/flow_graph.cc @@ -169,7 +169,11 @@ static void ValidateUseListsInInstruction(Instruction* instr) { while (curr != NULL) { ASSERT(prev == curr->previous_use()); ASSERT(defn == curr->definition()); - ASSERT(curr == curr->instruction()->InputAt(curr->use_index())); + Instruction* instr = curr->instruction(); + // The instruction should not be removed from the graph (phis are not + // removed until register allocation.) + ASSERT(instr->IsPhi() || (instr->previous() != NULL)); + ASSERT(curr == instr->InputAt(curr->use_index())); prev = curr; curr = curr->next_use(); } @@ -179,8 +183,11 @@ static void ValidateUseListsInInstruction(Instruction* instr) { while (curr != NULL) { ASSERT(prev == curr->previous_use()); ASSERT(defn == curr->definition()); - ASSERT(curr == - curr->instruction()->env()->ValueAtUseIndex(curr->use_index())); + Instruction* instr = curr->instruction(); + ASSERT(curr == instr->env()->ValueAtUseIndex(curr->use_index())); + // The instruction should not be removed from the graph (phis are not + // removed until register allocation.) + ASSERT(instr->IsPhi() || (instr->previous() != NULL)); prev = curr; curr = curr->next_use(); } diff --git a/runtime/vm/flow_graph_optimizer.cc b/runtime/vm/flow_graph_optimizer.cc index 87e4f45bcde..b1a10b853c1 100644 --- a/runtime/vm/flow_graph_optimizer.cc +++ b/runtime/vm/flow_graph_optimizer.cc @@ -176,7 +176,7 @@ static void EnsureSSATempIndex(FlowGraph* graph, } -static void ReplaceCurrentInstruction(ForwardInstructionIterator* it, +static void ReplaceCurrentInstruction(ForwardInstructionIterator* iterator, Instruction* current, Instruction* replacement, FlowGraph* graph) { @@ -200,7 +200,8 @@ static void ReplaceCurrentInstruction(ForwardInstructionIterator* it, OS::Print("Removing v%"Pd".\n", current_defn->ssa_temp_index()); } } - it->RemoveCurrentFromGraph(); + current->UnuseAllInputs(); + iterator->RemoveCurrentFromGraph(); } @@ -2680,7 +2681,10 @@ void RangeAnalysis::InferRangesRecursive(BlockEntryInstr* block) { CheckArrayBoundInstr* check = current->AsCheckArrayBound(); RangeBoundary array_length = RangeBoundary::FromDefinition(check->length()->definition()); - if (check->IsRedundant(array_length)) it.RemoveCurrentFromGraph(); + if (check->IsRedundant(array_length)) { + current->UnuseAllInputs(); + it.RemoveCurrentFromGraph(); + } } } @@ -2720,7 +2724,7 @@ void RangeAnalysis::RemoveConstraints() { def = def->AsConstraint()->value()->definition(); } constraints_[i]->ReplaceUsesWith(def); - constraints_[i]->RemoveDependency(); + constraints_[i]->UnuseAllInputs(); constraints_[i]->RemoveFromGraph(); } } @@ -3032,6 +3036,7 @@ void LICM::TryHoistCheckSmiThroughPhi(ForwardInstructionIterator* it, } if (phi->GetPropagatedCid() == kSmiCid) { + current->UnuseAllInputs(); it->RemoveCurrentFromGraph(); return; } @@ -3440,6 +3445,7 @@ class LoadOptimizer : public ValueObject { } defn->ReplaceUsesWith(replacement); + defn->UnuseAllInputs(); instr_it.RemoveCurrentFromGraph(); continue; } else if (!kill->Contains(expr_id)) { @@ -3651,6 +3657,7 @@ class LoadOptimizer : public ValueObject { } load->ReplaceUsesWith(replacement); + load->UnuseAllInputs(); load->RemoveFromGraph(); load->SetReplacement(replacement); } diff --git a/runtime/vm/intermediate_language.cc b/runtime/vm/intermediate_language.cc index f1c40a91a4c..7b8e997150c 100644 --- a/runtime/vm/intermediate_language.cc +++ b/runtime/vm/intermediate_language.cc @@ -696,6 +696,16 @@ void Definition::ReplaceUsesWith(Definition* other) { } +void Instruction::UnuseAllInputs() { + for (intptr_t i = InputCount() - 1; i >= 0; --i) { + InputAt(i)->RemoveFromUseList(); + } + for (Environment::DeepIterator it(env()); !it.Done(); it.Advance()) { + it.CurrentValue()->RemoveFromUseList(); + } +} + + void Definition::ReplaceWith(Definition* other, ForwardInstructionIterator* iterator) { if ((iterator != NULL) && (this == iterator->Current())) { @@ -2131,13 +2141,20 @@ Environment* Environment::From(const GrowableArray& definitions, Environment* Environment::DeepCopy() const { + return (this == NULL) ? NULL : DeepCopy(Length()); +} + + +Environment* Environment::DeepCopy(intptr_t length) const { + ASSERT(length <= values_.length()); + if (this == NULL) return NULL; Environment* copy = - new Environment(values_.length(), + new Environment(length, fixed_parameter_count_, deopt_id_, function_, - (outer_ == NULL) ? NULL : outer_->DeepCopy()); - for (intptr_t i = 0; i < values_.length(); ++i) { + outer_->DeepCopy()); + for (intptr_t i = 0; i < length; ++i) { copy->values_.Add(values_[i]->Copy()); } return copy; @@ -2146,6 +2163,10 @@ Environment* Environment::DeepCopy() const { // Copies the environment and updates the environment use lists. void Environment::DeepCopyTo(Instruction* instr) const { + for (Environment::DeepIterator it(instr->env()); !it.Done(); it.Advance()) { + it.CurrentValue()->RemoveFromUseList(); + } + Environment* copy = DeepCopy(); intptr_t use_index = 0; for (Environment::DeepIterator it(copy); !it.Done(); it.Advance()) { @@ -2161,18 +2182,11 @@ void Environment::DeepCopyTo(Instruction* instr) const { // Copies the environment as outer on an inlined instruction and updates the // environment use lists. void Environment::DeepCopyToOuter(Instruction* instr) const { - ASSERT(instr->env()->outer() == NULL); // Create a deep copy removing caller arguments from the environment. + ASSERT(this != NULL); + ASSERT(instr->env()->outer() == NULL); intptr_t argument_count = instr->env()->fixed_parameter_count(); - Environment* copy = - new Environment(values_.length() - argument_count, - fixed_parameter_count_, - deopt_id_, - function_, - (outer_ == NULL) ? NULL : outer_->DeepCopy()); - for (intptr_t i = 0; i < values_.length() - argument_count; ++i) { - copy->values_.Add(values_[i]->Copy()); - } + Environment* copy = DeepCopy(values_.length() - argument_count); intptr_t use_index = instr->env()->Length(); // Start index after inner. for (Environment::DeepIterator it(copy); !it.Done(); it.Advance()) { Value* value = it.CurrentValue(); diff --git a/runtime/vm/intermediate_language.h b/runtime/vm/intermediate_language.h index bb5b94ad522..1d70a291fb7 100644 --- a/runtime/vm/intermediate_language.h +++ b/runtime/vm/intermediate_language.h @@ -359,6 +359,10 @@ class Instruction : public ZoneAllocated { virtual Value* InputAt(intptr_t i) const = 0; virtual void SetInputAt(intptr_t i, Value* value) = 0; + // Remove all inputs (including in the environment) from their + // definition's use lists. + void UnuseAllInputs(); + // Call instructions override this function and return the number of // pushed arguments. virtual intptr_t ArgumentCount() const = 0; @@ -1810,6 +1814,10 @@ class ConstraintInstr : public TemplateDefinition<2> { DECLARE_INSTRUCTION(Constraint) + virtual intptr_t InputCount() const { + return (inputs_[1] == NULL) ? 1 : 2; + } + virtual RawAbstractType* CompileType() const { return Type::SmiType(); } @@ -1840,13 +1848,6 @@ class ConstraintInstr : public TemplateDefinition<2> { set_dependency(val); } - void RemoveDependency() { - if (dependency() != NULL) { - dependency()->RemoveFromUseList(); - set_dependency(NULL); - } - } - private: Value* dependency() { return inputs_[1]; @@ -4466,7 +4467,11 @@ class Environment : public ZoneAllocated { function_(function), outer_(outer) { } + // Deep copy the environment. A 'length' parameter can be given, which + // may be less than the environment's length in order to drop values + // (e.g., passed arguments) from the copy. Environment* DeepCopy() const; + Environment* DeepCopy(intptr_t length) const; GrowableArray values_; Location* locations_;