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) {