commit 1aaa9d4824f34ad8a2c70ab7c93598ef85e7443a
author Santiago Aboy Solanes <solanes@google.com> Fri Dec 16 12:04:22 2022 +0000
committer Treehugger Robot <treehugger-gerrit@google.com> Mon Dec 19 08:42:38 2022 +0000
tree 6be7e830e9c33182f3c09ce26e66df18165ea375
parent 419484b8d944122049f2517bf9a2e482d26575b5

Graph checker fixups

* Add missing VisitInstrucion in VisitLoadException.
* Add commented VisitInstrucion in VisitConstant. See bug for details.
* For consistency, moved all the VisitInstruction to the beginning of
  the methods.
* Add a check in VisitTryBoundary to check for the HasTryCatch flag.

Bug: 262862764
Test: art/test/testrunner/testrunner.py --host --64 --optimizing -b
Change-Id: I26f06dabea44671f6bce838c60083e1bb0babef4

compiler/optimizing/graph_checker.cc

diff --git a/compiler/optimizing/graph_checker.cc b/compiler/optimizing/graph_checker.cc
index ec89022..32b3bc1 100644
--- a/compiler/optimizing/graph_checker.cc
+++ b/compiler/optimizing/graph_checker.cc
@@ -309,27 +309,25 @@
 }
 
 void GraphChecker::VisitBoundsCheck(HBoundsCheck* check) {
+  VisitInstruction(check);
   if (!GetGraph()->HasBoundsChecks()) {
     AddError(StringPrintf("Instruction %s:%d is a HBoundsCheck, "
                           "but HasBoundsChecks() returns false",
                           check->DebugName(),
                           check->GetId()));
   }
-
-  // Perform the instruction base checks too.
-  VisitInstruction(check);
 }
 
 void GraphChecker::VisitDeoptimize(HDeoptimize* deopt) {
+  VisitInstruction(deopt);
   if (GetGraph()->IsCompilingOsr()) {
     AddError(StringPrintf("A graph compiled OSR cannot have a HDeoptimize instruction"));
   }
-
-  // Perform the instruction base checks too.
-  VisitInstruction(deopt);
 }
 
 void GraphChecker::VisitTryBoundary(HTryBoundary* try_boundary) {
+  VisitInstruction(try_boundary);
+
   ArrayRef<HBasicBlock* const> handlers = try_boundary->GetExceptionHandlers();
 
   // Ensure that all exception handlers are catch blocks.
@@ -356,10 +354,19 @@
     }
   }
 
-  VisitInstruction(try_boundary);
+  if (!GetGraph()->HasTryCatch()) {
+    AddError(
+        StringPrintf("The graph doesn't have the HasTryCatch bit set but we saw "
+                     "%s:%d in block %d.",
+                     try_boundary->DebugName(),
+                     try_boundary->GetId(),
+                     try_boundary->GetBlock()->GetBlockId()));
+  }
 }
 
 void GraphChecker::VisitLoadException(HLoadException* load) {
+  VisitInstruction(load);
+
   // Ensure that LoadException is the second instruction in a catch block. The first one should be a
   // Nop (checked separately).
   if (!load->GetBlock()->IsCatchBlock()) {
@@ -376,6 +383,8 @@
 }
 
 void GraphChecker::VisitMonitorOperation(HMonitorOperation* monitor_op) {
+  VisitInstruction(monitor_op);
+
   if (!GetGraph()->HasMonitorOperations()) {
     AddError(
         StringPrintf("The graph doesn't have the HasMonitorOperations bit set but we saw "
@@ -384,8 +393,6 @@
                      monitor_op->GetId(),
                      monitor_op->GetBlock()->GetBlockId()));
   }
-
-  VisitInstruction(monitor_op);
 }
 
 void GraphChecker::VisitInstruction(HInstruction* instruction) {
@@ -1152,6 +1159,10 @@
 }
 
 void GraphChecker::VisitConstant(HConstant* instruction) {
+  // TODO(solanes, 262862764): We should call VisitInstruction here but doing so makes the
+  // LoadStoreEliminationTest gtests fail.
+  // VisitInstruction(instruction);
+
   HBasicBlock* block = instruction->GetBlock();
   if (!block->IsEntryBlock()) {
     AddError(StringPrintf(