Remove the checkcast quickening optimization.

Quickening is now disabled, and the optimization uses VerifierDeps
post-verification, which we plan on not supporting for future verifier
improvements.

Test: test.py
Bug: 112676029
Change-Id: Ie9004b27c93e1189e6c1142494e79cd84b05400c

10 files changed, 1 insertions, 195 deletions

diff --git a/compiler/dex/verification_results.cc b/compiler/dex/verification_results.cc
index e7a3817c56..6241ff3c6c 100644
--- a/compiler/dex/verification_results.cc
+++ b/compiler/dex/verification_results.cc
@@ -82,14 +82,6 @@ void VerificationResults::ProcessVerifiedMethod(verifier::MethodVerifier* method
   } else {
     // TODO: Investigate why are we doing the work again for this method and try to avoid it.
     LOG(WARNING) << "Method processed more than once: " << ref.PrettyMethod();
-    if (!Runtime::Current()->UseJitCompilation()) {
-      if (kIsDebugBuild) {
-        auto ex_set = existing->GetSafeCastSet();
-        auto ve_set = verified_method->GetSafeCastSet();
-        CHECK_EQ(ex_set == nullptr, ve_set == nullptr);
-        CHECK((ex_set == nullptr) || (ex_set->size() == ve_set->size()));
-      }
-    }
     // Let the unique_ptr delete the new verified method since there was already an existing one
     // registered. It is unsafe to replace the existing one since the JIT may be using it to
     // generate a native GC map.
diff --git a/compiler/dex/verified_method.cc b/compiler/dex/verified_method.cc
index d7dc310230..04f7215015 100644
--- a/compiler/dex/verified_method.cc
+++ b/compiler/dex/verified_method.cc
@@ -43,67 +43,7 @@ const VerifiedMethod* VerifiedMethod::Create(verifier::MethodVerifier* method_ve
       new VerifiedMethod(method_verifier->GetEncounteredFailureTypes(),
                          method_verifier->HasInstructionThatWillThrow()));
 
-  if (method_verifier->HasCheckCasts()) {
-    verified_method->GenerateSafeCastSet(method_verifier);
-  }
-
   return verified_method.release();
 }
 
-bool VerifiedMethod::IsSafeCast(uint32_t pc) const {
-  if (safe_cast_set_ == nullptr) {
-    return false;
-  }
-  return std::binary_search(safe_cast_set_->begin(), safe_cast_set_->end(), pc);
-}
-
-void VerifiedMethod::GenerateSafeCastSet(verifier::MethodVerifier* method_verifier) {
-  /*
-   * Walks over the method code and adds any cast instructions in which
-   * the type cast is implicit to a set, which is used in the code generation
-   * to elide these casts.
-   */
-  if (method_verifier->HasFailures()) {
-    return;
-  }
-  for (const DexInstructionPcPair& pair : method_verifier->CodeItem()) {
-    const Instruction& inst = pair.Inst();
-    const Instruction::Code code = inst.Opcode();
-    if (code == Instruction::CHECK_CAST) {
-      const uint32_t dex_pc = pair.DexPc();
-      if (!method_verifier->GetInstructionFlags(dex_pc).IsVisited()) {
-        // Do not attempt to quicken this instruction, it's unreachable anyway.
-        continue;
-      }
-      const verifier::RegisterLine* line = method_verifier->GetRegLine(dex_pc);
-      DCHECK(line != nullptr) << "Did not have line for dex pc 0x" << std::hex << dex_pc;
-      const verifier::RegType& reg_type(line->GetRegisterType(method_verifier,
-                                                              inst.VRegA_21c()));
-      const verifier::RegType& cast_type =
-          method_verifier->ResolveCheckedClass(dex::TypeIndex(inst.VRegB_21c()));
-      // Pass null for the method verifier to not record the VerifierDeps dependency
-      // if the types are not assignable.
-      if (cast_type.IsStrictlyAssignableFrom(reg_type, /* verifier= */ nullptr)) {
-        // The types are assignable, we record that dependency in the VerifierDeps so
-        // that if this changes after OTA, we will re-verify again.
-        // We check if reg_type has a class, as the verifier may have inferred it's
-        // 'null'.
-        if (reg_type.HasClass()) {
-          DCHECK(cast_type.HasClass());
-          verifier::VerifierDeps::MaybeRecordAssignability(method_verifier->GetDexFile(),
-                                                           cast_type.GetClass(),
-                                                           reg_type.GetClass());
-        }
-        if (safe_cast_set_ == nullptr) {
-          safe_cast_set_.reset(new SafeCastSet());
-        }
-        // Verify ordering for push_back() to the sorted vector.
-        DCHECK(safe_cast_set_->empty() || safe_cast_set_->back() < dex_pc);
-        safe_cast_set_->push_back(dex_pc);
-      }
-    }
-  }
-  DCHECK(safe_cast_set_ == nullptr || !safe_cast_set_->empty());
-}
-
 }  // namespace art
diff --git a/compiler/dex/verified_method.h b/compiler/dex/verified_method.h
index f04392d44e..f100f78fb0 100644
--- a/compiler/dex/verified_method.h
+++ b/compiler/dex/verified_method.h
@@ -34,23 +34,10 @@ class VerifiedMethod {
  public:
   VerifiedMethod(uint32_t encountered_error_types, bool has_runtime_throw);
 
-  // Cast elision set type.
-  // Since we're adding the dex PCs to the set in increasing order, a sorted vector
-  // is better for performance (not just memory usage), especially for large sets.
-  typedef std::vector<uint32_t> SafeCastSet;
-
   static const VerifiedMethod* Create(verifier::MethodVerifier* method_verifier)
       REQUIRES_SHARED(Locks::mutator_lock_);
   ~VerifiedMethod() = default;
 
-  const SafeCastSet* GetSafeCastSet() const {
-    return safe_cast_set_.get();
-  }
-
-  // Returns true if the cast can statically be verified to be redundant
-  // by using the check-cast elision peephole optimization in the verifier.
-  bool IsSafeCast(uint32_t pc) const;
-
   // Returns true if there were any errors during verification.
   bool HasVerificationFailures() const {
     return encountered_error_types_ != 0;
@@ -65,12 +52,6 @@ class VerifiedMethod {
   }
 
  private:
-  // Generate safe case set into safe_cast_set_.
-  void GenerateSafeCastSet(verifier::MethodVerifier* method_verifier)
-      REQUIRES_SHARED(Locks::mutator_lock_);
-
-  std::unique_ptr<SafeCastSet> safe_cast_set_;
-
   const uint32_t encountered_error_types_;
   const bool has_runtime_throw_;
 };
diff --git a/dex2oat/dex/dex_to_dex_compiler.cc b/dex2oat/dex/dex_to_dex_compiler.cc
index caecf70615..384436d5e9 100644
--- a/dex2oat/dex/dex_to_dex_compiler.cc
+++ b/dex2oat/dex/dex_to_dex_compiler.cc
@@ -44,8 +44,6 @@ using android::base::StringPrintf;
 
 // Controls quickening activation.
 const bool kEnableQuickening = true;
-// Control check-cast elision.
-const bool kEnableCheckCastEllision = true;
 
 // Holds the state for compiling a single method.
 struct DexToDexCompiler::CompilationState {
@@ -76,11 +74,6 @@ struct DexToDexCompiler::CompilationState {
   // a barrier is required.
   void CompileReturnVoid(Instruction* inst, uint32_t dex_pc);
 
-  // Compiles a CHECK-CAST into 2 NOP instructions if it is known to be safe. In
-  // this case, returns the second NOP instruction pointer. Otherwise, returns
-  // the given "inst".
-  Instruction* CompileCheckCast(Instruction* inst, uint32_t dex_pc);
-
   // Compiles a field access into a quick field access.
   // The field index is replaced by an offset within an Object where we can read
   // from / write to this field. Therefore, this does not involve any resolution
@@ -224,15 +217,6 @@ std::vector<uint8_t> DexToDexCompiler::CompilationState::Compile() {
         CompileReturnVoid(inst, dex_pc);
         break;
 
-      case Instruction::CHECK_CAST:
-        inst = CompileCheckCast(inst, dex_pc);
-        if (inst->Opcode() == Instruction::NOP) {
-          // We turned the CHECK_CAST into two NOPs, avoid visiting the second NOP twice since this
-          // would add 2 quickening info entries.
-          ++it;
-        }
-        break;
-
       case Instruction::IGET:
       case Instruction::IGET_QUICK:
         CompileInstanceFieldAccess(inst, dex_pc, Instruction::IGET_QUICK, false);
@@ -313,26 +297,6 @@ std::vector<uint8_t> DexToDexCompiler::CompilationState::Compile() {
         CompileInvokeVirtual(inst, dex_pc, Instruction::INVOKE_VIRTUAL_RANGE_QUICK, true);
         break;
 
-      case Instruction::NOP:
-        if (already_quickened_) {
-          const uint16_t reference_index = NextIndex();
-          quickened_info_.push_back(QuickenedInfo(dex_pc, reference_index));
-          if (reference_index == DexFile::kDexNoIndex16) {
-            // This means it was a normal nop and not a check-cast.
-            break;
-          }
-          const uint16_t type_index = NextIndex();
-          if (driver_.IsSafeCast(&unit_, dex_pc)) {
-            quickened_info_.push_back(QuickenedInfo(dex_pc, type_index));
-          }
-          ++it;
-        } else {
-          // We need to differentiate between check cast inserted NOP and normal NOP, put an invalid
-          // index in the map for normal nops. This should be rare in real code.
-          quickened_info_.push_back(QuickenedInfo(dex_pc, DexFile::kDexNoIndex16));
-        }
-        break;
-
       default:
         // Nothing to do.
         break;
@@ -389,40 +353,6 @@ void DexToDexCompiler::CompilationState::CompileReturnVoid(Instruction* inst, ui
   optimized_return_void_ = true;
 }
 
-Instruction* DexToDexCompiler::CompilationState::CompileCheckCast(Instruction* inst,
-                                                                  uint32_t dex_pc) {
-  if (!kEnableCheckCastEllision) {
-    return inst;
-  }
-  if (!driver_.IsSafeCast(&unit_, dex_pc)) {
-    return inst;
-  }
-  // Ok, this is a safe cast. Since the "check-cast" instruction size is 2 code
-  // units and a "nop" instruction size is 1 code unit, we need to replace it by
-  // 2 consecutive NOP instructions.
-  // Because the caller loops over instructions by calling Instruction::Next onto
-  // the current instruction, we need to return the 2nd NOP instruction. Indeed,
-  // its next instruction is the former check-cast's next instruction.
-  VLOG(compiler) << "Removing " << Instruction::Name(inst->Opcode())
-                 << " by replacing it with 2 NOPs at dex pc "
-                 << StringPrintf("0x%x", dex_pc) << " in method "
-                 << GetDexFile().PrettyMethod(unit_.GetDexMethodIndex(), true);
-  if (!already_quickened_) {
-    quickened_info_.push_back(QuickenedInfo(dex_pc, inst->VRegA_21c()));
-    quickened_info_.push_back(QuickenedInfo(dex_pc, inst->VRegB_21c()));
-
-    // We are modifying 4 consecutive bytes.
-    inst->SetOpcode(Instruction::NOP);
-    inst->SetVRegA_10x(0u);  // keep compliant with verifier.
-    // Get to next instruction which is the second half of check-cast and replace
-    // it by a NOP.
-    inst = const_cast<Instruction*>(inst->Next());
-    inst->SetOpcode(Instruction::NOP);
-    inst->SetVRegA_10x(0u);  // keep compliant with verifier.
-  }
-  return inst;
-}
-
 void DexToDexCompiler::CompilationState::CompileInstanceFieldAccess(Instruction* inst,
                                                                     uint32_t dex_pc,
                                                                     Instruction::Code new_opcode,
diff --git a/dex2oat/driver/compiler_driver.cc b/dex2oat/driver/compiler_driver.cc
index e6a2554f10..515e8b045f 100644
--- a/dex2oat/driver/compiler_driver.cc
+++ b/dex2oat/driver/compiler_driver.cc
@@ -1483,21 +1483,6 @@ bool CompilerDriver::ComputeInstanceFieldInfo(uint32_t field_idx, const DexCompi
   }
 }
 
-bool CompilerDriver::IsSafeCast(const DexCompilationUnit* mUnit, uint32_t dex_pc) {
-  if (!compiler_options_->IsVerificationEnabled()) {
-    // If we didn't verify, every cast has to be treated as non-safe.
-    return false;
-  }
-  DCHECK(mUnit->GetVerifiedMethod() != nullptr);
-  bool result = mUnit->GetVerifiedMethod()->IsSafeCast(dex_pc);
-  if (result) {
-    stats_->SafeCast();
-  } else {
-    stats_->NotASafeCast();
-  }
-  return result;
-}
-
 class CompilationVisitor {
  public:
   virtual ~CompilationVisitor() {}
diff --git a/dex2oat/driver/compiler_driver.h b/dex2oat/driver/compiler_driver.h
index 4f2cb810b7..e869baec73 100644
--- a/dex2oat/driver/compiler_driver.h
+++ b/dex2oat/driver/compiler_driver.h
@@ -183,8 +183,6 @@ class CompilerDriver {
       REQUIRES_SHARED(Locks::mutator_lock_);
 
 
-  bool IsSafeCast(const DexCompilationUnit* mUnit, uint32_t dex_pc);
-
   size_t GetThreadCount() const {
     return parallel_thread_count_;
   }
diff --git a/runtime/quicken_info.h b/runtime/quicken_info.h
index 6c18590598..83dc5f1df8 100644
--- a/runtime/quicken_info.h
+++ b/runtime/quicken_info.h
@@ -57,7 +57,7 @@ class QuickenInfoTable {
 
   // Returns true if the dex instruction has an index in the table. (maybe dequickenable).
   static bool NeedsIndexForInstruction(const Instruction* inst) {
-    return inst->IsQuickened() || inst->Opcode() == Instruction::NOP;
+    return inst->IsQuickened();
   }
 
   static size_t NumberOfIndices(size_t bytes) {
diff --git a/runtime/verifier/method_verifier-inl.h b/runtime/verifier/method_verifier-inl.h
index dd91a85177..a9fb1a0773 100644
--- a/runtime/verifier/method_verifier-inl.h
+++ b/runtime/verifier/method_verifier-inl.h
@@ -34,10 +34,6 @@ inline MethodReference MethodVerifier::GetMethodReference() const {
   return MethodReference(dex_file_, dex_method_idx_);
 }
 
-inline bool MethodVerifier::HasCheckCasts() const {
-  return has_check_casts_;
-}
-
 inline bool MethodVerifier::HasFailures() const {
   return !failure_messages_.empty();
 }
diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc
index fea34ef34c..f726db7b2f 100644
--- a/runtime/verifier/method_verifier.cc
+++ b/runtime/verifier/method_verifier.cc
@@ -1079,15 +1079,6 @@ bool MethodVerifier<kVerifierDebug>::ComputeWidthsAndCountOps() {
     if (next.IsErrorState()) {
       break;
     }
-    Instruction::Code opcode = it->Opcode();
-    switch (opcode) {
-      case Instruction::APUT_OBJECT:
-      case Instruction::CHECK_CAST:
-        has_check_casts_ = true;
-        break;
-      default:
-        break;
-    }
     GetModifiableInstructionFlags(it.DexPc()).SetIsOpcode();
   }
 
@@ -5071,7 +5062,6 @@ MethodVerifier::MethodVerifier(Thread* self,
       encountered_failure_types_(0),
       can_load_classes_(can_load_classes),
       allow_soft_failures_(allow_soft_failures),
-      has_check_casts_(false),
       class_linker_(class_linker),
       link_(nullptr) {
   self->PushVerifier(this);
diff --git a/runtime/verifier/method_verifier.h b/runtime/verifier/method_verifier.h
index 83dafd3975..51325f0c21 100644
--- a/runtime/verifier/method_verifier.h
+++ b/runtime/verifier/method_verifier.h
@@ -191,7 +191,6 @@ class MethodVerifier {
   ALWAYS_INLINE const InstructionFlags& GetInstructionFlags(size_t index) const;
 
   MethodReference GetMethodReference() const;
-  bool HasCheckCasts() const;
   bool HasFailures() const;
   bool HasInstructionThatWillThrow() const {
     return flags_.have_any_pending_runtime_throw_failure_;
@@ -378,11 +377,6 @@ class MethodVerifier {
   // running and the verifier is called from the class linker.
   const bool allow_soft_failures_;
 
-  // Indicates the method being verified contains at least one check-cast or aput-object
-  // instruction. Aput-object operations implicitly check for array-store exceptions, similar to
-  // check-cast.
-  bool has_check_casts_;
-
   // Classlinker to use when resolving.
   ClassLinker* class_linker_;