ART: Cannot assume String.<init> called on NewInstance
Irreducible loops create uneliminatable phis for all live vregs. This
breaks the StringFactory optimization which assumes that the first
input is always a NewInstance instruction.
Bug: 26676472
Change-Id: Ib7dfdadbafbbfef89e1f5b1a80eb75ecf792621a
diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h
index e222ef7..019be5d 100644
--- a/compiler/optimizing/nodes.h
+++ b/compiler/optimizing/nodes.h
@@ -3689,19 +3689,13 @@
DCHECK(!IsStaticWithExplicitClinitCheck());
}
- HNewInstance* GetThisArgumentOfStringInit() const {
+ HInstruction* GetAndRemoveThisArgumentOfStringInit() {
DCHECK(IsStringInit());
size_t index = InputCount() - 1;
- DCHECK(InputAt(index)->IsNewInstance());
- return InputAt(index)->AsNewInstance();
- }
-
- void RemoveThisArgumentOfStringInit() {
- DCHECK(IsStringInit());
- size_t index = InputCount() - 1;
- DCHECK(InputAt(index)->IsNewInstance());
+ HInstruction* input = InputAt(index);
RemoveAsUserOfInput(index);
inputs_.pop_back();
+ return input;
}
// Is this a call to a static method whose declaring class has an
diff --git a/compiler/optimizing/ssa_builder.cc b/compiler/optimizing/ssa_builder.cc
index c0011cd..c8244aa 100644
--- a/compiler/optimizing/ssa_builder.cc
+++ b/compiler/optimizing/ssa_builder.cc
@@ -927,16 +927,21 @@
if (invoke->IsStringInit()) {
// This is a StringFactory call which acts as a String constructor. Its
// result replaces the empty String pre-allocated by NewInstance.
- HNewInstance* new_instance = invoke->GetThisArgumentOfStringInit();
- invoke->RemoveThisArgumentOfStringInit();
+ HInstruction* arg_this = invoke->GetAndRemoveThisArgumentOfStringInit();
// Replacing the NewInstance might render it redundant. Keep a list of these
// to be visited once it is clear whether it is has remaining uses.
- uninitialized_strings_.push_back(new_instance);
+ if (arg_this->IsNewInstance()) {
+ uninitialized_strings_.push_back(arg_this->AsNewInstance());
+ } else {
+ DCHECK(arg_this->IsIrreducibleLoopHeaderPhi());
+ // NewInstance is not the direct input of the StringFactory call. It might
+ // be redundant but optimizing this case is not worth the effort.
+ }
- // Walk over all vregs and replace any occurrence of `new_instance` with `invoke`.
+ // Walk over all vregs and replace any occurrence of `arg_this` with `invoke`.
for (size_t vreg = 0, e = current_locals_->size(); vreg < e; ++vreg) {
- if ((*current_locals_)[vreg] == new_instance) {
+ if ((*current_locals_)[vreg] == arg_this) {
(*current_locals_)[vreg] = invoke;
}
}
diff --git a/test/563-checker-fakestring/smali/TestCase.smali b/test/563-checker-fakestring/smali/TestCase.smali
index 4bd804d..823ed1e 100644
--- a/test/563-checker-fakestring/smali/TestCase.smali
+++ b/test/563-checker-fakestring/smali/TestCase.smali
@@ -124,3 +124,31 @@
return-object v0
.end method
+
+# Test that the compiler does not assume that the first argument of String.<init>
+# is a NewInstance by inserting an irreducible loop between them (b/26676472).
+
+## CHECK-START-DEBUGGABLE: java.lang.String TestCase.thisNotNewInstance(byte[], boolean) register (after)
+## CHECK-DAG: InvokeStaticOrDirect env:[[<<Phi:l\d+>>,{{.*]]}}
+## CHECK-DAG: <<Phi>> Phi
+
+.method public static thisNotNewInstance([BZ)Ljava/lang/String;
+ .registers 5
+
+ new-instance v0, Ljava/lang/String;
+
+ # Irreducible loop
+ if-eqz p1, :loop_entry
+ :loop_header
+ const v1, 0x1
+ xor-int p1, p1, v1
+ :loop_entry
+ if-eqz p1, :string_init
+ goto :loop_header
+
+ :string_init
+ const-string v1, "UTF8"
+ invoke-direct {v0, p0, v1}, Ljava/lang/String;-><init>([BLjava/lang/String;)V
+ return-object v0
+
+.end method
diff --git a/test/563-checker-fakestring/src/Main.java b/test/563-checker-fakestring/src/Main.java
index 04df0f6..5489a0d 100644
--- a/test/563-checker-fakestring/src/Main.java
+++ b/test/563-checker-fakestring/src/Main.java
@@ -63,5 +63,13 @@
String result = (String) m.invoke(null, new Object[] { testData });
assertEqual(testString, result);
}
+
+ {
+ Method m = c.getMethod("thisNotNewInstance", byte[].class, boolean.class);
+ String result = (String) m.invoke(null, new Object[] { testData, true });
+ assertEqual(testString, result);
+ result = (String) m.invoke(null, new Object[] { testData, false });
+ assertEqual(testString, result);
+ }
}
}
