Allow redefined intrinsics
We make a change to allow intrinsics to be redefined. Note that
redefined intrinisics will no-longer be optimized as much and will not
be inlined or moved out of loops.
Test: ART_TEST_JIT=true mma -j40 test-art-host
Change-Id: Id6df89bb247d21f7859b48356ceba310eef9d105
diff --git a/runtime/art_method.h b/runtime/art_method.h
index 3d51fdd..99d7a49 100644
--- a/runtime/art_method.h
+++ b/runtime/art_method.h
@@ -223,13 +223,10 @@
}
bool IsObsolete() {
- // TODO Should maybe make this IsIntrinsic check not needed
- return !IsIntrinsic() && (GetAccessFlags() & kAccObsoleteMethod) != 0;
+ return (GetAccessFlags() & kAccObsoleteMethod) != 0;
}
void SetIsObsolete() {
- // TODO We should really support redefining intrinsic if possible.
- DCHECK(!IsIntrinsic());
AddAccessFlags(kAccObsoleteMethod);
}
diff --git a/runtime/modifiers.h b/runtime/modifiers.h
index ae6b31d..461f870 100644
--- a/runtime/modifiers.h
+++ b/runtime/modifiers.h
@@ -45,6 +45,9 @@
static constexpr uint32_t kAccConstructor = 0x00010000; // method (dex only) <(cl)init>
static constexpr uint32_t kAccDeclaredSynchronized = 0x00020000; // method (dex only)
static constexpr uint32_t kAccClassIsProxy = 0x00040000; // class (dex only)
+// Set to indicate that the ArtMethod is obsolete and has a different DexCache + DexFile from its
+// declaring class. This flag may only be applied to methods.
+static constexpr uint32_t kAccObsoleteMethod = 0x00040000; // method (runtime)
// Used by a method to denote that its execution does not need to go through slow path interpreter.
static constexpr uint32_t kAccSkipAccessChecks = 0x00080000; // method (dex only)
// Used by a class to denote that the verifier has attempted to check it at least once.
@@ -67,10 +70,6 @@
// Set by the verifier for a method that could not be verified to follow structured locking.
static constexpr uint32_t kAccMustCountLocks = 0x02000000; // method (runtime)
-// Set to indicate that the ArtMethod is obsolete and has a different DexCache from its declaring
-// class.
-// TODO Might want to re-arrange some of these so that we can have obsolete + intrinsic methods.
-static constexpr uint32_t kAccObsoleteMethod = 0x04000000; // method (runtime)
// Set by the class linker for a method that has only one implementation for a
// virtual call.
diff --git a/runtime/openjdkjvmti/ti_redefine.cc b/runtime/openjdkjvmti/ti_redefine.cc
index 8436045..2d532d1 100644
--- a/runtime/openjdkjvmti/ti_redefine.cc
+++ b/runtime/openjdkjvmti/ti_redefine.cc
@@ -170,10 +170,6 @@
// We cannot ensure that the right dex file is used in inlined frames so we don't support
// redefining them.
DCHECK(!IsInInlinedFrame()) << "Inlined frames are not supported when using redefinition";
- // TODO We should really support intrinsic obsolete methods.
- // TODO We should really support redefining intrinsics.
- // We don't support intrinsics so check for them here.
- DCHECK(!old_method->IsIntrinsic());
art::ArtMethod* new_obsolete_method = obsolete_maps_->FindObsoleteVersion(old_method);
if (new_obsolete_method == nullptr) {
// Create a new Obsolete Method and put it in the list.
@@ -518,6 +514,11 @@
CallbackCtx ctx(&map, linker->GetAllocatorForClassLoader(art_klass->GetClassLoader()));
// Add all the declared methods to the map
for (auto& m : art_klass->GetDeclaredMethods(art::kRuntimePointerSize)) {
+ if (m.IsIntrinsic()) {
+ LOG(WARNING) << "Redefining intrinsic method " << m.PrettyMethod() << ". This may cause the "
+ << "unexpected use of the original definition of " << m.PrettyMethod() << "in "
+ << "methods that have already been compiled.";
+ }
// It is possible to simply filter out some methods where they cannot really become obsolete,
// such as native methods and keep their original (possibly optimized) implementations. We don't
// do this, however, since we would need to mark these functions (still in the classes
@@ -526,8 +527,6 @@
// error checking from the interpreter which ensure we don't try to start executing obsolete
// methods.
ctx.obsolete_methods.insert(&m);
- // TODO Allow this or check in IsModifiableClass.
- DCHECK(!m.IsIntrinsic());
}
{
art::MutexLock mu(driver_->self_, *art::Locks::thread_list_lock_);
@@ -1143,6 +1142,10 @@
holder.GetOriginalDexFileBytes(counter));
counter++;
}
+ // TODO We should check for if any of the redefined methods are intrinsic methods here and, if any
+ // are, force a full-world deoptimization before finishing redefinition. If we don't do this then
+ // methods that have been jitted prior to the current redefinition being applied might continue
+ // to use the old versions of the intrinsics!
// TODO Shrink the obsolete method maps if possible?
// TODO Put this into a scoped thing.
runtime_->GetThreadList()->ResumeAll();
@@ -1193,6 +1196,8 @@
linker->SetEntryPointsToInterpreter(&method);
method.SetCodeItemOffset(dex_file_->FindCodeItemOffset(class_def, dex_method_idx));
method.SetDexCacheResolvedMethods(new_dex_cache->GetResolvedMethods(), image_pointer_size);
+ // Clear all the intrinsics related flags.
+ method.ClearAccessFlags(art::kAccIntrinsic | (~art::kAccFlagsNotUsedByIntrinsic));
// Notify the jit that this method is redefined.
art::jit::Jit* jit = driver_->runtime_->GetJit();
if (jit != nullptr) {