Revert "Move rewritten StringFactory call results into dex registers for deopt"
This reverts commit 8ca33bf04060fadd5b35fa93fa56547c62fe52e7.
597-deopt-new-string is failing.
Bug: 28846692
Bug: 28555675
Change-Id: Ibfb59ec36e089c987ea64e4af4ca3709e536412a
diff --git a/runtime/interpreter/interpreter.cc b/runtime/interpreter/interpreter.cc
index 74a0ab6..1d0e600 100644
--- a/runtime/interpreter/interpreter.cc
+++ b/runtime/interpreter/interpreter.cc
@@ -484,43 +484,6 @@
self->PopShadowFrame();
}
-static bool IsStringInit(Thread* self,
- const Instruction* instr,
- ArtMethod* caller,
- ArtMethod* callee)
- SHARED_REQUIRES(Locks::mutator_lock_) {
- if (instr->Opcode() == Instruction::INVOKE_DIRECT ||
- instr->Opcode() == Instruction::INVOKE_DIRECT_RANGE) {
- if (callee == nullptr) {
- // Don't know the callee. Resolve and find out.
- uint16_t callee_method_idx = (instr->Opcode() == Instruction::INVOKE_DIRECT_RANGE) ?
- instr->VRegB_3rc() : instr->VRegB_35c();
- ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
- // We just returned from callee, the method should resolve.
- // If it's a String constructor, the resolved method should not be null.
- callee = class_linker->ResolveMethod<ClassLinker::kNoICCECheckForCache>(
- self, callee_method_idx, caller, kDirect);
- }
- if (callee == nullptr) {
- // In case null is returned from method resolution, just return false
- // since a string init (just called) must not resolve to null.
- return false;
- }
- if (callee->GetDeclaringClass()->IsStringClass() &&
- callee->IsConstructor()) {
- return true;
- }
- }
- return false;
-}
-
-static int16_t GetReceiverRegisterForStringInit(const Instruction* instr) {
- DCHECK(instr->Opcode() == Instruction::INVOKE_DIRECT_RANGE ||
- instr->Opcode() == Instruction::INVOKE_DIRECT);
- return (instr->Opcode() == Instruction::INVOKE_DIRECT_RANGE) ?
- instr->VRegC_3rc() : instr->VRegC_35c();
-}
-
void EnterInterpreterFromDeoptimize(Thread* self,
ShadowFrame* shadow_frame,
bool from_code,
@@ -531,7 +494,6 @@
value.SetJ(ret_val->GetJ());
// Are we executing the first shadow frame?
bool first = true;
- ArtMethod* callee_method = nullptr;
while (shadow_frame != nullptr) {
// We do not want to recover lock state for lock counting when deoptimizing. Currently,
// the compiler should not have compiled a method that failed structured-locking checks.
@@ -557,13 +519,6 @@
// TODO: should be tested more once b/17586779 is fixed.
const Instruction* instr = Instruction::At(&code_item->insns_[dex_pc]);
if (instr->IsInvoke()) {
- if (IsStringInit(self, instr, shadow_frame->GetMethod(), callee_method)) {
- uint16_t this_obj_vreg = GetReceiverRegisterForStringInit(instr);
- // Move the StringFactory.newStringFromChars() result into the register representing
- // "this object" when invoking the string constructor in the original dex instruction.
- // Also move the result into all aliases.
- SetStringInitValueToAllAliases(shadow_frame, this_obj_vreg, value);
- }
new_dex_pc = dex_pc + instr->SizeInCodeUnits();
} else if (instr->Opcode() == Instruction::NEW_INSTANCE) {
// It's possible to deoptimize at a NEW_INSTANCE dex instruciton that's for a
@@ -574,14 +529,12 @@
instr->VRegB_21c(), shadow_frame->GetMethod());
DCHECK(klass->IsStringClass());
}
- // Move the StringFactory.newEmptyString() result into the destination register.
- shadow_frame->SetVRegReference(instr->VRegA_21c(), value.GetL());
// Skip the dex instruction since we essentially come back from an invocation.
new_dex_pc = dex_pc + instr->SizeInCodeUnits();
} else {
- CHECK(false) << "Unexpected instruction opcode " << instr->Opcode()
- << " at dex_pc " << dex_pc
- << " of method: " << PrettyMethod(shadow_frame->GetMethod(), false);
+ DCHECK(false) << "Unexpected instruction opcode " << instr->Opcode()
+ << " at dex_pc " << dex_pc
+ << " of method: " << PrettyMethod(shadow_frame->GetMethod(), false);
}
} else {
// Nothing to do, the dex_pc is the one at which the code requested
@@ -591,7 +544,6 @@
shadow_frame->SetDexPC(new_dex_pc);
value = Execute(self, code_item, *shadow_frame, value);
}
- callee_method = shadow_frame->GetMethod();
ShadowFrame* old_frame = shadow_frame;
shadow_frame = shadow_frame->GetLink();
ShadowFrame::DeleteDeoptimizedFrame(old_frame);
diff --git a/runtime/interpreter/interpreter_common.cc b/runtime/interpreter/interpreter_common.cc
index 53d5e43..12d70c5 100644
--- a/runtime/interpreter/interpreter_common.cc
+++ b/runtime/interpreter/interpreter_common.cc
@@ -540,30 +540,6 @@
result, method->GetInterfaceMethodIfProxy(sizeof(void*))->GetShorty());
}
-void SetStringInitValueToAllAliases(ShadowFrame* shadow_frame,
- uint16_t this_obj_vreg,
- JValue result)
- SHARED_REQUIRES(Locks::mutator_lock_) {
- Object* existing = shadow_frame->GetVRegReference(this_obj_vreg);
- if (existing == nullptr) {
- // If it's null, we come from compiled code that was deoptimized. Nothing to do,
- // as the compiler verified there was no alias.
- // Set the new string result of the StringFactory.
- shadow_frame->SetVRegReference(this_obj_vreg, result.GetL());
- return;
- }
- // Set the string init result into all aliases.
- for (uint32_t i = 0, e = shadow_frame->NumberOfVRegs(); i < e; ++i) {
- if (shadow_frame->GetVRegReference(i) == existing) {
- DCHECK_EQ(shadow_frame->GetVRegReference(i),
- reinterpret_cast<mirror::Object*>(shadow_frame->GetVReg(i)));
- shadow_frame->SetVRegReference(i, result.GetL());
- DCHECK_EQ(shadow_frame->GetVRegReference(i),
- reinterpret_cast<mirror::Object*>(shadow_frame->GetVReg(i)));
- }
- }
-}
-
template <bool is_range,
bool do_assignability_check,
size_t kVarArgMax>
@@ -763,7 +739,24 @@
}
if (string_init && !self->IsExceptionPending()) {
- SetStringInitValueToAllAliases(&shadow_frame, string_init_vreg_this, *result);
+ mirror::Object* existing = shadow_frame.GetVRegReference(string_init_vreg_this);
+ if (existing == nullptr) {
+ // If it's null, we come from compiled code that was deoptimized. Nothing to do,
+ // as the compiler verified there was no alias.
+ // Set the new string result of the StringFactory.
+ shadow_frame.SetVRegReference(string_init_vreg_this, result->GetL());
+ } else {
+ // Replace the fake string that was allocated with the StringFactory result.
+ for (uint32_t i = 0; i < shadow_frame.NumberOfVRegs(); ++i) {
+ if (shadow_frame.GetVRegReference(i) == existing) {
+ DCHECK_EQ(shadow_frame.GetVRegReference(i),
+ reinterpret_cast<mirror::Object*>(shadow_frame.GetVReg(i)));
+ shadow_frame.SetVRegReference(i, result->GetL());
+ DCHECK_EQ(shadow_frame.GetVRegReference(i),
+ reinterpret_cast<mirror::Object*>(shadow_frame.GetVReg(i)));
+ }
+ }
+ }
}
return !self->IsExceptionPending();
diff --git a/runtime/interpreter/interpreter_common.h b/runtime/interpreter/interpreter_common.h
index cc470f3..69376fd 100644
--- a/runtime/interpreter/interpreter_common.h
+++ b/runtime/interpreter/interpreter_common.h
@@ -1021,12 +1021,6 @@
ShadowFrame* shadow_frame,
JValue* result);
-// Set string value created from StringFactory.newStringFromXXX() into all aliases of
-// StringFactory.newEmptyString().
-void SetStringInitValueToAllAliases(ShadowFrame* shadow_frame,
- uint16_t this_obj_vreg,
- JValue result);
-
// Explicitly instantiate all DoInvoke functions.
#define EXPLICIT_DO_INVOKE_TEMPLATE_DECL(_type, _is_range, _do_check) \
template SHARED_REQUIRES(Locks::mutator_lock_) \
diff --git a/test/597-deopt-new-string/src/Main.java b/test/597-deopt-new-string/src/Main.java
index 556ada1..1224e40 100644
--- a/test/597-deopt-new-string/src/Main.java
+++ b/test/597-deopt-new-string/src/Main.java
@@ -48,12 +48,7 @@
throw new Error();
}
char[] arr = {'a', 'b', 'c'};
- String str = new String(arr, 0, arr.length);
- if (!str.equals("abc")) {
- System.out.println("Failure! " + str);
- System.exit(0);
- }
- return str;
+ return new String(arr, 0, arr.length);
}
public void run() {
@@ -73,11 +68,7 @@
} else {
// This thread keeps doing new String() from a char array.
while (!done) {
- String str = $noinline$run0();
- if (!str.equals("abc")) {
- System.out.println("Failure! " + str);
- System.exit(0);
- }
+ $noinline$run0();
}
}
}
