Skip to content

Commit

Permalink
Reland "Support FastVerify with speed-profile."
Browse files Browse the repository at this point in the history
This reverts commit 4297f22.

Bug: 279728780

Reason for revert: Resolve classes in inliner.

Change-Id: I4f93ac5d195eb2f473ec50fe7cc70881dcddee6f
  • Loading branch information
Nicolas Geoffray committed May 4, 2023
1 parent 5c909cb commit be726b5
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 18 deletions.
4 changes: 2 additions & 2 deletions compiler/jni/quick/jni_compiler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -154,11 +154,11 @@ static JniCompiledMethod ArtJniCompileMethodInternal(const CompilerOptions& comp
// -- Don't allow any objects as parameter or return value
if (UNLIKELY(is_critical_native)) {
CHECK(is_static)
<< "@CriticalNative functions cannot be virtual since that would"
<< "@CriticalNative functions cannot be virtual since that would "
<< "require passing a reference parameter (this), which is illegal "
<< dex_file.PrettyMethod(method_idx, /* with_signature= */ true);
CHECK(!is_synchronized)
<< "@CriticalNative functions cannot be synchronized since that would"
<< "@CriticalNative functions cannot be synchronized since that would "
<< "require passing a (class and/or this) reference parameter, which is illegal "
<< dex_file.PrettyMethod(method_idx, /* with_signature= */ true);
for (size_t i = 0; i < strlen(shorty); ++i) {
Expand Down
6 changes: 4 additions & 2 deletions compiler/optimizing/inliner.cc
Original file line number Diff line number Diff line change
Expand Up @@ -702,12 +702,14 @@ HInliner::InlineCacheType HInliner::GetInlineCacheAOT(
// Walk over the class descriptors and look up the actual classes.
// If we cannot find a type we return kInlineCacheMissingTypes.
ClassLinker* class_linker = caller_compilation_unit_.GetClassLinker();
Thread* self = Thread::Current();
for (const dex::TypeIndex& type_index : dex_pc_data.classes) {
const DexFile* dex_file = caller_compilation_unit_.GetDexFile();
const char* descriptor = pci->GetTypeDescriptor(dex_file, type_index);
ObjPtr<mirror::ClassLoader> class_loader = caller_compilation_unit_.GetClassLoader().Get();
ObjPtr<mirror::Class> clazz = class_linker->LookupResolvedType(descriptor, class_loader);
ObjPtr<mirror::Class> clazz =
class_linker->FindClass(self, descriptor, caller_compilation_unit_.GetClassLoader());
if (clazz == nullptr) {
self->ClearException(); // Clean up the exception left by type resolution.
VLOG(compiler) << "Could not find class from inline cache in AOT mode "
<< invoke_instruction->GetMethodReference().PrettyMethod()
<< " : "
Expand Down
11 changes: 8 additions & 3 deletions compiler/optimizing/reference_type_propagation.cc
Original file line number Diff line number Diff line change
Expand Up @@ -539,9 +539,14 @@ void ReferenceTypePropagation::RTPVisitor::UpdateReferenceTypeInfo(HInstruction*
DCHECK_EQ(instr->GetType(), DataType::Type::kReference);

ScopedObjectAccess soa(Thread::Current());
ObjPtr<mirror::DexCache> dex_cache = FindDexCacheWithHint(soa.Self(), dex_file, hint_dex_cache_);
ObjPtr<mirror::Class> klass = Runtime::Current()->GetClassLinker()->LookupResolvedType(
type_idx, dex_cache, dex_cache->GetClassLoader());
StackHandleScope<2> hs(soa.Self());
Handle<mirror::DexCache> dex_cache =
hs.NewHandle(FindDexCacheWithHint(soa.Self(), dex_file, hint_dex_cache_));
Handle<mirror::ClassLoader> loader = hs.NewHandle(dex_cache->GetClassLoader());
ObjPtr<mirror::Class> klass = Runtime::Current()->GetClassLinker()->ResolveType(
type_idx, dex_cache, loader);
DCHECK_EQ(klass == nullptr, soa.Self()->IsExceptionPending());
soa.Self()->ClearException(); // Clean up the exception left by type resolution if any.
SetClassAsTypeInfo(instr, klass, is_exact);
}

Expand Down
4 changes: 1 addition & 3 deletions dex2oat/dex2oat.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1752,9 +1752,7 @@ class Dex2Oat final {
}

// Setup VerifierDeps for compilation and report if we fail to parse the data.
// When we do profile guided optimizations, the compiler currently needs to run
// full verification.
if (!DoProfileGuidedOptimizations() && input_vdex_file_ != nullptr) {
if (input_vdex_file_ != nullptr) {
std::unique_ptr<verifier::VerifierDeps> verifier_deps(
new verifier::VerifierDeps(dex_files, /*output_only=*/ false));
if (!verifier_deps->ParseStoredData(dex_files, input_vdex_file_->GetVerifierDepsData())) {
Expand Down
27 changes: 21 additions & 6 deletions dex2oat/driver/compiler_driver.cc
Original file line number Diff line number Diff line change
Expand Up @@ -471,7 +471,13 @@ static void CompileMethodQuick(
const DexFile& dex_file,
Handle<mirror::DexCache> dex_cache) {
DCHECK(driver != nullptr);
const VerificationResults* results = driver->GetVerificationResults();
DCHECK(results != nullptr);
MethodReference method_ref(&dex_file, method_idx);
CompiledMethod* compiled_method = nullptr;
if (results->IsUncompilableMethod(method_ref)) {
return compiled_method;
}

if ((access_flags & kAccNative) != 0) {
// Are we extracting only and have support for generic JNI down calls?
Expand All @@ -496,14 +502,9 @@ static void CompileMethodQuick(
// Method is annotated with @NeverCompile and should not be compiled.
} else {
const CompilerOptions& compiler_options = driver->GetCompilerOptions();
const VerificationResults* results = driver->GetVerificationResults();
DCHECK(results != nullptr);
MethodReference method_ref(&dex_file, method_idx);
// Don't compile class initializers unless kEverything.
bool compile = (compiler_options.GetCompilerFilter() == CompilerFilter::kEverything) ||
((access_flags & kAccConstructor) == 0) || ((access_flags & kAccStatic) == 0);
// Check if it's an uncompilable method found by the verifier.
compile = compile && !results->IsUncompilableMethod(method_ref);
// Check if we should compile based on the profile.
compile = compile && ShouldCompileBasedOnProfile(compiler_options, profile_index, method_ref);

Expand Down Expand Up @@ -1711,6 +1712,7 @@ static void LoadAndUpdateStatus(const ClassAccessor& accessor,
// a boot image class, or a class in a different dex file for multidex, and
// we should not update the status in that case.
if (&cls->GetDexFile() == &accessor.GetDexFile()) {
VLOG(compiler) << "Updating class status of " << std::string(descriptor) << " to " << status;
ObjectLock<mirror::Class> lock(self, cls);
mirror::Class::SetStatus(cls, status, self);
}
Expand Down Expand Up @@ -1753,6 +1755,8 @@ bool CompilerDriver::FastVerify(jobject jclass_loader,
!GetCompilerOptions().IsAnyCompilationEnabled() &&
!GetCompilerOptions().IsGeneratingImage();

const bool is_generating_image = GetCompilerOptions().IsGeneratingImage();

// We successfully validated the dependencies, now update class status
// of verified classes. Note that the dependencies also record which classes
// could not be fully verified; we could try again, but that would hurt verification
Expand All @@ -1776,14 +1780,24 @@ bool CompilerDriver::FastVerify(jobject jclass_loader,
// fail, but that's OK.
compiled_classes_.Insert(ref, existing, status);
} else {
if (is_generating_image &&
status == ClassStatus::kVerifiedNeedsAccessChecks &&
GetCompilerOptions().IsImageClass(accessor.GetDescriptor())) {
// If the class will be in the image, we can rely on the ArtMethods
// telling that they need access checks.
VLOG(compiler) << "Promoting "
<< std::string(accessor.GetDescriptor())
<< " from needs access checks to verified given it is an image class";
status = ClassStatus::kVerified;
}
// Update the class status, so later compilation stages know they don't need to verify
// the class.
LoadAndUpdateStatus(accessor, status, class_loader, soa.Self());
}

// Vdex marks class as unverified for two reasons only:
// 1. It has a hard failure, or
// 2. Once of its method needs lock counting.
// 2. One of its method needs lock counting.
//
// The optimizing compiler expects a method to not have a hard failure before
// compiling it, so for simplicity just disable any compilation of methods
Expand Down Expand Up @@ -2143,6 +2157,7 @@ class InitializeClassVisitor : public CompilationVisitor {
// Also return early and don't store the class status in the recorded class status.
return;
}

ClassStatus old_status = klass->GetStatus();
// Only try to initialize classes that were successfully verified.
if (klass->IsVerified()) {
Expand Down
1 change: 1 addition & 0 deletions test/660-clinit/profile
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,6 @@ LG;
LGs;
LObjectRef;
LInvokeStatic;
LInImage;
LClinitE;
LPrint;
10 changes: 8 additions & 2 deletions test/660-clinit/src/Main.java
Original file line number Diff line number Diff line change
Expand Up @@ -202,13 +202,19 @@ static int exec(int a, int b) {
}
}

class InImage {
static int exec(int a, int b) {
return a + b;
}
}

// test of INVOKE_STATIC instruction
class InvokeStatic {
static int a;
static int b;
static {
a = Add.exec(10, 20);
b = Mul.exec(10, 20);
a = InImage.exec(10, 20);
b = InImage.exec(10, 20);
}
}

Expand Down

0 comments on commit be726b5

Please sign in to comment.