commit c0f3646d88824dd0b1a376858d435b417c147f3f
author Clément Béra <clementbera@google.com> Tue Jul 30 10:57:37 2024 +0200
committer Clément Béra <clementbera@google.com> Tue Jul 30 09:02:23 2024 +0000
tree a7c8f23bd195652342c9005e57221e31012b53ea
parent 60229d5e317dba6e7f2ea43d1051f936c986428b

Fix SplitBranch with invalid phi

-Regress Tivi size by 0.007%

Bug: b/343136777
Change-Id: Ie42724bf73b57ebbaa5145a054cce9e6bae06f56

src/main/java/com/android/tools/r8/ir/conversion/passes/SplitBranch.java

diff --git a/src/main/java/com/android/tools/r8/ir/conversion/passes/SplitBranch.java b/src/main/java/com/android/tools/r8/ir/conversion/passes/SplitBranch.java
index e72d7ec..c73dd4e 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/passes/SplitBranch.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/passes/SplitBranch.java
@@ -179,7 +179,8 @@
       if (isNumberAgainstConstNumberIf(theIf)
           && nonConstNumberOperand.isPhi()
           && nonConstNumberOperand.hasSingleUniqueUser()
-          && !nonConstNumberOperand.hasPhiUsers()) {
+          && !nonConstNumberOperand.hasPhiUsers()
+          && nonConstNumberOperand.getBlock() == theIf.getBlock()) {
         candidates.add(block);
       }
     }

src/test/java/com/android/tools/r8/ir/optimize/ifs/Regress343136777Test.java

diff --git a/src/test/java/com/android/tools/r8/ir/optimize/ifs/Regress343136777Test.java b/src/test/java/com/android/tools/r8/ir/optimize/ifs/Regress343136777Test.java
index 2adf213..1b8c4ce 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/ifs/Regress343136777Test.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/ifs/Regress343136777Test.java
@@ -4,9 +4,6 @@
 
 package com.android.tools.r8.ir.optimize.ifs;
 
-import static org.junit.Assert.assertThrows;
-
-import com.android.tools.r8.CompilationFailedException;
 import com.android.tools.r8.TestBase;
 import com.android.tools.r8.TestParameters;
 import com.android.tools.r8.TestParametersCollection;
@@ -45,32 +42,24 @@
   @Test
   public void testD8Release() throws Exception {
     parameters.assumeDexRuntime();
-    // TODO(b/343136777: This should not fail.
-    assertThrows(
-        CompilationFailedException.class,
-        () ->
-            testForD8(parameters.getBackend())
-                .addProgramClasses(TestClass.class)
-                .setMinApi(parameters)
-                .release()
-                .compile()
-                .run(parameters.getRuntime(), TestClass.class)
-                .assertSuccessWithOutput(EXPECTED_OUTPUT));
+    testForD8(parameters.getBackend())
+        .addProgramClasses(TestClass.class)
+        .setMinApi(parameters)
+        .release()
+        .compile()
+        .run(parameters.getRuntime(), TestClass.class)
+        .assertSuccessWithOutput(EXPECTED_OUTPUT);
   }
 
   @Test
   public void testR8() throws Exception {
-    // TODO(b/343136777: This should not fail.
-    assertThrows(
-        CompilationFailedException.class,
-        () ->
-            testForR8(parameters.getBackend())
-                .addProgramClasses(TestClass.class)
-                .setMinApi(parameters)
-                .addKeepMainRule(TestClass.class)
-                .compile()
-                .run(parameters.getRuntime(), TestClass.class)
-                .assertSuccessWithOutput(EXPECTED_OUTPUT));
+    testForR8(parameters.getBackend())
+        .addProgramClasses(TestClass.class)
+        .setMinApi(parameters)
+        .addKeepMainRule(TestClass.class)
+        .compile()
+        .run(parameters.getRuntime(), TestClass.class)
+        .assertSuccessWithOutput(EXPECTED_OUTPUT);
   }
 
   static class TestClass {