Make SideEffectAnalysis a subphase SideEffectAnalysis is used in three other phases: GVN, LICM, BCE. This change is a no-op for GVN as SideEffectAnalysis would always run before GVN. We used to run only once for LICM+BCE. This CL changes that behavior to run SideEffectAnalysis inside each phase, if needed. This means graphs will run it 0, 1, or 2 times for LICM+BCE. In terms of compile time speed, this CL improves SideEffectAnalysis ~40-50%, which translates into ~0.2% improvement in compile speed based on local compiles. Test: art/test/testrunner/testrunner.py --host --64 --optimizing -b Flag: EXEMPT refactor Change-Id: Ibb63e96df4c911bcc8225afa099c79f281c3e300
diff --git a/compiler/optimizing/bounds_check_elimination.cc b/compiler/optimizing/bounds_check_elimination.cc index 40e6fa2..b44e3b0 100644 --- a/compiler/optimizing/bounds_check_elimination.cc +++ b/compiler/optimizing/bounds_check_elimination.cc
@@ -18,6 +18,7 @@ #include <limits> +#include "base/arena_allocator.h" #include "base/scoped_arena_allocator.h" #include "base/scoped_arena_containers.h" #include "induction_var_range.h" @@ -2071,12 +2072,15 @@ return false; } + SideEffectsAnalysis side_effects(graph_); + side_effects.Run(); + // Reverse post order guarantees a node's dominators are visited first. // We want to visit in the dominator-based order since if a value is known to // be bounded by a range at one instruction, it must be true that all uses of // that value dominated by that instruction fits in that range. Range of that // value can be narrowed further down in the dominator tree. - BCEVisitor visitor(graph_, side_effects_, induction_analysis_); + BCEVisitor visitor(graph_, side_effects, induction_analysis_); for (size_t i = 0, size = graph_->GetReversePostOrder().size(); i != size; ++i) { HBasicBlock* current = graph_->GetReversePostOrder()[i]; if (visitor.IsAddedBlock(current)) {
diff --git a/compiler/optimizing/bounds_check_elimination.h b/compiler/optimizing/bounds_check_elimination.h index f210fa9..ade7535 100644 --- a/compiler/optimizing/bounds_check_elimination.h +++ b/compiler/optimizing/bounds_check_elimination.h
@@ -22,17 +22,14 @@ namespace art HIDDEN { -class SideEffectsAnalysis; class HInductionVarAnalysis; class BoundsCheckElimination : public HOptimization { public: BoundsCheckElimination(HGraph* graph, - const SideEffectsAnalysis& side_effects, HInductionVarAnalysis* induction_analysis, const char* name = kBoundsCheckEliminationPassName) : HOptimization(graph, name), - side_effects_(side_effects), induction_analysis_(induction_analysis) {} bool Run() override; @@ -40,7 +37,6 @@ static constexpr const char* kBoundsCheckEliminationPassName = "BCE"; private: - const SideEffectsAnalysis& side_effects_; HInductionVarAnalysis* induction_analysis_; DISALLOW_COPY_AND_ASSIGN(BoundsCheckElimination);
diff --git a/compiler/optimizing/bounds_check_elimination_test.cc b/compiler/optimizing/bounds_check_elimination_test.cc index 246cd10..8fd46ee 100644 --- a/compiler/optimizing/bounds_check_elimination_test.cc +++ b/compiler/optimizing/bounds_check_elimination_test.cc
@@ -41,15 +41,12 @@ InstructionSimplifier(graph_, /* codegen= */ nullptr).Run(); - SideEffectsAnalysis side_effects(graph_); - side_effects.Run(); - - GVNOptimization(graph_, side_effects).Run(); + GVNOptimization(graph_).Run(); HInductionVarAnalysis induction(graph_); induction.Run(); - BoundsCheckElimination(graph_, side_effects, &induction).Run(); + BoundsCheckElimination(graph_, &induction).Run(); } HInstruction* BuildSSAGraph1(int initial, int increment, IfCondition cond = kCondGE);
diff --git a/compiler/optimizing/gvn.cc b/compiler/optimizing/gvn.cc index 188bfa4..248c3cb 100644 --- a/compiler/optimizing/gvn.cc +++ b/compiler/optimizing/gvn.cc
@@ -357,10 +357,10 @@ */ class GlobalValueNumberer : public ValueObject { public: - GlobalValueNumberer(HGraph* graph, const SideEffectsAnalysis& side_effects) + explicit GlobalValueNumberer(HGraph* graph) : graph_(graph), allocator_(graph->GetArenaStack()), - side_effects_(side_effects), + side_effects_(nullptr), sets_(graph->GetBlocks().size(), nullptr, allocator_.Adapter(kArenaAllocGvn)), dominated_to_visit_(graph->GetBlocks().size(), allocator_.Adapter(kArenaAllocGvn)), successors_to_visit_(graph->GetBlocks().size(), allocator_.Adapter(kArenaAllocGvn)), @@ -384,7 +384,7 @@ HGraph* graph_; ScopedArenaAllocator allocator_; - const SideEffectsAnalysis& side_effects_; + SideEffectsAnalysis* side_effects_; ValueSet* FindSetFor(HBasicBlock* block) const { ValueSet* result = sets_[block->GetBlockId()]; @@ -432,7 +432,13 @@ }; bool GlobalValueNumberer::Run() { - DCHECK(side_effects_.HasRun()); + if (graph_->HasLoops()) { + // SideEffectsAnalysis is only used when the graph has loops. + side_effects_ = new (&allocator_) SideEffectsAnalysis(graph_); + side_effects_->Run(); + DCHECK(side_effects_->HasRun()); + } + sets_[graph_->GetEntryBlock()->GetBlockId()] = new (&allocator_) ValueSet(&allocator_); // Use the reverse post order to ensure the non back-edge predecessors of a block are @@ -492,7 +498,7 @@ } else { DCHECK(!block->GetLoopInformation()->IsIrreducible()); DCHECK_EQ(block->GetDominator(), block->GetLoopInformation()->GetPreHeader()); - set->Kill(side_effects_.GetLoopEffects(block)); + set->Kill(side_effects_->GetLoopEffects(block)); } } else if (predecessors.size() > 1) { for (HBasicBlock* predecessor : predecessors) { @@ -602,7 +608,7 @@ } bool GVNOptimization::Run() { - GlobalValueNumberer gvn(graph_, side_effects_); + GlobalValueNumberer gvn(graph_); return gvn.Run(); }
diff --git a/compiler/optimizing/gvn.h b/compiler/optimizing/gvn.h index df4e3a8..5bce66f 100644 --- a/compiler/optimizing/gvn.h +++ b/compiler/optimizing/gvn.h
@@ -23,22 +23,16 @@ namespace art HIDDEN { -class SideEffectsAnalysis; - class GVNOptimization : public HOptimization { public: - GVNOptimization(HGraph* graph, - const SideEffectsAnalysis& side_effects, - const char* pass_name = kGlobalValueNumberingPassName) - : HOptimization(graph, pass_name), side_effects_(side_effects) {} + explicit GVNOptimization(HGraph* graph, const char* pass_name = kGlobalValueNumberingPassName) + : HOptimization(graph, pass_name) {} bool Run() override; static constexpr const char* kGlobalValueNumberingPassName = "GVN"; private: - const SideEffectsAnalysis& side_effects_; - DISALLOW_COPY_AND_ASSIGN(GVNOptimization); };
diff --git a/compiler/optimizing/gvn_test.cc b/compiler/optimizing/gvn_test.cc index fba53ee..5ad5154 100644 --- a/compiler/optimizing/gvn_test.cc +++ b/compiler/optimizing/gvn_test.cc
@@ -47,9 +47,7 @@ ASSERT_EQ(use_after_kill->GetBlock(), block); graph_->BuildDominatorTree(); - SideEffectsAnalysis side_effects(graph_); - side_effects.Run(); - GVNOptimization(graph_, side_effects).Run(); + GVNOptimization(graph_).Run(); ASSERT_TRUE(to_remove->GetBlock() == nullptr); ASSERT_EQ(different_offset->GetBlock(), block); @@ -71,9 +69,7 @@ MakeIFieldGet(join, parameter, DataType::Type::kBool, MemberOffset(42)); graph_->BuildDominatorTree(); - SideEffectsAnalysis side_effects(graph_); - side_effects.Run(); - GVNOptimization(graph_, side_effects).Run(); + GVNOptimization(graph_).Run(); // Check that all field get instructions have been GVN'ed. ASSERT_TRUE(then->GetFirstInstruction()->IsGoto()); @@ -109,11 +105,7 @@ ASSERT_EQ(field_get_in_return_block->GetBlock(), return_block); graph_->BuildDominatorTree(); - { - SideEffectsAnalysis side_effects(graph_); - side_effects.Run(); - GVNOptimization(graph_, side_effects).Run(); - } + GVNOptimization(graph_).Run(); // Check that all field get instructions are still there. ASSERT_EQ(field_get_in_loop_header->GetBlock(), loop_header); @@ -124,11 +116,7 @@ // Now remove the field set, and check that all field get instructions have been GVN'ed. loop_body->RemoveInstruction(field_set); - { - SideEffectsAnalysis side_effects(graph_); - side_effects.Run(); - GVNOptimization(graph_, side_effects).Run(); - } + GVNOptimization(graph_).Run(); ASSERT_TRUE(field_get_in_loop_header->GetBlock() == nullptr); ASSERT_TRUE(field_get_in_loop_body->GetBlock() == nullptr);
diff --git a/compiler/optimizing/licm.cc b/compiler/optimizing/licm.cc index 901ed40..b032c8f 100644 --- a/compiler/optimizing/licm.cc +++ b/compiler/optimizing/licm.cc
@@ -81,8 +81,16 @@ } bool LICM::Run() { + if (!graph_->HasLoops()) { + // Nothing to do. + return false; + } + bool didLICM = false; - DCHECK(side_effects_.HasRun()); + SideEffectsAnalysis side_effects(graph_); + side_effects.Run(); + + DCHECK(side_effects.HasRun()); // Only used during debug. ArenaBitVector* visited = nullptr; @@ -101,7 +109,7 @@ } HLoopInformation* loop_info = block->GetLoopInformation(); - SideEffects loop_effects = side_effects_.GetLoopEffects(block); + SideEffects loop_effects = side_effects.GetLoopEffects(block); HBasicBlock* pre_header = loop_info->GetPreHeader(); for (HBlocksInLoopIterator it_loop(*loop_info); !it_loop.Done(); it_loop.Advance()) {
diff --git a/compiler/optimizing/licm.h b/compiler/optimizing/licm.h index 1a86b6e..b82d9a6 100644 --- a/compiler/optimizing/licm.h +++ b/compiler/optimizing/licm.h
@@ -23,24 +23,18 @@ namespace art HIDDEN { -class SideEffectsAnalysis; - class LICM : public HOptimization { public: LICM(HGraph* graph, - const SideEffectsAnalysis& side_effects, OptimizingCompilerStats* stats, const char* name = kLoopInvariantCodeMotionPassName) - : HOptimization(graph, name, stats), - side_effects_(side_effects) {} + : HOptimization(graph, name, stats) {} bool Run() override; static constexpr const char* kLoopInvariantCodeMotionPassName = "licm"; private: - const SideEffectsAnalysis& side_effects_; - DISALLOW_COPY_AND_ASSIGN(LICM); };
diff --git a/compiler/optimizing/licm_test.cc b/compiler/optimizing/licm_test.cc index 08a2dcc..5ae5748 100644 --- a/compiler/optimizing/licm_test.cc +++ b/compiler/optimizing/licm_test.cc
@@ -59,9 +59,7 @@ // Performs LICM optimizations (after proper set up). void PerformLICM() { graph_->BuildDominatorTree(); - SideEffectsAnalysis side_effects(graph_); - side_effects.Run(); - LICM(graph_, side_effects, nullptr).Run(); + LICM(graph_, nullptr).Run(); } // Specific basic blocks.
diff --git a/compiler/optimizing/load_store_elimination.h b/compiler/optimizing/load_store_elimination.h index e771685..60164e1 100644 --- a/compiler/optimizing/load_store_elimination.h +++ b/compiler/optimizing/load_store_elimination.h
@@ -22,8 +22,6 @@ namespace art HIDDEN { -class SideEffectsAnalysis; - class LoadStoreElimination : public HOptimization { public: // Controls whether to enable VLOG(compiler) logs explaining the transforms taking place.
diff --git a/compiler/optimizing/optimization.cc b/compiler/optimizing/optimization.cc index 3178073..c621293 100644 --- a/compiler/optimizing/optimization.cc +++ b/compiler/optimizing/optimization.cc
@@ -68,8 +68,6 @@ const char* OptimizationPassName(OptimizationPass pass) { switch (pass) { - case OptimizationPass::kSideEffectsAnalysis: - return SideEffectsAnalysis::kSideEffectsAnalysisPassName; case OptimizationPass::kInductionVarAnalysis: return HInductionVarAnalysis::kInductionPassName; case OptimizationPass::kGlobalValueNumbering: @@ -160,7 +158,6 @@ X(OptimizationPass::kLoopOptimization); X(OptimizationPass::kReferenceTypePropagation); X(OptimizationPass::kScheduling); - X(OptimizationPass::kSideEffectsAnalysis); #ifdef ART_ENABLE_CODEGEN_arm X(OptimizationPass::kInstructionSimplifierArm); X(OptimizationPass::kCriticalNativeAbiFixupArm); @@ -192,10 +189,9 @@ const DexCompilationUnit& dex_compilation_unit) { ArenaVector<HOptimization*> optimizations(allocator->Adapter()); - // Some optimizations require SideEffectsAnalysis or HInductionVarAnalysis + // Some optimizations require HInductionVarAnalysis // instances. This method uses the nearest instance preceeding it in the pass // name list or fails fatally if no such analysis can be found. - SideEffectsAnalysis* most_recent_side_effects = nullptr; HInductionVarAnalysis* most_recent_induction = nullptr; // Loop over the requested optimizations. @@ -211,9 +207,6 @@ // // Analysis passes (kept in most recent for subsequent passes). // - case OptimizationPass::kSideEffectsAnalysis: - opt = most_recent_side_effects = new (allocator) SideEffectsAnalysis(graph, pass_name); - break; case OptimizationPass::kInductionVarAnalysis: opt = most_recent_induction = new (allocator) HInductionVarAnalysis(graph, stats, pass_name); @@ -222,12 +215,10 @@ // Passes that need prior analysis. // case OptimizationPass::kGlobalValueNumbering: - CHECK(most_recent_side_effects != nullptr); - opt = new (allocator) GVNOptimization(graph, *most_recent_side_effects, pass_name); + opt = new (allocator) GVNOptimization(graph, pass_name); break; case OptimizationPass::kInvariantCodeMotion: - CHECK(most_recent_side_effects != nullptr); - opt = new (allocator) LICM(graph, *most_recent_side_effects, stats, pass_name); + opt = new (allocator) LICM(graph, stats, pass_name); break; case OptimizationPass::kLoopOptimization: CHECK(most_recent_induction != nullptr); @@ -235,9 +226,8 @@ graph, *codegen, most_recent_induction, stats, pass_name); break; case OptimizationPass::kBoundsCheckElimination: - CHECK(most_recent_side_effects != nullptr && most_recent_induction != nullptr); - opt = new (allocator) BoundsCheckElimination( - graph, *most_recent_side_effects, most_recent_induction, pass_name); + CHECK(most_recent_induction != nullptr); + opt = new (allocator) BoundsCheckElimination(graph, most_recent_induction, pass_name); break; // // Regular passes.
diff --git a/compiler/optimizing/optimization.h b/compiler/optimizing/optimization.h index 575d344..19c961c 100644 --- a/compiler/optimizing/optimization.h +++ b/compiler/optimizing/optimization.h
@@ -84,7 +84,6 @@ kLoopOptimization, kReferenceTypePropagation, kScheduling, - kSideEffectsAnalysis, kWriteBarrierElimination, #ifdef ART_ENABLE_CODEGEN_arm kInstructionSimplifierArm,
diff --git a/compiler/optimizing/optimizing_compiler.cc b/compiler/optimizing/optimizing_compiler.cc index b979d03..1fea4cc 100644 --- a/compiler/optimizing/optimizing_compiler.cc +++ b/compiler/optimizing/optimizing_compiler.cc
@@ -505,7 +505,6 @@ case InstructionSet::kArm: { static constexpr OptimizationDef arm_optimizations[] = { OptDef(OptimizationPass::kInstructionSimplifierArm), - OptDef(OptimizationPass::kSideEffectsAnalysis), OptDef(OptimizationPass::kGlobalValueNumbering, "GVN$after_arch"), OptDef(OptimizationPass::kCriticalNativeAbiFixupArm), OptDef(OptimizationPass::kScheduling) @@ -521,7 +520,6 @@ case InstructionSet::kArm64: { static constexpr OptimizationDef arm64_optimizations[] = { OptDef(OptimizationPass::kInstructionSimplifierArm64), - OptDef(OptimizationPass::kSideEffectsAnalysis), OptDef(OptimizationPass::kGlobalValueNumbering, "GVN$after_arch"), OptDef(OptimizationPass::kScheduling) }; @@ -536,7 +534,6 @@ case InstructionSet::kRiscv64: { static constexpr OptimizationDef riscv64_optimizations[] = { OptDef(OptimizationPass::kInstructionSimplifierRiscv64), - OptDef(OptimizationPass::kSideEffectsAnalysis), OptDef(OptimizationPass::kGlobalValueNumbering, "GVN$after_arch"), OptDef(OptimizationPass::kCriticalNativeAbiFixupRiscv64) }; @@ -551,7 +548,6 @@ case InstructionSet::kX86: { static constexpr OptimizationDef x86_optimizations[] = { OptDef(OptimizationPass::kInstructionSimplifierX86), - OptDef(OptimizationPass::kSideEffectsAnalysis), OptDef(OptimizationPass::kGlobalValueNumbering, "GVN$after_arch"), OptDef(OptimizationPass::kPcRelativeFixupsX86), OptDef(OptimizationPass::kX86MemoryOperandGeneration) @@ -567,7 +563,6 @@ case InstructionSet::kX86_64: { static constexpr OptimizationDef x86_64_optimizations[] = { OptDef(OptimizationPass::kInstructionSimplifierX86_64), - OptDef(OptimizationPass::kSideEffectsAnalysis), OptDef(OptimizationPass::kGlobalValueNumbering, "GVN$after_arch"), OptDef(OptimizationPass::kX86MemoryOperandGeneration) }; @@ -661,8 +656,6 @@ "dead_code_elimination$after_inlining", OptimizationPass::kInliner), // GVN. - OptDef(OptimizationPass::kSideEffectsAnalysis, - "side_effects$before_gvn"), OptDef(OptimizationPass::kGlobalValueNumbering), OptDef(OptimizationPass::kReferenceTypePropagation, "reference_type_propagation$after_gvn", @@ -676,8 +669,6 @@ OptDef(OptimizationPass::kDeadCodeElimination, "dead_code_elimination$after_gvn"), // High-level optimizations. - OptDef(OptimizationPass::kSideEffectsAnalysis, - "side_effects$before_licm"), OptDef(OptimizationPass::kInvariantCodeMotion), OptDef(OptimizationPass::kInductionVarAnalysis), OptDef(OptimizationPass::kBoundsCheckElimination),
diff --git a/compiler/optimizing/side_effects_analysis.cc b/compiler/optimizing/side_effects_analysis.cc index d0498cc..7295564 100644 --- a/compiler/optimizing/side_effects_analysis.cc +++ b/compiler/optimizing/side_effects_analysis.cc
@@ -19,11 +19,6 @@ namespace art HIDDEN { bool SideEffectsAnalysis::Run() { - // Inlining might have created more blocks, so we need to increase the size - // if needed. - block_effects_.resize(graph_->GetBlocks().size()); - loop_effects_.resize(graph_->GetBlocks().size()); - // In DEBUG mode, ensure side effects are properly initialized to empty. if (kIsDebugBuild) { for (HBasicBlock* block : graph_->GetReversePostOrder()) {
diff --git a/compiler/optimizing/side_effects_analysis.h b/compiler/optimizing/side_effects_analysis.h index bb2d794..b512cee 100644 --- a/compiler/optimizing/side_effects_analysis.h +++ b/compiler/optimizing/side_effects_analysis.h
@@ -20,17 +20,17 @@ #include "base/arena_containers.h" #include "base/macros.h" #include "nodes.h" -#include "optimization.h" namespace art HIDDEN { -class SideEffectsAnalysis : public HOptimization { +class SideEffectsAnalysis : public ArenaObject<kArenaAllocOptimization> { public: - explicit SideEffectsAnalysis(HGraph* graph, const char* pass_name = kSideEffectsAnalysisPassName) - : HOptimization(graph, pass_name), - graph_(graph), - block_effects_(graph->GetAllocator()->Adapter(kArenaAllocSideEffectsAnalysis)), - loop_effects_(graph->GetAllocator()->Adapter(kArenaAllocSideEffectsAnalysis)) {} + explicit SideEffectsAnalysis(HGraph* graph) + : graph_(graph), + block_effects_(graph_->GetBlocks().size(), + graph->GetAllocator()->Adapter(kArenaAllocSideEffectsAnalysis)), + loop_effects_(graph_->GetBlocks().size(), + graph->GetAllocator()->Adapter(kArenaAllocSideEffectsAnalysis)) {} SideEffects GetLoopEffects(HBasicBlock* block) const; SideEffects GetBlockEffects(HBasicBlock* block) const; @@ -40,8 +40,6 @@ bool HasRun() const { return has_run_; } - static constexpr const char* kSideEffectsAnalysisPassName = "side_effects"; - private: void UpdateLoopEffects(HLoopInformation* info, SideEffects effects);