Do not look at dead phis during SsaRedundantPhiElimination.
Otherwise, we may replace a dead loop phi with its incoming input.
This broke an assumption during liveness analysis in the presence
of irreducible loops.
bug:28256552
Change-Id: Ia9e82026555e6b7f6a2cd37df4b765cd3684079d
diff --git a/compiler/optimizing/ssa_phi_elimination.cc b/compiler/optimizing/ssa_phi_elimination.cc
index aeb3109..0978ae1 100644
--- a/compiler/optimizing/ssa_phi_elimination.cc
+++ b/compiler/optimizing/ssa_phi_elimination.cc
@@ -139,8 +139,9 @@
continue;
}
- if (phi->InputCount() == 0) {
- DCHECK(phi->IsDead());
+ // If the phi is dead, we know we won't revive it and it will be removed,
+ // so don't process it.
+ if (phi->IsDead()) {
continue;
}
diff --git a/test/596-checker-dead-phi/expected.txt b/test/596-checker-dead-phi/expected.txt
new file mode 100644
index 0000000..d81cc07
--- /dev/null
+++ b/test/596-checker-dead-phi/expected.txt
@@ -0,0 +1 @@
+42
diff --git a/test/596-checker-dead-phi/info.txt b/test/596-checker-dead-phi/info.txt
new file mode 100644
index 0000000..7f7cf0f
--- /dev/null
+++ b/test/596-checker-dead-phi/info.txt
@@ -0,0 +1,2 @@
+Regression test for optimizing where we used to replace a dead loop
+phi with its first incoming input.
diff --git a/test/596-checker-dead-phi/smali/IrreducibleLoop.smali b/test/596-checker-dead-phi/smali/IrreducibleLoop.smali
new file mode 100644
index 0000000..bab2ba9
--- /dev/null
+++ b/test/596-checker-dead-phi/smali/IrreducibleLoop.smali
@@ -0,0 +1,74 @@
+# Copyright (C) 2016 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.
+
+.class public LIrreducibleLoop;
+
+.super Ljava/lang/Object;
+
+# Test case where liveness analysis produces linear order where loop blocks are
+# not adjacent. This revealed a bug in our SSA builder, where a dead loop phi would
+# be replaced by its incoming input during SsaRedundantPhiElimination.
+
+# Check that the outer loop suspend check environment only has the parameter vreg.
+## CHECK-START: int IrreducibleLoop.liveness(int) builder (after)
+## CHECK-DAG: <<Phi:i\d+>> Phi reg:4 loop:{{B\d+}} irreducible:false
+## CHECK-DAG: SuspendCheck env:[[_,_,_,_,<<Phi>>]] loop:{{B\d+}} irreducible:false
+
+# Check that the linear order has non-adjacent loop blocks.
+## CHECK-START: int IrreducibleLoop.liveness(int) liveness (after)
+## CHECK-DAG: Mul liveness:<<LPreEntry2:\d+>>
+## CHECK-DAG: Add liveness:<<LBackEdge1:\d+>>
+## CHECK-EVAL: <<LBackEdge1>> < <<LPreEntry2>>
+
+.method public static liveness(I)I
+ .registers 5
+
+ const-string v1, "MyString"
+
+ :header1
+ if-eqz p0, :body1
+
+ :exit
+ return p0
+
+ :body1
+ # The test will generate an incorrect linear order when the following IF swaps
+ # its successors. To do that, load a boolean value and compare NotEqual to 1.
+ sget-boolean v2, LIrreducibleLoop;->f:Z
+ const v3, 1
+ if-ne v2, v3, :pre_header2
+
+ :pre_entry2
+ # Add a marker on the irreducible loop entry.
+ mul-int/2addr p0, p0
+ goto :back_edge2
+
+ :back_edge2
+ goto :header2
+
+ :header2
+ if-eqz p0, :back_edge2
+
+ :back_edge1
+ # Add a marker on the outer loop back edge.
+ add-int/2addr p0, p0
+ # Set a wide register, to have v1 undefined at the back edge.
+ const-wide/16 v0, 0x1
+ goto :header1
+
+ :pre_header2
+ goto :header2
+.end method
+
+.field public static f:Z
diff --git a/test/596-checker-dead-phi/src/Main.java b/test/596-checker-dead-phi/src/Main.java
new file mode 100644
index 0000000..5a3fffc
--- /dev/null
+++ b/test/596-checker-dead-phi/src/Main.java
@@ -0,0 +1,32 @@
+/*
+ * Copyright (C) 2016 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.
+ */
+
+import java.lang.reflect.Method;
+
+public class Main {
+ // Workaround for b/18051191.
+ class InnerClass {}
+
+ public static void main(String[] args) throws Exception {
+ Class<?> c = Class.forName("IrreducibleLoop");
+ // Note that we don't actually enter the loops in the 'liveness'
+ // method, so this is just a sanity check that part of the code we
+ // generated for that method is correct.
+ Method m = c.getMethod("liveness", int.class);
+ Object[] arguments = { 42 };
+ System.out.println(m.invoke(null, arguments));
+ }
+}