ART: Run GraphChecker after Builder and SsaBuilder

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
diff --git a/compiler/optimizing/boolean_simplifier.h b/compiler/optimizing/boolean_simplifier.h
index 733ebaa..e12a12c 100644
--- a/compiler/optimizing/boolean_simplifier.h
+++ b/compiler/optimizing/boolean_simplifier.h
@@ -63,7 +63,7 @@
 class HBooleanSimplifier : public HOptimization {
  public:
   explicit HBooleanSimplifier(HGraph* graph)
-    : HOptimization(graph, true, kBooleanSimplifierPassName) {}
+    : HOptimization(graph, kBooleanSimplifierPassName) {}
 
   void Run() OVERRIDE;
 
diff --git a/compiler/optimizing/bounds_check_elimination.h b/compiler/optimizing/bounds_check_elimination.h
index 9e98ccf..24b8ea7 100644
--- a/compiler/optimizing/bounds_check_elimination.h
+++ b/compiler/optimizing/bounds_check_elimination.h
@@ -24,7 +24,7 @@
 class BoundsCheckElimination : public HOptimization {
  public:
   explicit BoundsCheckElimination(HGraph* graph)
-      : HOptimization(graph, true, kBoundsCheckEliminiationPassName) {}
+      : HOptimization(graph, kBoundsCheckEliminiationPassName) {}
 
   void Run() OVERRIDE;
 
diff --git a/compiler/optimizing/constant_folding.h b/compiler/optimizing/constant_folding.h
index 66ff578..2698b2d 100644
--- a/compiler/optimizing/constant_folding.h
+++ b/compiler/optimizing/constant_folding.h
@@ -33,7 +33,7 @@
 class HConstantFolding : public HOptimization {
  public:
   explicit HConstantFolding(HGraph* graph, const char* name = kConstantFoldingPassName)
-      : HOptimization(graph, true, name) {}
+      : HOptimization(graph, name) {}
 
   void Run() OVERRIDE;
 
diff --git a/compiler/optimizing/dead_code_elimination.h b/compiler/optimizing/dead_code_elimination.h
index 59a57c4..8d6008b 100644
--- a/compiler/optimizing/dead_code_elimination.h
+++ b/compiler/optimizing/dead_code_elimination.h
@@ -32,7 +32,7 @@
   HDeadCodeElimination(HGraph* graph,
                        OptimizingCompilerStats* stats = nullptr,
                        const char* name = kInitialDeadCodeEliminationPassName)
-      : HOptimization(graph, true, name, stats) {}
+      : HOptimization(graph, name, stats) {}
 
   void Run() OVERRIDE;
 
diff --git a/compiler/optimizing/gvn.h b/compiler/optimizing/gvn.h
index e74d969..14a503b 100644
--- a/compiler/optimizing/gvn.h
+++ b/compiler/optimizing/gvn.h
@@ -27,7 +27,7 @@
 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;
 
diff --git a/compiler/optimizing/inliner.h b/compiler/optimizing/inliner.h
index 24044b7..ffd7569 100644
--- a/compiler/optimizing/inliner.h
+++ b/compiler/optimizing/inliner.h
@@ -37,7 +37,7 @@
            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),
diff --git a/compiler/optimizing/instruction_simplifier.h b/compiler/optimizing/instruction_simplifier.h
index 668956a..faee2dd 100644
--- a/compiler/optimizing/instruction_simplifier.h
+++ b/compiler/optimizing/instruction_simplifier.h
@@ -31,7 +31,7 @@
   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";
 
diff --git a/compiler/optimizing/intrinsics.h b/compiler/optimizing/intrinsics.h
index 741fb64..9044982 100644
--- a/compiler/optimizing/intrinsics.h
+++ b/compiler/optimizing/intrinsics.h
@@ -31,7 +31,7 @@
 class IntrinsicsRecognizer : public HOptimization {
  public:
   IntrinsicsRecognizer(HGraph* graph, CompilerDriver* driver)
-      : HOptimization(graph, true, kIntrinsicsRecognizerPassName),
+      : HOptimization(graph, kIntrinsicsRecognizerPassName),
         driver_(driver) {}
 
   void Run() OVERRIDE;
diff --git a/compiler/optimizing/licm.h b/compiler/optimizing/licm.h
index cb6170e..0b5a0f1 100644
--- a/compiler/optimizing/licm.h
+++ b/compiler/optimizing/licm.h
@@ -27,7 +27,7 @@
 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;
 
diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h
index 581645f..0f5b1ab 100644
--- a/compiler/optimizing/nodes.h
+++ b/compiler/optimizing/nodes.h
@@ -160,6 +160,8 @@
   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; }
diff --git a/compiler/optimizing/optimization.cc b/compiler/optimizing/optimization.cc
index c46a219..3d76949 100644
--- a/compiler/optimizing/optimization.cc
+++ b/compiler/optimizing/optimization.cc
@@ -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::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
diff --git a/compiler/optimizing/optimization.h b/compiler/optimizing/optimization.h
index 2d1c0ba..bc56546 100644
--- a/compiler/optimizing/optimization.h
+++ b/compiler/optimizing/optimization.h
@@ -29,12 +29,10 @@
 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 @@
   // 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 @@
   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_;
 
diff --git a/compiler/optimizing/optimizing_compiler.cc b/compiler/optimizing/optimizing_compiler.cc
index 0c7b6f7..1466366 100644
--- a/compiler/optimizing/optimizing_compiler.cc
+++ b/compiler/optimizing/optimizing_compiler.cc
@@ -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 @@
  */
 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 @@
     }
   }
 
-  ~PassInfoPrinter() {
+  ~PassObserver() {
     if (timing_logger_enabled_) {
       LOG(INFO) << "TIMINGS " << method_name_;
       LOG(INFO) << Dumpable<TimingLogger>(timing_logger_);
@@ -123,6 +126,8 @@
     }
   }
 
+  void SetGraphInBadState() { graph_in_bad_state_ = true; }
+
  private:
   void StartPass(const char* pass_name) {
     // Dump graph first, then start timer.
@@ -142,8 +147,28 @@
     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 @@
   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 @@
                                    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 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 @@
                              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 @@
     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 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 @@
                                                      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 @@
       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 @@
     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 @@
       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 @@
   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 @@
   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 @@
     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 @@
                             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 @@
     return CompileBaseline(codegen.get(),
                            compiler_driver,
                            dex_compilation_unit,
-                           &pass_info_printer);
+                           &pass_observer);
   } else {
     return nullptr;
   }
diff --git a/compiler/optimizing/reference_type_propagation.h b/compiler/optimizing/reference_type_propagation.h
index 0d687d2..17cfed4 100644
--- a/compiler/optimizing/reference_type_propagation.h
+++ b/compiler/optimizing/reference_type_propagation.h
@@ -31,7 +31,7 @@
 class ReferenceTypePropagation : public HOptimization {
  public:
   ReferenceTypePropagation(HGraph* graph, StackHandleScopeCollection* handles)
-    : HOptimization(graph, true, kReferenceTypePropagationPassName),
+    : HOptimization(graph, kReferenceTypePropagationPassName),
       handles_(handles),
       worklist_(graph->GetArena(), kDefaultWorklistSize) {}
 
diff --git a/compiler/optimizing/side_effects_analysis.h b/compiler/optimizing/side_effects_analysis.h
index 415d10c..7d300ad 100644
--- a/compiler/optimizing/side_effects_analysis.h
+++ b/compiler/optimizing/side_effects_analysis.h
@@ -25,7 +25,7 @@
 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()) {}
diff --git a/compiler/optimizing/ssa_phi_elimination.h b/compiler/optimizing/ssa_phi_elimination.h
index c4b63ab..67351f2 100644
--- a/compiler/optimizing/ssa_phi_elimination.h
+++ b/compiler/optimizing/ssa_phi_elimination.h
@@ -29,7 +29,7 @@
 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 SsaRedundantPhiElimination : public HOptimization {
  public:
   explicit SsaRedundantPhiElimination(HGraph* graph)
-      : HOptimization(graph, true, kSsaRedundantPhiEliminationPassName),
+      : HOptimization(graph, kSsaRedundantPhiEliminationPassName),
         worklist_(graph->GetArena(), kDefaultWorklistSize) {}
 
   void Run() OVERRIDE;