Merge "Minor refactoring of class initialization analysis"
diff --git a/src/main/java/com/android/tools/r8/cf/LoadStoreHelper.java b/src/main/java/com/android/tools/r8/cf/LoadStoreHelper.java
index 8c79ea1..c5cb84e 100644
--- a/src/main/java/com/android/tools/r8/cf/LoadStoreHelper.java
+++ b/src/main/java/com/android/tools/r8/cf/LoadStoreHelper.java
@@ -26,6 +26,7 @@
import java.util.ArrayList;
import java.util.IdentityHashMap;
import java.util.List;
+import java.util.ListIterator;
import java.util.Map;
public class LoadStoreHelper {
@@ -35,7 +36,7 @@
private final AppInfo appInfo;
private Map<Value, ConstInstruction> clonableConstants = null;
- private BasicBlock block = null;
+ private ListIterator<BasicBlock> blockIterator = null;
public LoadStoreHelper(IRCode code, TypeVerificationHelper typesHelper, AppInfo appInfo) {
this.code = code;
@@ -96,16 +97,16 @@
public void insertLoadsAndStores() {
clonableConstants = new IdentityHashMap<>();
- for (BasicBlock block : code.blocks) {
- this.block = block;
- InstructionListIterator it = block.listIterator();
+ blockIterator = code.blocks.listIterator();
+ while (blockIterator.hasNext()) {
+ InstructionListIterator it = blockIterator.next().listIterator();
while (it.hasNext()) {
it.next().insertLoadAndStores(it, this);
}
clonableConstants.clear();
}
clonableConstants = null;
- block = null;
+ blockIterator = null;
}
public void insertPhiMoves(CfRegisterAllocator allocator) {
@@ -170,8 +171,7 @@
assert !(instruction.outValue() instanceof StackValue);
if (instruction.isConstInstruction()) {
ConstInstruction constInstruction = instruction.asConstInstruction();
- assert block != null;
- if (canRemoveConstInstruction(constInstruction, block)) {
+ if (canRemoveConstInstruction(constInstruction, instruction.getBlock())) {
assert !constInstruction.isDexItemBasedConstString()
|| constInstruction.outValue().numberOfUsers() == 1;
clonableConstants.put(instruction.outValue(), constInstruction);
@@ -180,7 +180,8 @@
return;
}
assert instruction.outValue().isUsed(); // Should have removed it above.
- } else if (!instruction.outValue().isUsed()) {
+ }
+ if (!instruction.outValue().isUsed()) {
popOutValue(instruction.outValue(), instruction, it);
return;
}
@@ -189,19 +190,40 @@
Store store = new Store(oldOutValue, newOutValue);
// Move the debugging-locals liveness pertaining to the instruction to the store.
instruction.moveDebugValues(store);
- add(store, instruction, it);
- }
-
- public void popOutValue(Value value, Instruction instruction, InstructionListIterator it) {
- StackValue newOutValue = createStackValue(value, 0);
- instruction.swapOutValue(newOutValue);
- add(new Pop(newOutValue), instruction, it);
+ BasicBlock storeBlock = instruction.getBlock();
+ // If the block has catch-handlers we are not allowed to modify the locals of the block. If the
+ // instruction is throwing, the action should be moved to a new block - otherwise, the store
+ // should be inserted and the remaining instructions should be moved along with the handlers to
+ // the new block.
+ boolean hasCatchHandlers = instruction.getBlock().hasCatchHandlers();
+ if (hasCatchHandlers && instruction.instructionTypeCanThrow()) {
+ storeBlock = it.split(this.code, this.blockIterator);
+ it = storeBlock.listIterator();
+ }
+ add(store, storeBlock, instruction.getPosition(), it);
+ if (hasCatchHandlers && !instruction.instructionTypeCanThrow()) {
+ it.split(this.code, this.blockIterator);
+ this.blockIterator.previous();
+ }
}
public void popOutType(DexType type, Instruction instruction, InstructionListIterator it) {
- StackValue newOutValue = createStackValue(type, 0);
+ popOutValue(createStackValue(type, 0), instruction, it);
+ }
+
+ private void popOutValue(Value value, Instruction instruction, InstructionListIterator it) {
+ popOutValue(createStackValue(value, 0), instruction, it);
+ }
+
+ private void popOutValue(
+ StackValue newOutValue, Instruction instruction, InstructionListIterator it) {
+ BasicBlock insertBlock = instruction.getBlock();
+ if (insertBlock.hasCatchHandlers() && instruction.instructionTypeCanThrow()) {
+ insertBlock = it.split(this.code, this.blockIterator);
+ it = insertBlock.listIterator();
+ }
instruction.swapOutValue(newOutValue);
- add(new Pop(newOutValue), instruction, it);
+ add(new Pop(newOutValue), insertBlock, instruction.getPosition(), it);
}
private static class PhiMove {
diff --git a/src/main/java/com/android/tools/r8/ir/code/IRCode.java b/src/main/java/com/android/tools/r8/ir/code/IRCode.java
index 3b33879..870ae6f 100644
--- a/src/main/java/com/android/tools/r8/ir/code/IRCode.java
+++ b/src/main/java/com/android/tools/r8/ir/code/IRCode.java
@@ -665,15 +665,8 @@
}
// After the throwing instruction only debug instructions and the final jump
// instruction is allowed.
- // TODO(mkroghj) Temporarily allow stack-operations to be after throwing instructions.
if (seenThrowing) {
- assert instruction.isDebugInstruction()
- || instruction.isJumpInstruction()
- || instruction.isStore()
- || instruction.isPop()
- || instruction.isDup()
- || instruction.isDup2()
- || instruction.isSwap();
+ assert instruction.isDebugInstruction() || instruction.isGoto();
}
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/Store.java b/src/main/java/com/android/tools/r8/ir/code/Store.java
index 21cb146..202bac6 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Store.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Store.java
@@ -75,7 +75,7 @@
@Override
public void insertLoadAndStores(InstructionListIterator it, LoadStoreHelper helper) {
- // Nothing to do. This is only hit because loads and stores are insert for phis.
+ throw new Unreachable("This IR must not be inserted before load and store insertion.");
}
@Override
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java b/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java
index 6b5f6c4..8a46e1e 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java
@@ -149,9 +149,6 @@
computeInitializers();
typeVerificationHelper = new TypeVerificationHelper(code, factory, appInfo);
typeVerificationHelper.computeVerificationTypes();
- if (!options.testing.noSplittingExceptionalEdges) {
- splitExceptionalBlocks();
- }
rewriter.converter.deadCodeRemover.run(code);
rewriteNots();
LoadStoreHelper loadStoreHelper = new LoadStoreHelper(code, typeVerificationHelper, appInfo);
@@ -223,38 +220,6 @@
return initializers;
}
- // Split all blocks with throwing instructions and exceptional edges such that any non-throwing
- // instructions that might define values prior to the throwing exception are excluded from the
- // try-catch range. Failure to do so will result in code that does not verify on the JVM.
- private void splitExceptionalBlocks() {
- ListIterator<BasicBlock> it = code.listIterator();
- while (it.hasNext()) {
- BasicBlock block = it.next();
- if (!block.hasCatchHandlers()) {
- continue;
- }
- int size = block.getInstructions().size();
- boolean isThrow = block.exit().isThrow();
- if ((isThrow && size == 1) || (!isThrow && size == 2)) {
- // Fast-path to avoid processing blocks with just a single throwing instruction.
- continue;
- }
- InstructionListIterator instructions = block.listIterator();
- boolean hasOutValues = false;
- while (instructions.hasNext()) {
- Instruction instruction = instructions.next();
- if (instruction.instructionTypeCanThrow()) {
- break;
- }
- hasOutValues |= instruction.outValue() != null;
- }
- if (hasOutValues) {
- instructions.previous();
- instructions.split(code, it);
- }
- }
- }
-
private void rewriteNots() {
for (BasicBlock block : code.blocks) {
InstructionListIterator it = block.listIterator();
diff --git a/src/main/java/com/android/tools/r8/utils/InternalOptions.java b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
index 3997982..7f4fbe1 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -539,7 +539,6 @@
public boolean forceNameReflectionOptimization = false;
public boolean disallowLoadStoreOptimization = false;
public Consumer<IRCode> irModifier = null;
- public boolean noSplittingExceptionalEdges = false;
}
private boolean hasMinApi(AndroidApiLevel level) {
diff --git a/src/test/java/com/android/tools/r8/cf/TryRangeTestRunner.java b/src/test/java/com/android/tools/r8/cf/TryRangeTestRunner.java
index e4c7098..926a22d 100644
--- a/src/test/java/com/android/tools/r8/cf/TryRangeTestRunner.java
+++ b/src/test/java/com/android/tools/r8/cf/TryRangeTestRunner.java
@@ -8,9 +8,19 @@
import com.android.tools.r8.CompilationMode;
import com.android.tools.r8.TestBase;
+import com.android.tools.r8.cf.code.CfInstruction;
+import com.android.tools.r8.cf.code.CfInvoke;
+import com.android.tools.r8.cf.code.CfLabel;
+import com.android.tools.r8.cf.code.CfLoad;
+import com.android.tools.r8.cf.code.CfStackInstruction;
+import com.android.tools.r8.graph.CfCode;
import com.android.tools.r8.ir.code.BasicBlock;
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.Instruction;
+import com.android.tools.r8.utils.StringUtils;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import java.util.List;
import java.util.ListIterator;
import org.junit.Test;
@@ -18,8 +28,7 @@
* This tests that we produce valid code when having normal-flow with exceptional edges in blocks.
* We might perform optimizations that add operations (dup, swap, etc.) before and after
* instructions that lie on the boundary of the exception table that is generated for a basic block.
- * If live-ranges are minimized this could produce VerifyErrors. TODO(b/119771771) Will fail if
- * shorten live ranges without shorten exception table range.
+ * If live-ranges are minimized this could produce VerifyErrors.
*/
public class TryRangeTestRunner extends TestBase {
@@ -37,27 +46,45 @@
o.testing.disallowLoadStoreOptimization = true;
})
.run(TryRangeTest.class)
- .assertSuccess();
+ .assertSuccessWithOutput(StringUtils.lines("10", "7.0"));
}
@Test
public void testRegisterAllocationLimitLeadingRange() throws Exception {
- testForR8(Backend.CF)
- .addProgramClasses(TryRangeTestLimitRange.class)
- .addKeepMainRule(TryRangeTestLimitRange.class)
- .setMode(CompilationMode.RELEASE)
- .minification(false)
- .noTreeShaking()
- .enableInliningAnnotations()
- .addOptionsModification(
- o -> {
- o.testing.disallowLoadStoreOptimization = true;
- o.testing.irModifier = this::processIR;
- // TODO(mkroghj) Remove this option entirely when splittingExceptionalEdges is moved.
- o.testing.noSplittingExceptionalEdges = true;
- })
- .run(TryRangeTestLimitRange.class)
- .assertFailure();
+ CodeInspector inspector =
+ testForR8(Backend.CF)
+ .addProgramClasses(TryRangeTestLimitRange.class)
+ .addKeepMainRule(TryRangeTestLimitRange.class)
+ .setMode(CompilationMode.RELEASE)
+ .minification(false)
+ .noTreeShaking()
+ .enableInliningAnnotations()
+ .addOptionsModification(
+ o -> {
+ o.testing.disallowLoadStoreOptimization = true;
+ o.testing.irModifier = this::processIR;
+ })
+ .run(TryRangeTestLimitRange.class)
+ .assertSuccessWithOutput("")
+ .inspector();
+ // Assert that we do not have any register-modifying instructions in the throwing block:
+ // L0: ; locals:
+ // iload 1;
+ // invokestatic com.android.tools.r8.cf.TryRangeTestLimitRange.doSomething(I)F
+ // L1: ; locals:
+ // 11: pop
+ ClassSubject clazz = inspector.clazz("com.android.tools.r8.cf.TryRangeTestLimitRange");
+ CfCode cfCode = clazz.uniqueMethodWithName("main").getMethod().getCode().asCfCode();
+ List<CfInstruction> instructions = cfCode.getInstructions();
+ CfLabel startLabel = cfCode.getTryCatchRanges().get(0).start;
+ int index = 0;
+ while (instructions.get(index) != startLabel) {
+ index++;
+ }
+ assert instructions.get(index + 1) instanceof CfLoad;
+ assert instructions.get(index + 2) instanceof CfInvoke;
+ assert instructions.get(index + 3) == cfCode.getTryCatchRanges().get(0).end;
+ assert instructions.get(index + 4) instanceof CfStackInstruction;
}
private void processIR(IRCode code) {
diff --git a/tools/as_utils.py b/tools/as_utils.py
index 10fb29f..5f224a6 100644
--- a/tools/as_utils.py
+++ b/tools/as_utils.py
@@ -126,6 +126,9 @@
for line in lines:
if '-printconfiguration' not in line:
f.write(line)
+ # Check that there is a line-break at the end of the file or insert one.
+ if lines[-1].strip():
+ f.write('\n')
f.write('-printconfiguration {}\n'.format(destination))
def FindProguardConfigurationFile(app, config, checkout_dir):
diff --git a/tools/run_on_as_app.py b/tools/run_on_as_app.py
index aacfbe9..3a9bf8c 100755
--- a/tools/run_on_as_app.py
+++ b/tools/run_on_as_app.py
@@ -113,6 +113,10 @@
'git_repo': 'https://github.com/mkj-gram/Tusky.git',
'flavor': 'blue'
},
+ 'Vungle-Android-SDK': {
+ 'app_id': 'com.publisher.vungle.sample',
+ 'git_repo': 'https://github.com/mkj-gram/Vungle-Android-SDK.git',
+ },
# This does not build yet.
'muzei': {
'git_repo': 'https://github.com/sgjesse/muzei.git',
diff --git a/tools/test.py b/tools/test.py
index f4cee88..bb6a98a 100755
--- a/tools/test.py
+++ b/tools/test.py
@@ -121,6 +121,8 @@
def Main():
(options, args) = ParseOptions()
if utils.is_bot():
+ print "Result of 'java -version':"
+ print subprocess.check_output(['java', '-version'])
gradle.RunGradle(['clean'])
gradle_args = ['--stacktrace']
