Remove RTI validation from our checks

It is possible to have untyped RTI in valid graphs due to e.g.
dead code, redundant phis not replaced yet. By removing this
validation, we shift the burden of the validation to the user
of the RTI.

Bug: 252799892
Fixes: 252799892
Test: art/test/testrunner/testrunner.py --host --64 --optimizing -b
Test: dex2oat compiling the apps in the bug
Change-Id: I112497bf10c180b651fd5b6e268799bc3abb27b3

8 files changed, 32 insertions, 90 deletions

diff --git a/compiler/optimizing/graph_checker.cc b/compiler/optimizing/graph_checker.cc
index ca29a09440..8e6c64dbf0 100644
--- a/compiler/optimizing/graph_checker.cc
+++ b/compiler/optimizing/graph_checker.cc
@@ -453,6 +453,20 @@ void GraphChecker::VisitTryBoundary(HTryBoundary* try_boundary) {
   flag_info_.seen_try_boundary = true;
 }
 
+void GraphChecker::VisitLoadClass(HLoadClass* load) {
+  VisitInstruction(load);
+
+  if (load->GetLoadedClassRTI().IsValid() && !load->GetLoadedClassRTI().IsExact()) {
+    std::stringstream ssRTI;
+    ssRTI << load->GetLoadedClassRTI();
+    AddError(StringPrintf("%s:%d in block %d with RTI %s has valid but inexact RTI.",
+                          load->DebugName(),
+                          load->GetId(),
+                          load->GetBlock()->GetBlockId(),
+                          ssRTI.str().c_str()));
+  }
+}
+
 void GraphChecker::VisitLoadException(HLoadException* load) {
   VisitInstruction(load);
 
@@ -627,16 +641,6 @@ void GraphChecker::VisitInstruction(HInstruction* instruction) {
     }
   }
 
-  // Ensure that reference type instructions have reference type info.
-  if (check_reference_type_info_ && instruction->GetType() == DataType::Type::kReference) {
-    if (!instruction->GetReferenceTypeInfo().IsValid()) {
-      AddError(StringPrintf("Reference type instruction %s:%d does not have "
-                            "valid reference type information.",
-                            instruction->DebugName(),
-                            instruction->GetId()));
-    }
-  }
-
   if (instruction->CanThrow() && !instruction->HasEnvironment()) {
     AddError(StringPrintf("Throwing instruction %s:%d in block %d does not have an environment.",
                           instruction->DebugName(),
@@ -752,6 +756,17 @@ void GraphChecker::CheckTypeCheckBitstringInput(HTypeCheckInstruction* check,
 
 void GraphChecker::HandleTypeCheckInstruction(HTypeCheckInstruction* check) {
   VisitInstruction(check);
+
+  if (check->GetTargetClassRTI().IsValid() && !check->GetTargetClassRTI().IsExact()) {
+    std::stringstream ssRTI;
+    ssRTI << check->GetTargetClassRTI();
+    AddError(StringPrintf("%s:%d in block %d with RTI %s has valid but inexact RTI.",
+                          check->DebugName(),
+                          check->GetId(),
+                          check->GetBlock()->GetBlockId(),
+                          ssRTI.str().c_str()));
+  }
+
   HInstruction* input = check->InputAt(1);
   if (check->GetTypeCheckKind() == TypeCheckKind::kBitstringCheck) {
     if (!input->IsNullConstant()) {
diff --git a/compiler/optimizing/graph_checker.h b/compiler/optimizing/graph_checker.h
index 674798e20b..d6644f3b50 100644
--- a/compiler/optimizing/graph_checker.h
+++ b/compiler/optimizing/graph_checker.h
@@ -68,6 +68,7 @@ class GraphChecker : public HGraphDelegateVisitor {
   void VisitInstanceOf(HInstanceOf* check) override;
   void VisitInvoke(HInvoke* invoke) override;
   void VisitInvokeStaticOrDirect(HInvokeStaticOrDirect* invoke) override;
+  void VisitLoadClass(HLoadClass* load) override;
   void VisitLoadException(HLoadException* load) override;
   void VisitMonitorOperation(HMonitorOperation* monitor_operation) override;
   void VisitNeg(HNeg* instruction) override;
@@ -106,15 +107,6 @@ class GraphChecker : public HGraphDelegateVisitor {
     }
   }
 
-  // Enable/Disable the reference type info check.
-  //
-  // Return: the previous status of the check.
-  bool SetRefTypeInfoCheckEnabled(bool value = true) {
-    bool old_value = check_reference_type_info_;
-    check_reference_type_info_ = value;
-    return old_value;
-  }
-
  protected:
   // Report a new error.
   void AddError(const std::string& error) {
@@ -136,10 +128,6 @@ class GraphChecker : public HGraphDelegateVisitor {
   const char* const dump_prefix_;
   ScopedArenaAllocator allocator_;
   ArenaBitVector seen_ids_;
-  // Whether to perform the reference type info check for instructions which use or produce
-  // object references, e.g. HNewInstance, HLoadClass.
-  // The default value is true.
-  bool check_reference_type_info_ = true;
 
   // Used to access target information.
   CodeGenerator* codegen_;
diff --git a/compiler/optimizing/graph_visualizer.cc b/compiler/optimizing/graph_visualizer.cc
index 96eaa61209..6abe2cbd2d 100644
--- a/compiler/optimizing/graph_visualizer.cc
+++ b/compiler/optimizing/graph_visualizer.cc
@@ -764,15 +764,7 @@ class HGraphVisualizerPrinter : public HGraphDelegateVisitor {
                  instruction->IsCheckCast()) {
         StartAttributeStream("klass") << "unresolved";
       } else {
-        // The NullConstant may be added to the graph during other passes that happen between
-        // ReferenceTypePropagation and Inliner (e.g. InstructionSimplifier). If the inliner
-        // doesn't run or doesn't inline anything, the NullConstant remains untyped.
-        // So we should check NullConstants for validity only after reference type propagation.
-        DCHECK(graph_in_bad_state_ ||
-               IsDebugDump() ||
-               (!is_after_pass_ && IsPass(HGraphBuilder::kBuilderPassName)))
-            << instruction->DebugName() << instruction->GetId() << " has invalid rti "
-            << (is_after_pass_ ? "after" : "before") << " pass " << pass_name_;
+        StartAttributeStream("klass") << "invalid";
       }
     }
     if (disasm_info_ != nullptr) {
diff --git a/compiler/optimizing/load_store_analysis_test.cc b/compiler/optimizing/load_store_analysis_test.cc
index b6cf8b4abb..ee8069a907 100644
--- a/compiler/optimizing/load_store_analysis_test.cc
+++ b/compiler/optimizing/load_store_analysis_test.cc
@@ -144,7 +144,7 @@ TEST_F(LoadStoreAnalysisTest, ArrayHeapLocations) {
   ASSERT_TRUE(heap_location_collector.MayAlias(loc1, loc3));
   ASSERT_TRUE(heap_location_collector.MayAlias(loc1, loc3));
 
-  EXPECT_TRUE(CheckGraph(graph_));
+  EXPECT_TRUE(CheckGraph());
 }
 
 TEST_F(LoadStoreAnalysisTest, FieldHeapLocations) {
@@ -225,7 +225,7 @@ TEST_F(LoadStoreAnalysisTest, FieldHeapLocations) {
   // accesses to different fields of the same object should not alias.
   ASSERT_FALSE(heap_location_collector.MayAlias(loc1, loc2));
 
-  EXPECT_TRUE(CheckGraph(graph_));
+  EXPECT_TRUE(CheckGraph());
 }
 
 TEST_F(LoadStoreAnalysisTest, ArrayIndexAliasingTest) {
@@ -319,7 +319,7 @@ TEST_F(LoadStoreAnalysisTest, ArrayIndexAliasingTest) {
   loc2 = heap_location_collector.GetArrayHeapLocation(arr_set8);
   ASSERT_TRUE(heap_location_collector.MayAlias(loc1, loc2));
 
-  EXPECT_TRUE(CheckGraphSkipRefTypeInfoChecks(graph_));
+  EXPECT_TRUE(CheckGraph());
 }
 
 TEST_F(LoadStoreAnalysisTest, ArrayAliasingTest) {
diff --git a/compiler/optimizing/load_store_elimination_test.cc b/compiler/optimizing/load_store_elimination_test.cc
index f0c2541488..1ee109980f 100644
--- a/compiler/optimizing/load_store_elimination_test.cc
+++ b/compiler/optimizing/load_store_elimination_test.cc
@@ -73,7 +73,7 @@ class LoadStoreEliminationTestBase : public SuperTest, public OptimizingUnitTest
     LoadStoreElimination lse(graph_, /*stats=*/nullptr);
     lse.Run(with_partial);
     std::ostringstream oss;
-    EXPECT_TRUE(CheckGraphSkipRefTypeInfoChecks(oss)) << oss.str();
+    EXPECT_TRUE(CheckGraph(oss)) << oss.str();
   }
 
   void PerformLSEWithPartial(const AdjacencyListGraph& blks) {
diff --git a/compiler/optimizing/optimizing_unit_test.h b/compiler/optimizing/optimizing_unit_test.h
index e62ccf0f3a..2ebaf58d65 100644
--- a/compiler/optimizing/optimizing_unit_test.h
+++ b/compiler/optimizing/optimizing_unit_test.h
@@ -321,25 +321,10 @@ class OptimizingUnitTestHelper {
   // Run GraphChecker with all checks.
   //
   // Return: the status whether the run is successful.
-  bool CheckGraph(HGraph* graph, std::ostream& oss = std::cerr) {
-    return CheckGraph(graph, /*check_ref_type_info=*/true, oss);
-  }
-
   bool CheckGraph(std::ostream& oss = std::cerr) {
     return CheckGraph(graph_, oss);
   }
 
-  // Run GraphChecker with all checks except reference type information checks.
-  //
-  // Return: the status whether the run is successful.
-  bool CheckGraphSkipRefTypeInfoChecks(HGraph* graph, std::ostream& oss = std::cerr) {
-    return CheckGraph(graph, /*check_ref_type_info=*/false, oss);
-  }
-
-  bool CheckGraphSkipRefTypeInfoChecks(std::ostream& oss = std::cerr) {
-    return CheckGraphSkipRefTypeInfoChecks(graph_, oss);
-  }
-
   HEnvironment* ManuallyBuildEnvFor(HInstruction* instruction,
                                     ArenaVector<HInstruction*>* current_locals) {
     HEnvironment* environment = new (GetAllocator()) HEnvironment(
@@ -532,9 +517,8 @@ class OptimizingUnitTestHelper {
   }
 
  protected:
-  bool CheckGraph(HGraph* graph, bool check_ref_type_info, std::ostream& oss) {
+  bool CheckGraph(HGraph* graph, std::ostream& oss) {
     GraphChecker checker(graph);
-    checker.SetRefTypeInfoCheckEnabled(check_ref_type_info);
     checker.Run();
     checker.Dump(oss);
     return checker.IsValid();
diff --git a/compiler/optimizing/reference_type_propagation.cc b/compiler/optimizing/reference_type_propagation.cc
index 48d33f2b8c..74e295e70f 100644
--- a/compiler/optimizing/reference_type_propagation.cc
+++ b/compiler/optimizing/reference_type_propagation.cc
@@ -122,40 +122,6 @@ ReferenceTypePropagation::ReferenceTypePropagation(HGraph* graph,
                                                    const char* name)
     : HOptimization(graph, name), hint_dex_cache_(hint_dex_cache), is_first_run_(is_first_run) {}
 
-void ReferenceTypePropagation::ValidateTypes() {
-  // TODO: move this to the graph checker.
-  if (kIsDebugBuild) {
-    ScopedObjectAccess soa(Thread::Current());
-    for (HBasicBlock* block : graph_->GetReversePostOrder()) {
-      for (HInstructionIterator iti(block->GetInstructions()); !iti.Done(); iti.Advance()) {
-        HInstruction* instr = iti.Current();
-        if (instr->GetType() == DataType::Type::kReference) {
-          DCHECK(instr->GetReferenceTypeInfo().IsValid())
-              << "Invalid RTI for instruction: " << instr->DebugName();
-          if (instr->IsBoundType()) {
-            DCHECK(instr->AsBoundType()->GetUpperBound().IsValid());
-          } else if (instr->IsLoadClass()) {
-            HLoadClass* cls = instr->AsLoadClass();
-            DCHECK(cls->GetReferenceTypeInfo().IsExact());
-            DCHECK_IMPLIES(cls->GetLoadedClassRTI().IsValid(), cls->GetLoadedClassRTI().IsExact());
-          } else if (instr->IsNullCheck()) {
-            DCHECK(instr->GetReferenceTypeInfo().IsEqual(instr->InputAt(0)->GetReferenceTypeInfo()))
-                << "NullCheck " << instr->GetReferenceTypeInfo()
-                << "Input(0) " << instr->InputAt(0)->GetReferenceTypeInfo();
-          }
-        } else if (instr->IsInstanceOf()) {
-          HInstanceOf* iof = instr->AsInstanceOf();
-          DCHECK_IMPLIES(iof->GetTargetClassRTI().IsValid(), iof->GetTargetClassRTI().IsExact());
-        } else if (instr->IsCheckCast()) {
-          HCheckCast* check = instr->AsCheckCast();
-          DCHECK_IMPLIES(check->GetTargetClassRTI().IsValid(),
-                         check->GetTargetClassRTI().IsExact());
-        }
-      }
-    }
-  }
-}
-
 void ReferenceTypePropagation::Visit(HInstruction* instruction) {
   RTPVisitor visitor(graph_, hint_dex_cache_, is_first_run_);
   instruction->Accept(&visitor);
@@ -346,7 +312,6 @@ bool ReferenceTypePropagation::Run() {
   }
 
   visitor.ProcessWorklist();
-  ValidateTypes();
   return true;
 }
 
diff --git a/compiler/optimizing/reference_type_propagation.h b/compiler/optimizing/reference_type_propagation.h
index d696e28bc2..655f62b3da 100644
--- a/compiler/optimizing/reference_type_propagation.h
+++ b/compiler/optimizing/reference_type_propagation.h
@@ -71,8 +71,6 @@ class ReferenceTypePropagation : public HOptimization {
                                       HandleCache* handle_cache)
       REQUIRES_SHARED(Locks::mutator_lock_);
 
-  void ValidateTypes();
-
   // Note: hint_dex_cache_ is usually, but not necessarily, the dex cache associated with
   // graph_->GetDexFile(). Since we may look up also in other dex files, it's used only
   // as a hint, to reduce the number of calls to the costly ClassLinker::FindDexCache().