Add access flag for previously warm methods

We want to know if the method was warm instead of just having a non
zero counter. This is required if we want to not compile all of the
startup methods, but still compile warm methods.

Test: test-art-host ART_TEST_JIT=true

Bug: 62200509

Change-Id: I6e04866f39f970b04b47342b7af5ed474e1f4172

5 files changed, 30 insertions, 18 deletions

diff --git a/compiler/optimizing/intrinsics.cc b/compiler/optimizing/intrinsics.cc
index 6236bd87ab..b1d2727e39 100644
--- a/compiler/optimizing/intrinsics.cc
+++ b/compiler/optimizing/intrinsics.cc
@@ -146,7 +146,7 @@ void IntrinsicsRecognizer::Run() {
           Intrinsics intrinsic = static_cast<Intrinsics>(art_method->GetIntrinsic());
           if (!CheckInvokeType(intrinsic, invoke)) {
             LOG(WARNING) << "Found an intrinsic with unexpected invoke type: "
-                << intrinsic << " for "
+                << static_cast<uint32_t>(intrinsic) << " for "
                 << art_method->PrettyMethod()
                 << invoke->DebugName();
           } else {
diff --git a/runtime/jit/jit_code_cache.cc b/runtime/jit/jit_code_cache.cc
index 42d7653b9b..fdac24e5a0 100644
--- a/runtime/jit/jit_code_cache.cc
+++ b/runtime/jit/jit_code_cache.cc
@@ -526,6 +526,15 @@ void JitCodeCache::CopyInlineCacheInto(const InlineCache& ic,
   }
 }
 
+static void ClearMethodCounter(ArtMethod* method, bool was_warm) {
+  if (was_warm) {
+    method->AddAccessFlags(kAccPreviouslyWarm);
+  }
+  // We reset the counter to 1 so that the profile knows that the method was executed at least once.
+  // This is required for layout purposes.
+  method->SetCounter(1);
+}
+
 uint8_t* JitCodeCache::CommitCodeInternal(Thread* self,
                                           ArtMethod* method,
                                           uint8_t* stack_map,
@@ -600,7 +609,7 @@ uint8_t* JitCodeCache::CommitCodeInternal(Thread* self,
         // Simply discard the compiled code. Clear the counter so that it may be recompiled later.
         // Hopefully the class hierarchy will be more stable when compilation is retried.
         single_impl_still_valid = false;
-        method->SetCounter(1);
+        ClearMethodCounter(method, /*was_warm*/ false);
         break;
       }
     }
@@ -1068,9 +1077,8 @@ void JitCodeCache::DoCollection(Thread* self, bool collect_profiling_info) {
         if (info->GetSavedEntryPoint() != nullptr) {
           info->SetSavedEntryPoint(nullptr);
           // We are going to move this method back to interpreter. Clear the counter now to
-          // give it a chance to be hot again, but set it to 1 so that this method can still be
-          // considered a startup method in case it's not executed again.
-          info->GetMethod()->SetCounter(1);
+          // give it a chance to be hot again.
+          ClearMethodCounter(info->GetMethod(), /*was_warm*/ true);
         }
       }
     } else if (kIsDebugBuild) {
@@ -1379,7 +1387,7 @@ bool JitCodeCache::NotifyCompilationOf(ArtMethod* method, Thread* self, bool osr
     VLOG(jit) << method->PrettyMethod() << " needs a ProfilingInfo to be compiled";
     // Because the counter is not atomic, there are some rare cases where we may not hit the
     // threshold for creating the ProfilingInfo. Reset the counter now to "correct" this.
-    method->SetCounter(1);
+    ClearMethodCounter(method, /*was_warm*/ false);
     return false;
   }
 
@@ -1432,11 +1440,10 @@ void JitCodeCache::InvalidateCompiledCodeFor(ArtMethod* method,
 
   if (method->GetEntryPointFromQuickCompiledCode() == header->GetEntryPoint()) {
     // The entrypoint is the one to invalidate, so we just update it to the interpreter entry point
-    // and clear the counter to get the method Jitted again. We reset the counter to 1 to preserve
-    // it as a potential startup method.
+    // and clear the counter to get the method Jitted again.
     Runtime::Current()->GetInstrumentation()->UpdateMethodsCode(
         method, GetQuickToInterpreterBridge());
-    method->SetCounter(1);
+    ClearMethodCounter(method, /*was_warm*/ profiling_info != nullptr);
   } else {
     MutexLock mu(Thread::Current(), lock_);
     auto it = osr_code_map_.find(method);
diff --git a/runtime/jit/profile_saver.cc b/runtime/jit/profile_saver.cc
index 166b6f4ba1..bc829cf9a7 100644
--- a/runtime/jit/profile_saver.cc
+++ b/runtime/jit/profile_saver.cc
@@ -194,7 +194,8 @@ class GetMethodsVisitor : public ClassVisitor {
     for (ArtMethod& method : klass->GetMethods(kRuntimePointerSize)) {
       if (!method.IsNative()) {
         if (method.GetCounter() >= startup_method_samples_ ||
-            method.GetProfilingInfo(kRuntimePointerSize) != nullptr) {
+            method.GetProfilingInfo(kRuntimePointerSize) != nullptr ||
+            (method.GetAccessFlags() & kAccPreviouslyWarm) != 0) {
           // Have samples, add to profile.
           const DexFile* dex_file =
               method.GetInterfaceMethodIfProxy(kRuntimePointerSize)->GetDexFile();
diff --git a/runtime/modifiers.h b/runtime/modifiers.h
index 461f870b1c..68ab4a4035 100644
--- a/runtime/modifiers.h
+++ b/runtime/modifiers.h
@@ -60,16 +60,20 @@ static constexpr uint32_t kAccFastNative =            0x00080000;  // method (de
 static constexpr uint32_t kAccCopied =                0x00100000;  // method (runtime)
 static constexpr uint32_t kAccMiranda =               0x00200000;  // method (dex only)
 static constexpr uint32_t kAccDefault =               0x00400000;  // method (runtime)
+
+// Set by the JIT when clearing profiling infos to denote that a method was previously warm.
+static constexpr uint32_t kAccPreviouslyWarm =        0x00800000;  // method (runtime)
+
 // This is set by the class linker during LinkInterfaceMethods. Prior to that point we do not know
 // if any particular method needs to be a default conflict. Used to figure out at runtime if
 // invoking this method will throw an exception.
-static constexpr uint32_t kAccDefaultConflict =       0x00800000;  // method (runtime)
+static constexpr uint32_t kAccDefaultConflict =       0x01000000;  // method (runtime)
 
 // Set by the verifier for a method we do not want the compiler to compile.
-static constexpr uint32_t kAccCompileDontBother =     0x01000000;  // method (runtime)
+static constexpr uint32_t kAccCompileDontBother =     0x02000000;  // method (runtime)
 
 // Set by the verifier for a method that could not be verified to follow structured locking.
-static constexpr uint32_t kAccMustCountLocks =        0x02000000;  // method (runtime)
+static constexpr uint32_t kAccMustCountLocks =        0x04000000;  // method (runtime)
 
 // Set by the class linker for a method that has only one implementation for a
 // virtual call.
@@ -85,8 +89,8 @@ static constexpr uint32_t kAccHasDefaultMethod          = 0x40000000;
 // class/ancestor overrides finalize()
 static constexpr uint32_t kAccClassIsFinalizable        = 0x80000000;
 
-static constexpr uint32_t kAccFlagsNotUsedByIntrinsic   = 0x007FFFFF;
-static constexpr uint32_t kAccMaxIntrinsic              = 0xFF;
+static constexpr uint32_t kAccFlagsNotUsedByIntrinsic   = 0x00FFFFFF;
+static constexpr uint32_t kAccMaxIntrinsic              = 0x7F;
 
 // Valid (meaningful) bits for a field.
 static constexpr uint32_t kAccValidFieldFlags = kAccPublic | kAccPrivate | kAccProtected |
@@ -96,7 +100,7 @@ static constexpr uint32_t kAccValidFieldFlags = kAccPublic | kAccPrivate | kAccP
 static constexpr uint32_t kAccValidMethodFlags = kAccPublic | kAccPrivate | kAccProtected |
     kAccStatic | kAccFinal | kAccSynchronized | kAccBridge | kAccVarargs | kAccNative |
     kAccAbstract | kAccStrict | kAccSynthetic | kAccMiranda | kAccConstructor |
-    kAccDeclaredSynchronized;
+    kAccDeclaredSynchronized | kAccPreviouslyWarm;
 
 // Valid (meaningful) bits for a class (not interface).
 // Note 1. These are positive bits. Other bits may have to be zero.
diff --git a/runtime/openjdkjvmti/ti_redefine.cc b/runtime/openjdkjvmti/ti_redefine.cc
index cca1486975..ca3a0e631a 100644
--- a/runtime/openjdkjvmti/ti_redefine.cc
+++ b/runtime/openjdkjvmti/ti_redefine.cc
@@ -602,8 +602,8 @@ bool Redefiner::ClassRedefinition::CheckSameMethods() {
     // Since direct methods have different flags than virtual ones (specifically direct methods must
     // have kAccPrivate or kAccStatic or kAccConstructor flags) we can tell if a method changes from
     // virtual to direct.
-    uint32_t new_flags = new_iter.GetMethodAccessFlags();
-    if (new_flags != (old_method->GetAccessFlags() & art::kAccValidMethodFlags)) {
+    uint32_t new_flags = new_iter.GetMethodAccessFlags() & ~art::kAccPreviouslyWarm;
+    if (new_flags != (old_method->GetAccessFlags() & (art::kAccValidMethodFlags ^ art::kAccPreviouslyWarm))) {
       RecordFailure(ERR(UNSUPPORTED_REDEFINITION_METHOD_MODIFIERS_CHANGED),
                     StringPrintf("method '%s' (sig: %s) had different access flags",
                                  new_method_name,