Fix a bug in the type propagation phase of optimizing.

The compiler was placing phis (and their floating point equivalent),
in a way that it did not expect.

Change-Id: I974be1ee4aae5d27d68c6bba171db0ed25377b70

5 files changed, 60 insertions, 9 deletions

diff --git a/compiler/optimizing/ssa_builder.cc b/compiler/optimizing/ssa_builder.cc
index fec40f93c7..b2cc11996e 100644
--- a/compiler/optimizing/ssa_builder.cc
+++ b/compiler/optimizing/ssa_builder.cc
@@ -183,8 +183,7 @@ static HDoubleConstant* GetDoubleEquivalent(HLongConstant* constant) {
 static HPhi* GetFloatOrDoubleEquivalentOfPhi(HPhi* phi, Primitive::Type type) {
   // We place the floating point phi next to this phi.
   HInstruction* next = phi->GetNext();
-  if (next == nullptr
-      || (next->GetType() != Primitive::kPrimDouble && next->GetType() != Primitive::kPrimFloat)) {
+  if (next == nullptr || (next->AsPhi()->GetRegNumber() != phi->GetRegNumber())) {
     ArenaAllocator* allocator = phi->GetBlock()->GetGraph()->GetArena();
     HPhi* new_phi = new (allocator) HPhi(allocator, phi->GetRegNumber(), phi->InputCount(), type);
     for (size_t i = 0, e = phi->InputCount(); i < e; ++i) {
@@ -195,9 +194,7 @@ static HPhi* GetFloatOrDoubleEquivalentOfPhi(HPhi* phi, Primitive::Type type) {
     phi->GetBlock()->InsertPhiAfter(new_phi, phi);
     return new_phi;
   } else {
-    // If there is already a phi with the expected type, we know it is the floating
-    // point equivalent of this phi.
-    DCHECK_EQ(next->AsPhi()->GetRegNumber(), phi->GetRegNumber());
+    DCHECK_EQ(next->GetType(), type);
     return next->AsPhi();
   }
 }
diff --git a/compiler/optimizing/ssa_type_propagation.cc b/compiler/optimizing/ssa_type_propagation.cc
index 3828142ed2..cb5ce20c46 100644
--- a/compiler/optimizing/ssa_type_propagation.cc
+++ b/compiler/optimizing/ssa_type_propagation.cc
@@ -90,10 +90,12 @@ void SsaTypePropagation::VisitBasicBlock(HBasicBlock* block) {
     }
   } else {
     for (HInstructionIterator it(block->GetPhis()); !it.Done(); it.Advance()) {
-      HPhi* phi = it.Current()->AsPhi();
-      if (UpdateType(phi)) {
-        AddDependentInstructionsToWorklist(phi);
-      }
+      // Eagerly compute the type of the phi, for quicker convergence. Note
+      // that we don't need to add users to the worklist because we are
+      // doing a reverse post-order visit, therefore either the phi users are
+      // non-loop phi and will be visited later in the visit, or are loop-phis,
+      // and they are already in the work list.
+      UpdateType(it.Current()->AsPhi());
     }
   }
 }
diff --git a/test/429-ssa-builder/expected.txt b/test/429-ssa-builder/expected.txt
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/test/429-ssa-builder/expected.txt
diff --git a/test/429-ssa-builder/info.txt b/test/429-ssa-builder/info.txt
new file mode 100644
index 0000000000..509d00f5de
--- /dev/null
+++ b/test/429-ssa-builder/info.txt
@@ -0,0 +1,3 @@
+Regression test for the type propagation phase of the optimizing
+compiler, that used to crash when dealing with phi floating-point
+equivalents.
diff --git a/test/429-ssa-builder/src/Main.java b/test/429-ssa-builder/src/Main.java
new file mode 100644
index 0000000000..32fcef0aa9
--- /dev/null
+++ b/test/429-ssa-builder/src/Main.java
@@ -0,0 +1,49 @@
+/*
+ * Copyright (C) 2014 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+public class Main {
+  public static void main(String[] args) {
+    if (new Main().$opt$TestFloatPhi() != 33.0f) {
+      throw new Error("Unexpected result");
+    }
+  }
+
+  public float $opt$TestFloatPhi() {
+    float a = floatField;
+    float b = 42.0f;
+    if (test1) {
+      // The phi for `a` will be found to be of type float.
+      a = otherFloatField;
+      // The phi for `b` will be found to be of type int (constants in DEX).
+      b = 33.0f;
+    }
+    // Use a different condition to avoid having dx being too clever.
+    if (test2) {
+      // Type propagation now realizes that `b` must be of type float. So
+      // it requests a float equivalent for `b`. Because the phi for `a` is
+      // next to the phi for `b` in the phi list, the compiler used to crash,
+      // assuming that a float phi following a phi *must* be for the same DEX
+      // register.
+      a = b;
+    }
+    return a;
+  }
+
+  float floatField = 4.2f;
+  float otherFloatField = 42.2f;
+  boolean test1 = true;
+  boolean test2 = true;
+}