Revert "Flipping if statement in case fall-through successor always throws."
This reverts commit ff4d5858bb45dc9090f0bd05325eb5bfb8ebd6fc.
The reason for revert was breaking the Art test
test-art-host-gtest-exception_test.
Change-Id: I0bacad20aa86081f157bcf916780719b5eb4bbf2
diff --git a/src/main/java/com/android/tools/r8/graph/DexDebugEventBuilder.java b/src/main/java/com/android/tools/r8/graph/DexDebugEventBuilder.java
index 931fbfa..bee0910 100644
--- a/src/main/java/com/android/tools/r8/graph/DexDebugEventBuilder.java
+++ b/src/main/java/com/android/tools/r8/graph/DexDebugEventBuilder.java
@@ -60,15 +60,12 @@
// True if running in debug-mode with input code that contains line information, otherwise false.
private final boolean hasDebugPositions;
- // True if all throwing instructions had positions in the input code.
- private final boolean allThrowingInstructionsHavePositions;
public DexDebugEventBuilder(IRCode code, InternalOptions options) {
this.method = code.method;
this.factory = options.itemFactory;
this.options = options;
hasDebugPositions = code.hasDebugPositions;
- allThrowingInstructionsHavePositions = code.doAllThrowingInstructionsHavePositions();
}
/** Add events at pc for instruction. */
@@ -90,7 +87,6 @@
// In debug mode check that all non-nop instructions have positions.
assert startLine == NO_LINE_INFO || !hasDebugPositions || !pcAdvancing || position.isSome()
- || (instruction.instructionTypeCanThrow() && !allThrowingInstructionsHavePositions)
: "PC-advancing instruction " + instruction + " expected to have an associated position.";
// In any mode check that nop instructions have no position info.
diff --git a/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java b/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java
index 8663b91..8d087b6 100644
--- a/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java
+++ b/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java
@@ -182,14 +182,6 @@
}
}
- public void swapSuccessors(BasicBlock a, BasicBlock b) {
- assert a != b;
- int aIndex = successors.indexOf(a);
- int bIndex = successors.indexOf(b);
- assert aIndex >= 0 && bIndex >= 0;
- swapSuccessorsByIndex(aIndex, bIndex);
- }
-
public void swapSuccessorsByIndex(int index1, int index2) {
assert index1 != index2;
if (hasCatchHandlers()) {
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
index 15dd25d..a353e93 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
@@ -1867,16 +1867,11 @@
public void simplifyIf(IRCode code, TypeEnvironment typeEnvironment) {
int color = code.reserveMarkingColor();
- boolean ifBranchFlipped = false;
for (BasicBlock block : code.blocks) {
if (block.isMarked(color)) {
continue;
}
if (block.exit().isIf()) {
- // Flip then/else branches if needed.
- if (flipIfBranchesIfNeeded(block)) {
- ifBranchFlipped = true;
- }
// First rewrite zero comparison.
rewriteIfWithConstZero(block);
@@ -1932,9 +1927,6 @@
}
code.removeMarkedBlocks(color);
code.returnMarkingColor(color);
- if (ifBranchFlipped) {
- code.traceBlocks();
- }
assert code.isConsistentSSA();
}
@@ -2200,27 +2192,6 @@
}
}
- private synchronized boolean flipIfBranchesIfNeeded(BasicBlock block) {
- If theIf = block.exit().asIf();
- BasicBlock trueTarget = theIf.getTrueTarget();
- BasicBlock fallthrough = theIf.fallthroughBlock();
- assert trueTarget != fallthrough;
-
- if (!fallthrough.isSimpleAlwaysThrowingPath() || trueTarget.isSimpleAlwaysThrowingPath()) {
- return false;
- }
-
- // In case fall-through block always trows there is a good chance that it
- // is created for error checks and 'trueTarget' represents most more common
- // non-error case. Flipping the if in this case may result in faster code
- // on older Android versions.
- List<Value> inValues = theIf.inValues();
- If newIf = new If(theIf.getType().inverted(), inValues);
- block.replaceLastInstruction(newIf);
- block.swapSuccessors(trueTarget, fallthrough);
- return true;
- }
-
public void rewriteLongCompareAndRequireNonNull(IRCode code, InternalOptions options) {
if (options.canUseLongCompareAndObjectsNonNull()) {
return;
diff --git a/src/test/java/com/android/tools/r8/movestringconstants/MoveStringConstantsTest.java b/src/test/java/com/android/tools/r8/movestringconstants/MoveStringConstantsTest.java
index 490162a..9dbfb13 100644
--- a/src/test/java/com/android/tools/r8/movestringconstants/MoveStringConstantsTest.java
+++ b/src/test/java/com/android/tools/r8/movestringconstants/MoveStringConstantsTest.java
@@ -52,39 +52,21 @@
// Check the instruction used for testInlinedIntoVoidMethod
MethodSubject methodThrowToBeInlined =
- clazz.method("void", "foo", ImmutableList.of(
- "java.lang.String", "java.lang.String", "java.lang.String", "java.lang.String"));
+ clazz.method("void", "foo", ImmutableList.of("java.lang.String"));
assertTrue(methodThrowToBeInlined.isPresent());
validateSequence(methodThrowToBeInlined.iterateInstructions(),
- // 'if' with "foo#1" is flipped.
- InstructionSubject::isIfEqz,
-
- // 'if' with "foo#2" is removed along with the constant.
-
- // 'if' with "foo#3" is removed so now we have unconditional call.
- insn -> insn.isConstString("StringConstants::foo#3"),
- InstructionSubject::isInvokeStatic,
- InstructionSubject::isThrow,
-
- // 'if's with "foo#4" and "foo#5" are flipped, but their throwing branches
- // are not moved to the end of the code (area for improvement?).
- insn -> insn.isConstString("StringConstants::foo#4"),
- InstructionSubject::isIfEqz, // Flipped if
- InstructionSubject::isGoto, // Jump around throwing branch.
- InstructionSubject::isInvokeStatic, // Throwing branch.
- InstructionSubject::isThrow,
-
- insn -> insn.isConstString("StringConstants::foo#5"),
- InstructionSubject::isIfEqz, // Flipped if
- InstructionSubject::isReturnVoid, // Final return statement.
- InstructionSubject::isInvokeStatic, // Throwing branch.
- InstructionSubject::isThrow,
-
- // After 'if' with "foo#1" flipped, always throwing branch
- // moved here along with the constant.
+ InstructionSubject::isIfNez,
insn -> insn.isConstString("StringConstants::foo#1"),
- InstructionSubject::isInvokeStatic,
- InstructionSubject::isThrow
+ // Below two are removed by optimization: non-null argument "".
+ // InstructionSubject::isIfNez,
+ // insn -> insn.isConstString("StringConstants::foo#2"),
+ // InstructionSubject::isIfNez, 'removed by optimization'
+ insn -> insn.isConstString("StringConstants::foo#3")
+ // Below four are removed, since a safe call of arg.length() indicates arg is not null.
+ // insn -> insn.isConstString("StringConstants::foo#4"),
+ // InstructionSubject::isIfNez,
+ // insn -> insn.isConstString("StringConstants::foo#5"),
+ // InstructionSubject::isIfNez
);
}
diff --git a/src/test/java/com/android/tools/r8/movestringconstants/TestClass.java b/src/test/java/com/android/tools/r8/movestringconstants/TestClass.java
index bf13fc3..b801804 100644
--- a/src/test/java/com/android/tools/r8/movestringconstants/TestClass.java
+++ b/src/test/java/com/android/tools/r8/movestringconstants/TestClass.java
@@ -5,19 +5,19 @@
package com.android.tools.r8.movestringconstants;
public class TestClass {
- static void foo(String arg1, String arg2, String arg3, String arg4) {
- Utils.check(arg1, "StringConstants::foo#1");
+ static void foo(String arg) {
+ Utils.check(arg, "StringConstants::foo#1");
Utils.check("", "StringConstants::foo#2");
- if (arg2.length() == 12345) {
+ if (arg.length() == 12345) {
Utils.check(null, "StringConstants::foo#3");
}
try {
- Utils.check(arg3, "StringConstants::foo#4");
+ Utils.check(arg, "StringConstants::foo#4");
} catch (Exception e) {
System.out.println(e.getMessage());
}
try {
- Utils.check(arg4, "StringConstants::foo#5");
+ Utils.check(arg, "StringConstants::foo#5");
} finally {
System.out.println("finally");
}
diff --git a/src/test/java/com/android/tools/r8/utils/DexInspector.java b/src/test/java/com/android/tools/r8/utils/DexInspector.java
index d4cec96..7db0d6c 100644
--- a/src/test/java/com/android/tools/r8/utils/DexInspector.java
+++ b/src/test/java/com/android/tools/r8/utils/DexInspector.java
@@ -6,7 +6,6 @@
import com.android.tools.r8.code.Const4;
import com.android.tools.r8.code.ConstString;
import com.android.tools.r8.code.Goto;
-import com.android.tools.r8.code.IfEqz;
import com.android.tools.r8.code.IfNez;
import com.android.tools.r8.code.Iget;
import com.android.tools.r8.code.IgetBoolean;
@@ -1000,10 +999,6 @@
return instruction instanceof IfNez;
}
- boolean isIfEqz(Instruction instruction) {
- return instruction instanceof IfEqz;
- }
-
boolean isFieldAccess(Instruction instruction) {
return isInstanceGet(instruction)
|| isInstancePut(instruction)
@@ -1114,10 +1109,6 @@
return factory.isIfNez(instruction);
}
- public boolean isIfEqz() {
- return factory.isIfEqz(instruction);
- }
-
public boolean isReturnVoid() {
return factory.isReturnVoid(instruction);
}
