Minor nterp improvements.

Do not resolve primitive field types for iput*, sput*
before updating the interpreter cache.

Introduce and use `ArtMethod::IsStringConstructor(),
a convenience helper function where we  avoid a read
barrier for checking if the declaring class is the
`String` class.

Change one `CHECK_LE()` to `DCHECK_LE()`.

Test: testrunner.py --host --interpreter
Change-Id: I17b0409cee5321e0ca389f053da1f767d2913d08

7 files changed, 40 insertions, 25 deletions

diff --git a/compiler/optimizing/instruction_builder.cc b/compiler/optimizing/instruction_builder.cc
index b7c4ba9e05..f9a513804c 100644
--- a/compiler/optimizing/instruction_builder.cc
+++ b/compiler/optimizing/instruction_builder.cc
@@ -985,8 +985,7 @@ static ArtMethod* ResolveMethod(uint16_t method_idx,
     DCHECK_EQ(*imt_or_vtable_index, ImTable::GetImtIndex(resolved_method));
   }
 
-  *is_string_constructor =
-      resolved_method->IsConstructor() && resolved_method->GetDeclaringClass()->IsStringClass();
+  *is_string_constructor = resolved_method->IsStringConstructor();
 
   return resolved_method;
 }
diff --git a/compiler/optimizing/sharpening.cc b/compiler/optimizing/sharpening.cc
index 1985b8c792..277edff33e 100644
--- a/compiler/optimizing/sharpening.cc
+++ b/compiler/optimizing/sharpening.cc
@@ -63,9 +63,9 @@ HInvokeStaticOrDirect::DispatchInfo HSharpening::SharpenLoadMethod(
     bool for_interface_call,
     CodeGenerator* codegen) {
   if (kIsDebugBuild) {
-    ScopedObjectAccess soa(Thread::Current());  // Required for GetDeclaringClass below.
+    ScopedObjectAccess soa(Thread::Current());  // Required for `IsStringConstructor()` below.
     DCHECK(callee != nullptr);
-    DCHECK(!(callee->IsConstructor() && callee->GetDeclaringClass()->IsStringClass()));
+    DCHECK(!callee->IsStringConstructor());
   }
 
   MethodLoadKind method_load_kind;
diff --git a/runtime/art_method-inl.h b/runtime/art_method-inl.h
index 0057bf9145..d105d9e41d 100644
--- a/runtime/art_method-inl.h
+++ b/runtime/art_method-inl.h
@@ -254,6 +254,15 @@ inline ObjPtr<mirror::Class> ArtMethod::ResolveClassFromTypeIndex(dex::TypeIndex
   return type;
 }
 
+inline bool ArtMethod::IsStringConstructor() {
+  uint32_t access_flags = GetAccessFlags();
+  DCHECK(!IsClassInitializer(access_flags));
+  return IsConstructor(access_flags) &&
+         // No read barrier needed for reading a constant reference only to read
+         // a constant string class flag. See `ReadBarrierOption`.
+         GetDeclaringClass<kWithoutReadBarrier>()->IsStringClass();
+}
+
 inline bool ArtMethod::IsOverridableByDefaultMethod() {
   // It is safe to avoid the read barrier here since the constant interface flag
   // in the `Class` object is stored before creating the `ArtMethod` and storing
diff --git a/runtime/art_method.h b/runtime/art_method.h
index f7f900f5e3..308a0702ef 100644
--- a/runtime/art_method.h
+++ b/runtime/art_method.h
@@ -584,6 +584,11 @@ class ArtMethod final {
     AddAccessFlags(kAccNterpInvokeFastPathFlag);
   }
 
+  // Returns whether the method is a string constructor. The method must not
+  // be a class initializer. (Class initializers are called from a different
+  // context where we do not need to check for string constructors.)
+  bool IsStringConstructor() REQUIRES_SHARED(Locks::mutator_lock_);
+
   // Returns true if this method could be overridden by a default method.
   bool IsOverridableByDefaultMethod() REQUIRES_SHARED(Locks::mutator_lock_);
 
diff --git a/runtime/interpreter/mterp/nterp.cc b/runtime/interpreter/mterp/nterp.cc
index 2498319651..a652d625ca 100644
--- a/runtime/interpreter/mterp/nterp.cc
+++ b/runtime/interpreter/mterp/nterp.cc
@@ -293,7 +293,7 @@ extern "C" size_t NterpGetMethod(Thread* self, ArtMethod* caller, const uint16_t
   Instruction::Code opcode = inst->Opcode();
   DCHECK(IsUint<8>(static_cast<std::underlying_type_t<Instruction::Code>>(opcode)));
   uint8_t raw_invoke_type = kOpcodeInvokeTypes[opcode];
-  CHECK_LE(raw_invoke_type, kMaxInvokeType);
+  DCHECK_LE(raw_invoke_type, kMaxInvokeType);
   InvokeType invoke_type = static_cast<InvokeType>(raw_invoke_type);
 
   // In release mode, this is just a simple load.
@@ -341,9 +341,7 @@ extern "C" size_t NterpGetMethod(Thread* self, ArtMethod* caller, const uint16_t
     }
     UpdateCache(self, dex_pc_ptr, result);
     return result;
-  } else if (resolved_method->GetDeclaringClass()->IsStringClass()
-             && !resolved_method->IsStatic()
-             && resolved_method->IsConstructor()) {
+  } else if (resolved_method->IsStringConstructor()) {
     CHECK_NE(invoke_type, kSuper);
     resolved_method = WellKnownClasses::StringInitToStringFactory(resolved_method);
     // Or the result with 1 to notify to nterp this is a string init method. We
@@ -369,14 +367,14 @@ extern "C" size_t NterpGetStaticField(Thread* self,
   const Instruction* inst = Instruction::At(dex_pc_ptr);
   uint16_t field_index = inst->VRegB_21c();
   ClassLinker* const class_linker = Runtime::Current()->GetClassLinker();
-  bool is_put = IsInstructionSPut(inst->Opcode());
+  Instruction::Code opcode = inst->Opcode();
   ArtField* resolved_field = ResolveFieldWithAccessChecks(
       self,
       class_linker,
       field_index,
       caller,
-      /* is_static */ true,
-      is_put,
+      /*is_static=*/ true,
+      /*is_put=*/ IsInstructionSPut(opcode),
       resolve_field_type);
 
   if (resolved_field == nullptr) {
@@ -399,11 +397,13 @@ extern "C" size_t NterpGetStaticField(Thread* self,
     // check for it.
     return reinterpret_cast<size_t>(resolved_field) | 1;
   } else {
-    // Try to resolve the field type even if we were not requested to. Only if
-    // the field type is successfully resolved can we update the cache. If we
+    // For sput-object, try to resolve the field type even if we were not requested to.
+    // Only if the field type is successfully resolved can we update the cache. If we
     // fail to resolve the type, we clear the exception to keep interpreter
     // semantics of not throwing when null is stored.
-    if (is_put && resolve_field_type == 0 && resolved_field->ResolveType() == nullptr) {
+    if (opcode == Instruction::SPUT_OBJECT &&
+        resolve_field_type == 0 &&
+        resolved_field->ResolveType() == nullptr) {
       DCHECK(self->IsExceptionPending());
       self->ClearException();
     } else {
@@ -422,14 +422,14 @@ extern "C" uint32_t NterpGetInstanceFieldOffset(Thread* self,
   const Instruction* inst = Instruction::At(dex_pc_ptr);
   uint16_t field_index = inst->VRegC_22c();
   ClassLinker* const class_linker = Runtime::Current()->GetClassLinker();
-  bool is_put = IsInstructionIPut(inst->Opcode());
+  Instruction::Code opcode = inst->Opcode();
   ArtField* resolved_field = ResolveFieldWithAccessChecks(
       self,
       class_linker,
       field_index,
       caller,
-      /* is_static */ false,
-      is_put,
+      /*is_static=*/ false,
+      /*is_put=*/ IsInstructionIPut(opcode),
       resolve_field_type);
   if (resolved_field == nullptr) {
     DCHECK(self->IsExceptionPending());
@@ -440,11 +440,13 @@ extern "C" uint32_t NterpGetInstanceFieldOffset(Thread* self,
     // of volatile.
     return -resolved_field->GetOffset().Uint32Value();
   }
-  // Try to resolve the field type even if we were not requested to. Only if
-  // the field type is successfully resolved can we update the cache. If we
+  // For iput-object, try to resolve the field type even if we were not requested to.
+  // Only if the field type is successfully resolved can we update the cache. If we
   // fail to resolve the type, we clear the exception to keep interpreter
   // semantics of not throwing when null is stored.
-  if (is_put && resolve_field_type == 0 && resolved_field->ResolveType() == nullptr) {
+  if (opcode == Instruction::IPUT_OBJECT &&
+      resolve_field_type == 0 &&
+      resolved_field->ResolveType() == nullptr) {
     DCHECK(self->IsExceptionPending());
     self->ClearException();
   } else {
diff --git a/runtime/method_handles.cc b/runtime/method_handles.cc
index 6dad810b18..73ec31ec0a 100644
--- a/runtime/method_handles.cc
+++ b/runtime/method_handles.cc
@@ -459,7 +459,7 @@ ArtMethod* RefineTargetMethod(Thread* self,
   } else if (handle_kind == mirror::MethodHandle::Kind::kInvokeDirect) {
     // String constructors are a special case, they are replaced with
     // StringFactory methods.
-    if (target_method->IsConstructor() && target_method->GetDeclaringClass()->IsStringClass()) {
+    if (target_method->IsStringConstructor()) {
       DCHECK(handle_type->GetRType()->IsStringClass());
       return WellKnownClasses::StringInitToStringFactory(target_method);
     }
diff --git a/runtime/reflection.cc b/runtime/reflection.cc
index 66cda453de..8f31e0bb68 100644
--- a/runtime/reflection.cc
+++ b/runtime/reflection.cc
@@ -535,7 +535,7 @@ JValue InvokeWithVarArgs(const ScopedObjectAccessAlreadyRunnable& soa,
     ThrowStackOverflowError(soa.Self());
     return JValue();
   }
-  bool is_string_init = method->GetDeclaringClass()->IsStringClass() && method->IsConstructor();
+  bool is_string_init = method->IsStringConstructor();
   if (is_string_init) {
     // Replace calls to String.<init> with equivalent StringFactory call.
     method = WellKnownClasses::StringInitToStringFactory(method);
@@ -577,7 +577,7 @@ JValue InvokeWithJValues(const ScopedObjectAccessAlreadyRunnable& soa,
     ThrowStackOverflowError(soa.Self());
     return JValue();
   }
-  bool is_string_init = method->GetDeclaringClass()->IsStringClass() && method->IsConstructor();
+  bool is_string_init = method->IsStringConstructor();
   if (is_string_init) {
     // Replace calls to String.<init> with equivalent StringFactory call.
     method = WellKnownClasses::StringInitToStringFactory(method);
@@ -620,7 +620,7 @@ JValue InvokeVirtualOrInterfaceWithJValues(const ScopedObjectAccessAlreadyRunnab
   }
   ObjPtr<mirror::Object> receiver = soa.Decode<mirror::Object>(obj);
   ArtMethod* method = FindVirtualMethod(receiver, interface_method);
-  bool is_string_init = method->GetDeclaringClass()->IsStringClass() && method->IsConstructor();
+  bool is_string_init = method->IsStringConstructor();
   if (is_string_init) {
     // Replace calls to String.<init> with equivalent StringFactory call.
     method = WellKnownClasses::StringInitToStringFactory(method);
@@ -664,7 +664,7 @@ JValue InvokeVirtualOrInterfaceWithVarArgs(const ScopedObjectAccessAlreadyRunnab
 
   ObjPtr<mirror::Object> receiver = soa.Decode<mirror::Object>(obj);
   ArtMethod* method = FindVirtualMethod(receiver, interface_method);
-  bool is_string_init = method->GetDeclaringClass()->IsStringClass() && method->IsConstructor();
+  bool is_string_init = method->IsStringConstructor();
   if (is_string_init) {
     // Replace calls to String.<init> with equivalent StringFactory call.
     method = WellKnownClasses::StringInitToStringFactory(method);