OSDN Git Service

ART: Run GraphChecker after Builder and SsaBuilder
authorDavid Brazdil <dbrazdil@google.com>
Tue, 23 Jun 2015 17:27:30 +0000 (18:27 +0100)
committerDavid Brazdil <dbrazdil@google.com>
Wed, 24 Jun 2015 14:02:15 +0000 (15:02 +0100)
This patch refactors the way GraphChecker is invoked, utilizing the
same scoping mechanism as pass timing and graph visualizer. Therefore,
GraphChecker will now run not just after instances of HOptimization
but after the builders and reg alloc, too.

Change-Id: I8173b98b79afa95e1fcbf3ac9630a873d7f6c1d4

16 files changed:
compiler/optimizing/boolean_simplifier.h
compiler/optimizing/bounds_check_elimination.h
compiler/optimizing/constant_folding.h
compiler/optimizing/dead_code_elimination.h
compiler/optimizing/gvn.h
compiler/optimizing/inliner.h
compiler/optimizing/instruction_simplifier.h
compiler/optimizing/intrinsics.h
compiler/optimizing/licm.h
compiler/optimizing/nodes.h
compiler/optimizing/optimization.cc
compiler/optimizing/optimization.h
compiler/optimizing/optimizing_compiler.cc
compiler/optimizing/reference_type_propagation.h
compiler/optimizing/side_effects_analysis.h
compiler/optimizing/ssa_phi_elimination.h

index 733ebaa..e12a12c 100644 (file)
@@ -63,7 +63,7 @@ namespace art {
 class HBooleanSimplifier : public HOptimization {
  public:
   explicit HBooleanSimplifier(HGraph* graph)
-    : HOptimization(graph, true, kBooleanSimplifierPassName) {}
+    : HOptimization(graph, kBooleanSimplifierPassName) {}
 
   void Run() OVERRIDE;
 
index 9e98ccf..24b8ea7 100644 (file)
@@ -24,7 +24,7 @@ namespace art {
 class BoundsCheckElimination : public HOptimization {
  public:
   explicit BoundsCheckElimination(HGraph* graph)
-      : HOptimization(graph, true, kBoundsCheckEliminiationPassName) {}
+      : HOptimization(graph, kBoundsCheckEliminiationPassName) {}
 
   void Run() OVERRIDE;
 
index 66ff578..2698b2d 100644 (file)
@@ -33,7 +33,7 @@ namespace art {
 class HConstantFolding : public HOptimization {
  public:
   explicit HConstantFolding(HGraph* graph, const char* name = kConstantFoldingPassName)
-      : HOptimization(graph, true, name) {}
+      : HOptimization(graph, name) {}
 
   void Run() OVERRIDE;
 
index 59a57c4..8d6008b 100644 (file)
@@ -32,7 +32,7 @@ class HDeadCodeElimination : public HOptimization {
   HDeadCodeElimination(HGraph* graph,
                        OptimizingCompilerStats* stats = nullptr,
                        const char* name = kInitialDeadCodeEliminationPassName)
-      : HOptimization(graph, true, name, stats) {}
+      : HOptimization(graph, name, stats) {}
 
   void Run() OVERRIDE;
 
index e74d969..14a503b 100644 (file)
@@ -27,7 +27,7 @@ class SideEffectsAnalysis;
 class GVNOptimization : public HOptimization {
  public:
   GVNOptimization(HGraph* graph, const SideEffectsAnalysis& side_effects)
-      : HOptimization(graph, true, kGlobalValueNumberingPassName), side_effects_(side_effects) {}
+      : HOptimization(graph, kGlobalValueNumberingPassName), side_effects_(side_effects) {}
 
   void Run() OVERRIDE;
 
index 24044b7..ffd7569 100644 (file)
@@ -37,7 +37,7 @@ class HInliner : public HOptimization {
            StackHandleScopeCollection* handles,
            OptimizingCompilerStats* stats,
            size_t depth = 0)
-      : HOptimization(outer_graph, true, kInlinerPassName, stats),
+      : HOptimization(outer_graph, kInlinerPassName, stats),
         outer_compilation_unit_(outer_compilation_unit),
         caller_compilation_unit_(caller_compilation_unit),
         compiler_driver_(compiler_driver),
index 668956a..faee2dd 100644 (file)
@@ -31,7 +31,7 @@ class InstructionSimplifier : public HOptimization {
   InstructionSimplifier(HGraph* graph,
                         OptimizingCompilerStats* stats = nullptr,
                         const char* name = kInstructionSimplifierPassName)
-    : HOptimization(graph, true, name, stats) {}
+    : HOptimization(graph, name, stats) {}
 
   static constexpr const char* kInstructionSimplifierPassName = "instruction_simplifier";
 
index 741fb64..9044982 100644 (file)
@@ -31,7 +31,7 @@ class DexFile;
 class IntrinsicsRecognizer : public HOptimization {
  public:
   IntrinsicsRecognizer(HGraph* graph, CompilerDriver* driver)
-      : HOptimization(graph, true, kIntrinsicsRecognizerPassName),
+      : HOptimization(graph, kIntrinsicsRecognizerPassName),
         driver_(driver) {}
 
   void Run() OVERRIDE;
index cb6170e..0b5a0f1 100644 (file)
@@ -27,7 +27,7 @@ class SideEffectsAnalysis;
 class LICM : public HOptimization {
  public:
   LICM(HGraph* graph, const SideEffectsAnalysis& side_effects)
-      : HOptimization(graph, true, kLoopInvariantCodeMotionPassName), side_effects_(side_effects) {}
+      : HOptimization(graph, kLoopInvariantCodeMotionPassName), side_effects_(side_effects) {}
 
   void Run() OVERRIDE;
 
index 581645f..0f5b1ab 100644 (file)
@@ -160,6 +160,8 @@ class HGraph : public ArenaObject<kArenaAllocMisc> {
   const GrowableArray<HBasicBlock*>& GetBlocks() const { return blocks_; }
   HBasicBlock* GetBlock(size_t id) const { return blocks_.Get(id); }
 
+  bool IsInSsaForm() const { return in_ssa_form_; }
+
   HBasicBlock* GetEntryBlock() const { return entry_block_; }
   HBasicBlock* GetExitBlock() const { return exit_block_; }
   bool HasExitBlock() const { return exit_block_ != nullptr; }
index c46a219..3d76949 100644 (file)
@@ -16,9 +16,6 @@
 
 #include "optimization.h"
 
-#include "base/dumpable.h"
-#include "graph_checker.h"
-
 namespace art {
 
 void HOptimization::MaybeRecordStat(MethodCompilationStat compilation_stat, size_t count) const {
@@ -27,24 +24,4 @@ void HOptimization::MaybeRecordStat(MethodCompilationStat compilation_stat, size
   }
 }
 
-void HOptimization::Check() {
-  if (kIsDebugBuild) {
-    if (is_in_ssa_form_) {
-      SSAChecker checker(graph_->GetArena(), graph_);
-      checker.Run();
-      if (!checker.IsValid()) {
-        LOG(FATAL) << "Error after " << GetPassName() << ": "
-                   << Dumpable<SSAChecker>(checker);
-      }
-    } else {
-      GraphChecker checker(graph_->GetArena(), graph_);
-      checker.Run();
-      if (!checker.IsValid()) {
-        LOG(FATAL) << "Error after " << GetPassName() << ": "
-                   << Dumpable<GraphChecker>(checker);
-      }
-    }
-  }
-}
-
 }  // namespace art
index 2d1c0ba..bc56546 100644 (file)
@@ -29,12 +29,10 @@ namespace art {
 class HOptimization : public ArenaObject<kArenaAllocMisc> {
  public:
   HOptimization(HGraph* graph,
-                bool is_in_ssa_form,
                 const char* pass_name,
                 OptimizingCompilerStats* stats = nullptr)
       : graph_(graph),
         stats_(stats),
-        is_in_ssa_form_(is_in_ssa_form),
         pass_name_(pass_name) {}
 
   virtual ~HOptimization() {}
@@ -45,9 +43,6 @@ class HOptimization : public ArenaObject<kArenaAllocMisc> {
   // Peform the analysis itself.
   virtual void Run() = 0;
 
-  // Verify the graph; abort if it is not valid.
-  void Check();
-
  protected:
   void MaybeRecordStat(MethodCompilationStat compilation_stat, size_t count = 1) const;
 
@@ -56,8 +51,6 @@ class HOptimization : public ArenaObject<kArenaAllocMisc> {
   OptimizingCompilerStats* const stats_;
 
  private:
-  // Does the analyzed graph use the SSA form?
-  const bool is_in_ssa_form_;
   // Optimization pass name.
   const char* pass_name_;
 
index 0c7b6f7..1466366 100644 (file)
@@ -38,6 +38,7 @@
 #include "driver/compiler_options.h"
 #include "driver/dex_compilation_unit.h"
 #include "elf_writer_quick.h"
+#include "graph_checker.h"
 #include "graph_visualizer.h"
 #include "gvn.h"
 #include "inliner.h"
@@ -86,21 +87,23 @@ class CodeVectorAllocator FINAL : public CodeAllocator {
  */
 static const char* kStringFilter = "";
 
-class PassInfo;
+class PassScope;
 
-class PassInfoPrinter : public ValueObject {
+class PassObserver : public ValueObject {
  public:
-  PassInfoPrinter(HGraph* graph,
-                  const char* method_name,
-                  CodeGenerator* codegen,
-                  std::ostream* visualizer_output,
-                  CompilerDriver* compiler_driver)
-      : method_name_(method_name),
+  PassObserver(HGraph* graph,
+               const char* method_name,
+               CodeGenerator* codegen,
+               std::ostream* visualizer_output,
+               CompilerDriver* compiler_driver)
+      : graph_(graph),
+        method_name_(method_name),
         timing_logger_enabled_(compiler_driver->GetDumpPasses()),
         timing_logger_(method_name, true, true),
         disasm_info_(graph->GetArena()),
         visualizer_enabled_(!compiler_driver->GetDumpCfgFileName().empty()),
-        visualizer_(visualizer_output, graph, *codegen) {
+        visualizer_(visualizer_output, graph, *codegen),
+        graph_in_bad_state_(false) {
     if (strstr(method_name, kStringFilter) == nullptr) {
       timing_logger_enabled_ = visualizer_enabled_ = false;
     }
@@ -110,7 +113,7 @@ class PassInfoPrinter : public ValueObject {
     }
   }
 
-  ~PassInfoPrinter() {
+  ~PassObserver() {
     if (timing_logger_enabled_) {
       LOG(INFO) << "TIMINGS " << method_name_;
       LOG(INFO) << Dumpable<TimingLogger>(timing_logger_);
@@ -123,6 +126,8 @@ class PassInfoPrinter : public ValueObject {
     }
   }
 
+  void SetGraphInBadState() { graph_in_bad_state_ = true; }
+
  private:
   void StartPass(const char* pass_name) {
     // Dump graph first, then start timer.
@@ -142,8 +147,28 @@ class PassInfoPrinter : public ValueObject {
     if (visualizer_enabled_) {
       visualizer_.DumpGraph(pass_name, /* is_after_pass */ true);
     }
+
+    // Validate the HGraph if running in debug mode.
+    if (kIsDebugBuild) {
+      if (!graph_in_bad_state_) {
+        if (graph_->IsInSsaForm()) {
+          SSAChecker checker(graph_->GetArena(), graph_);
+          checker.Run();
+          if (!checker.IsValid()) {
+            LOG(FATAL) << "Error after " << pass_name << ": " << Dumpable<SSAChecker>(checker);
+          }
+        } else {
+          GraphChecker checker(graph_->GetArena(), graph_);
+          checker.Run();
+          if (!checker.IsValid()) {
+            LOG(FATAL) << "Error after " << pass_name << ": " << Dumpable<GraphChecker>(checker);
+          }
+        }
+      }
+    }
   }
 
+  HGraph* const graph_;
   const char* method_name_;
 
   bool timing_logger_enabled_;
@@ -154,26 +179,30 @@ class PassInfoPrinter : public ValueObject {
   bool visualizer_enabled_;
   HGraphVisualizer visualizer_;
 
-  friend PassInfo;
+  // Flag to be set by the compiler if the pass failed and the graph is not
+  // expected to validate.
+  bool graph_in_bad_state_;
 
-  DISALLOW_COPY_AND_ASSIGN(PassInfoPrinter);
+  friend PassScope;
+
+  DISALLOW_COPY_AND_ASSIGN(PassObserver);
 };
 
-class PassInfo : public ValueObject {
+class PassScope : public ValueObject {
  public:
-  PassInfo(const char *pass_name, PassInfoPrinter* pass_info_printer)
+  PassScope(const char *pass_name, PassObserver* pass_observer)
       : pass_name_(pass_name),
-        pass_info_printer_(pass_info_printer) {
-    pass_info_printer_->StartPass(pass_name_);
+        pass_observer_(pass_observer) {
+    pass_observer_->StartPass(pass_name_);
   }
 
-  ~PassInfo() {
-    pass_info_printer_->EndPass(pass_name_);
+  ~PassScope() {
+    pass_observer_->EndPass(pass_name_);
   }
 
  private:
   const char* const pass_name_;
-  PassInfoPrinter* const pass_info_printer_;
+  PassObserver* const pass_observer_;
 };
 
 class OptimizingCompiler FINAL : public Compiler {
@@ -234,13 +263,13 @@ class OptimizingCompiler FINAL : public Compiler {
                                    CodeGenerator* codegen,
                                    CompilerDriver* driver,
                                    const DexCompilationUnit& dex_compilation_unit,
-                                   PassInfoPrinter* pass_info_printer) const;
+                                   PassObserver* pass_observer) const;
 
   // Just compile without doing optimizations.
   CompiledMethod* CompileBaseline(CodeGenerator* codegen,
                                   CompilerDriver* driver,
                                   const DexCompilationUnit& dex_compilation_unit,
-                                  PassInfoPrinter* pass_info_printer) const;
+                                  PassObserver* pass_observer) const;
 
   std::unique_ptr<OptimizingCompilerStats> compilation_stats_;
 
@@ -313,14 +342,10 @@ static bool CanOptimize(const DexFile::CodeItem& code_item) {
 
 static void RunOptimizations(HOptimization* optimizations[],
                              size_t length,
-                             PassInfoPrinter* pass_info_printer) {
+                             PassObserver* pass_observer) {
   for (size_t i = 0; i < length; ++i) {
-    HOptimization* optimization = optimizations[i];
-    {
-      PassInfo pass_info(optimization->GetPassName(), pass_info_printer);
-      optimization->Run();
-    }
-    optimization->Check();
+    PassScope scope(optimizations[i]->GetPassName(), pass_observer);
+    optimizations[i]->Run();
   }
 }
 
@@ -328,7 +353,7 @@ static void RunOptimizations(HGraph* graph,
                              CompilerDriver* driver,
                              OptimizingCompilerStats* stats,
                              const DexCompilationUnit& dex_compilation_unit,
-                             PassInfoPrinter* pass_info_printer,
+                             PassObserver* pass_observer,
                              StackHandleScopeCollection* handles) {
   ArenaAllocator* arena = graph->GetArena();
   HDeadCodeElimination* dce1 = new (arena) HDeadCodeElimination(
@@ -387,7 +412,7 @@ static void RunOptimizations(HGraph* graph,
     simplify4,
   };
 
-  RunOptimizations(optimizations, arraysize(optimizations), pass_info_printer);
+  RunOptimizations(optimizations, arraysize(optimizations), pass_observer);
 }
 
 // The stack map we generate must be 4-byte aligned on ARM. Since existing
@@ -403,15 +428,15 @@ static ArrayRef<const uint8_t> AlignVectorSize(std::vector<uint8_t>& vector) {
 
 static void AllocateRegisters(HGraph* graph,
                               CodeGenerator* codegen,
-                              PassInfoPrinter* pass_info_printer) {
+                              PassObserver* pass_observer) {
   PrepareForRegisterAllocation(graph).Run();
   SsaLivenessAnalysis liveness(graph, codegen);
   {
-    PassInfo pass_info(SsaLivenessAnalysis::kLivenessPassName, pass_info_printer);
+    PassScope scope(SsaLivenessAnalysis::kLivenessPassName, pass_observer);
     liveness.Analyze();
   }
   {
-    PassInfo pass_info(RegisterAllocator::kRegisterAllocatorPassName, pass_info_printer);
+    PassScope scope(RegisterAllocator::kRegisterAllocatorPassName, pass_observer);
     RegisterAllocator(graph->GetArena(), codegen, liveness).AllocateRegisters();
   }
 }
@@ -420,12 +445,12 @@ CompiledMethod* OptimizingCompiler::CompileOptimized(HGraph* graph,
                                                      CodeGenerator* codegen,
                                                      CompilerDriver* compiler_driver,
                                                      const DexCompilationUnit& dex_compilation_unit,
-                                                     PassInfoPrinter* pass_info_printer) const {
+                                                     PassObserver* pass_observer) const {
   StackHandleScopeCollection handles(Thread::Current());
   RunOptimizations(graph, compiler_driver, compilation_stats_.get(),
-                   dex_compilation_unit, pass_info_printer, &handles);
+                   dex_compilation_unit, pass_observer, &handles);
 
-  AllocateRegisters(graph, codegen, pass_info_printer);
+  AllocateRegisters(graph, codegen, pass_observer);
 
   CodeVectorAllocator allocator;
   codegen->CompileOptimized(&allocator);
@@ -456,7 +481,7 @@ CompiledMethod* OptimizingCompiler::CompileOptimized(HGraph* graph,
       ArrayRef<const uint8_t>(),  // native_gc_map.
       ArrayRef<const uint8_t>(*codegen->GetAssembler()->cfi().data()),
       ArrayRef<const LinkerPatch>());
-  pass_info_printer->DumpDisassembly();
+  pass_observer->DumpDisassembly();
   return compiled_method;
 }
 
@@ -464,7 +489,7 @@ CompiledMethod* OptimizingCompiler::CompileBaseline(
     CodeGenerator* codegen,
     CompilerDriver* compiler_driver,
     const DexCompilationUnit& dex_compilation_unit,
-    PassInfoPrinter* pass_info_printer) const {
+    PassObserver* pass_observer) const {
   CodeVectorAllocator allocator;
   codegen->CompileBaseline(&allocator);
 
@@ -496,7 +521,7 @@ CompiledMethod* OptimizingCompiler::CompileBaseline(
       AlignVectorSize(gc_map),
       ArrayRef<const uint8_t>(*codegen->GetAssembler()->cfi().data()),
       ArrayRef<const LinkerPatch>());
-  pass_info_printer->DumpDisassembly();
+  pass_observer->DumpDisassembly();
   return compiled_method;
 }
 
@@ -571,11 +596,11 @@ CompiledMethod* OptimizingCompiler::TryCompile(const DexFile::CodeItem* code_ite
   codegen->GetAssembler()->cfi().SetEnabled(
       compiler_driver->GetCompilerOptions().GetGenerateDebugInfo());
 
-  PassInfoPrinter pass_info_printer(graph,
-                                    method_name.c_str(),
-                                    codegen.get(),
-                                    visualizer_output_.get(),
-                                    compiler_driver);
+  PassObserver pass_observer(graph,
+                             method_name.c_str(),
+                             codegen.get(),
+                             visualizer_output_.get(),
+                             compiler_driver);
 
   HGraphBuilder builder(graph,
                         &dex_compilation_unit,
@@ -587,9 +612,10 @@ CompiledMethod* OptimizingCompiler::TryCompile(const DexFile::CodeItem* code_ite
   VLOG(compiler) << "Building " << method_name;
 
   {
-    PassInfo pass_info(HGraphBuilder::kBuilderPassName, &pass_info_printer);
+    PassScope scope(HGraphBuilder::kBuilderPassName, &pass_observer);
     if (!builder.BuildGraph(*code_item)) {
       CHECK(!shouldCompile) << "Could not build graph in optimizing compiler";
+      pass_observer.SetGraphInBadState();
       return nullptr;
     }
   }
@@ -605,7 +631,7 @@ CompiledMethod* OptimizingCompiler::TryCompile(const DexFile::CodeItem* code_ite
     VLOG(compiler) << "Optimizing " << method_name;
 
     {
-      PassInfo pass_info(SsaBuilder::kSsaBuilderPassName, &pass_info_printer);
+      PassScope scope(SsaBuilder::kSsaBuilderPassName, &pass_observer);
       if (!graph->TryBuildingSsa()) {
         // We could not transform the graph to SSA, bailout.
         LOG(INFO) << "Skipping compilation of " << method_name << ": it contains a non natural loop";
@@ -618,7 +644,7 @@ CompiledMethod* OptimizingCompiler::TryCompile(const DexFile::CodeItem* code_ite
                             codegen.get(),
                             compiler_driver,
                             dex_compilation_unit,
-                            &pass_info_printer);
+                            &pass_observer);
   } else if (shouldOptimize && can_allocate_registers) {
     LOG(FATAL) << "Could not allocate registers in optimizing compiler";
     UNREACHABLE();
@@ -636,7 +662,7 @@ CompiledMethod* OptimizingCompiler::TryCompile(const DexFile::CodeItem* code_ite
     return CompileBaseline(codegen.get(),
                            compiler_driver,
                            dex_compilation_unit,
-                           &pass_info_printer);
+                           &pass_observer);
   } else {
     return nullptr;
   }
index 0d687d2..17cfed4 100644 (file)
@@ -31,7 +31,7 @@ namespace art {
 class ReferenceTypePropagation : public HOptimization {
  public:
   ReferenceTypePropagation(HGraph* graph, StackHandleScopeCollection* handles)
-    : HOptimization(graph, true, kReferenceTypePropagationPassName),
+    : HOptimization(graph, kReferenceTypePropagationPassName),
       handles_(handles),
       worklist_(graph->GetArena(), kDefaultWorklistSize) {}
 
index 415d10c..7d300ad 100644 (file)
@@ -25,7 +25,7 @@ namespace art {
 class SideEffectsAnalysis : public HOptimization {
  public:
   explicit SideEffectsAnalysis(HGraph* graph)
-      : HOptimization(graph, true, kSideEffectsAnalysisPassName),
+      : HOptimization(graph, kSideEffectsAnalysisPassName),
         graph_(graph),
         block_effects_(graph->GetArena(), graph->GetBlocks().Size(), SideEffects::None()),
         loop_effects_(graph->GetArena(), graph->GetBlocks().Size(), SideEffects::None()) {}
index c4b63ab..67351f2 100644 (file)
@@ -29,7 +29,7 @@ namespace art {
 class SsaDeadPhiElimination : public HOptimization {
  public:
   explicit SsaDeadPhiElimination(HGraph* graph)
-      : HOptimization(graph, true, kSsaDeadPhiEliminationPassName),
+      : HOptimization(graph, kSsaDeadPhiEliminationPassName),
         worklist_(graph->GetArena(), kDefaultWorklistSize) {}
 
   void Run() OVERRIDE;
@@ -56,7 +56,7 @@ class SsaDeadPhiElimination : public HOptimization {
 class SsaRedundantPhiElimination : public HOptimization {
  public:
   explicit SsaRedundantPhiElimination(HGraph* graph)
-      : HOptimization(graph, true, kSsaRedundantPhiEliminationPassName),
+      : HOptimization(graph, kSsaRedundantPhiEliminationPassName),
         worklist_(graph->GetArena(), kDefaultWorklistSize) {}
 
   void Run() OVERRIDE;