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

1 files changed, 10 insertions, 5 deletions

diff --git a/compiler/optimizing/ssa_builder.cc b/compiler/optimizing/ssa_builder.cc
index c0011cde8d..c8244aa800 100644
--- a/compiler/optimizing/ssa_builder.cc
+++ b/compiler/optimizing/ssa_builder.cc
@@ -927,16 +927,21 @@ void SsaBuilder::VisitInvokeStaticOrDirect(HInvokeStaticOrDirect* invoke) {
   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;
       }
     }