Fix a bug in DeadPhiHandling around conflicting phis.
Change-Id: I44ec40f71a7e05eb4295bea55f6045cb86017329
diff --git a/compiler/optimizing/ssa_builder.cc b/compiler/optimizing/ssa_builder.cc
index fb11d76..40c75af 100644
--- a/compiler/optimizing/ssa_builder.cc
+++ b/compiler/optimizing/ssa_builder.cc
@@ -56,6 +56,24 @@
DISALLOW_COPY_AND_ASSIGN(DeadPhiHandling);
};
+static bool HasConflictingEquivalent(HPhi* phi) {
+ if (phi->GetNext() == nullptr) {
+ return false;
+ }
+ HPhi* next = phi->GetNext()->AsPhi();
+ if (next->GetRegNumber() == phi->GetRegNumber()) {
+ if (next->GetType() == Primitive::kPrimVoid) {
+ // We only get a void type for an equivalent phi we processed and found out
+ // it was conflicting.
+ return true;
+ } else {
+ // Go to the next phi, in case it is also an equivalent.
+ return HasConflictingEquivalent(next);
+ }
+ }
+ return false;
+}
+
bool DeadPhiHandling::UpdateType(HPhi* phi) {
if (phi->IsDead()) {
// Phi was rendered dead while waiting in the worklist because it was replaced
@@ -87,21 +105,26 @@
if (new_type == Primitive::kPrimVoid) {
new_type = input_type;
} else if (new_type == Primitive::kPrimNot && input_type == Primitive::kPrimInt) {
+ if (input->IsPhi() && HasConflictingEquivalent(input->AsPhi())) {
+ // If we already asked for an equivalent of the input phi, but that equivalent
+ // ended up conflicting, make this phi conflicting too.
+ conflict = true;
+ break;
+ }
HInstruction* equivalent = SsaBuilder::GetReferenceTypeEquivalent(input);
if (equivalent == nullptr) {
conflict = true;
break;
- } else {
- phi->ReplaceInput(equivalent, i);
- if (equivalent->IsPhi()) {
- DCHECK_EQ(equivalent->GetType(), Primitive::kPrimNot);
- // We created a new phi, but that phi has the same inputs as the old phi. We
- // add it to the worklist to ensure its inputs can also be converted to reference.
- // If not, it will remain dead, and the algorithm will make the current phi dead
- // as well.
- equivalent->AsPhi()->SetLive();
- AddToWorklist(equivalent->AsPhi());
- }
+ }
+ phi->ReplaceInput(equivalent, i);
+ if (equivalent->IsPhi()) {
+ DCHECK_EQ(equivalent->GetType(), Primitive::kPrimNot);
+ // We created a new phi, but that phi has the same inputs as the old phi. We
+ // add it to the worklist to ensure its inputs can also be converted to reference.
+ // If not, it will remain dead, and the algorithm will make the current phi dead
+ // as well.
+ equivalent->AsPhi()->SetLive();
+ AddToWorklist(equivalent->AsPhi());
}
} else if (new_type == Primitive::kPrimInt && input_type == Primitive::kPrimNot) {
new_type = Primitive::kPrimNot;
diff --git a/test/533-regression-debugphi/expected.txt b/test/533-regression-debugphi/expected.txt
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/test/533-regression-debugphi/expected.txt
diff --git a/test/533-regression-debugphi/info.txt b/test/533-regression-debugphi/info.txt
new file mode 100644
index 0000000..a4d4857
--- /dev/null
+++ b/test/533-regression-debugphi/info.txt
@@ -0,0 +1,2 @@
+Test a regression where DeadPhiHandling would infinitely loop over
+complicated phi dependencies.
diff --git a/test/533-regression-debugphi/smali/TestCase.smali b/test/533-regression-debugphi/smali/TestCase.smali
new file mode 100644
index 0000000..1908e72
--- /dev/null
+++ b/test/533-regression-debugphi/smali/TestCase.smali
@@ -0,0 +1,72 @@
+# Copyright (C) 2015 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 LTestCase;
+.super Ljava/lang/Object;
+
+# This is a reduced test case that used to trigger an infinite loop
+# in the DeadPhiHandling phase of the optimizing compiler (only used
+# with debuggable flag).
+.method public static testCase(IILjava/lang/Object;)V
+ .registers 5
+ const/4 v0, 0x0
+
+ :B4
+ invoke-static {}, Ljava/lang/System;->nanoTime()J
+ goto :B7
+
+ :B7
+ invoke-static {}, Ljava/lang/System;->nanoTime()J
+ if-nez p2, :Btmp
+ goto :B111
+
+ :Btmp
+ invoke-static {}, Ljava/lang/System;->nanoTime()J
+ if-nez p2, :B9
+ goto :B110
+
+ :B13
+ invoke-static {}, Ljava/lang/System;->nanoTime()J
+ add-int v0, p0, p1
+ goto :B7
+
+ :B110
+ invoke-static {}, Ljava/lang/System;->nanoTime()J
+ add-int v0, p0, p1
+ goto :B111
+
+ :B111
+ invoke-static {}, Ljava/lang/System;->nanoTime()J
+ goto :B4
+
+ :B9
+ invoke-static {}, Ljava/lang/System;->nanoTime()J
+ if-nez p2, :B10
+
+ :B11
+ invoke-static {}, Ljava/lang/System;->nanoTime()J
+ move v1, v0
+ goto :B12
+
+ :B10
+ invoke-static {}, Ljava/lang/System;->nanoTime()J
+ move-object v1, p2
+ goto :B12
+
+ :B12
+ invoke-static {}, Ljava/lang/System;->nanoTime()J
+ goto :B13
+
+ return-void
+.end method
diff --git a/test/533-regression-debugphi/src/Main.java b/test/533-regression-debugphi/src/Main.java
new file mode 100644
index 0000000..858770f
--- /dev/null
+++ b/test/533-regression-debugphi/src/Main.java
@@ -0,0 +1,22 @@
+/*
+ * Copyright (C) 2015 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 {
+ // Workaround for b/18051191.
+ class InnerClass {}
+
+ public static void main(String[] args) {}
+}