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
diff --git a/compiler/dex/verification_results.cc b/compiler/dex/verification_results.cc
index e7a3817..6241ff3 100644
--- a/compiler/dex/verification_results.cc
+++ b/compiler/dex/verification_results.cc
@@ -82,14 +82,6 @@
} 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 d7dc310..04f7215 100644
--- a/compiler/dex/verified_method.cc
+++ b/compiler/dex/verified_method.cc
@@ -43,67 +43,7 @@
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 f04392d..f100f78 100644
--- a/compiler/dex/verified_method.h
+++ b/compiler/dex/verified_method.h
@@ -34,23 +34,10 @@
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 @@
}
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 caecf70..384436d 100644
--- a/dex2oat/dex/dex_to_dex_compiler.cc
+++ b/dex2oat/dex/dex_to_dex_compiler.cc
@@ -44,8 +44,6 @@
// 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 @@
// 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 @@
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 @@
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 @@
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 e6a2554..515e8b0 100644
--- a/dex2oat/driver/compiler_driver.cc
+++ b/dex2oat/driver/compiler_driver.cc
@@ -1483,21 +1483,6 @@
}
}
-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 4f2cb81..e869bae 100644
--- a/dex2oat/driver/compiler_driver.h
+++ b/dex2oat/driver/compiler_driver.h
@@ -183,8 +183,6 @@
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 6c18590..83dc5f1 100644
--- a/runtime/quicken_info.h
+++ b/runtime/quicken_info.h
@@ -57,7 +57,7 @@
// 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 dd91a85..a9fb1a0 100644
--- a/runtime/verifier/method_verifier-inl.h
+++ b/runtime/verifier/method_verifier-inl.h
@@ -34,10 +34,6 @@
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 fea34ef..f726db7 100644
--- a/runtime/verifier/method_verifier.cc
+++ b/runtime/verifier/method_verifier.cc
@@ -1079,15 +1079,6 @@
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 @@
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 83dafd3..51325f0 100644
--- a/runtime/verifier/method_verifier.h
+++ b/runtime/verifier/method_verifier.h
@@ -191,7 +191,6 @@
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 @@
// 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_;