Extend shorten live ranges to values used in invoke/range
Bug: b/302281605
Change-Id: I07c8100929435bf2676c7a8ef796f22d168fbd7c
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/passes/DexConstantOptimizer.java b/src/main/java/com/android/tools/r8/ir/conversion/passes/DexConstantOptimizer.java
index e4845ca..25fd979 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/passes/DexConstantOptimizer.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/passes/DexConstantOptimizer.java
@@ -244,11 +244,7 @@
LinkedList<BasicBlock> blocks = code.blocks;
for (BasicBlock block : blocks) {
shortenLiveRangesInsideBlock(
- code,
- block,
- dominatorTreeMemoization,
- addConstantInBlock,
- canonicalizer::isConstantCanonicalizationCandidate);
+ code, block, dominatorTreeMemoization, addConstantInBlock, canonicalizer::isConstant);
}
// Heuristic to decide if constant instructions are shared in dominator block
@@ -446,10 +442,13 @@
DominatorTree dominatorTree = dominatorTreeMemoization.computeIfAbsent();
BasicBlock dominator = dominatorTree.closestDominator(userBlocks);
+ // Do not move the constant if the constant instruction can throw and the dominator or the
+ // original block has catch handlers, or if the code may have monitor instructions, since this
+ // could lead to verification errors.
if (instruction.instructionTypeCanThrow()) {
- if (block.hasCatchHandlers() || dominator.hasCatchHandlers()) {
- // Do not move the constant if the constant instruction can throw
- // and the dominator or the original block has catch handlers.
+ if (block.hasCatchHandlers()
+ || dominator.hasCatchHandlers()
+ || code.metadata().mayHaveMonitorInstruction()) {
continue;
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/ConstantCanonicalizer.java b/src/main/java/com/android/tools/r8/ir/optimize/ConstantCanonicalizer.java
index cdd49d1..1b14342 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/ConstantCanonicalizer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/ConstantCanonicalizer.java
@@ -403,6 +403,27 @@
}
public boolean isConstantCanonicalizationCandidate(Instruction instruction) {
+ if (!isConstant(instruction)) {
+ return false;
+ }
+ // Do not canonicalize throwing instructions if there are monitor operations in the code.
+ // That could lead to unbalanced locking and could lead to situations where OOM exceptions
+ // could leave a synchronized method without unlocking the monitor.
+ if (instruction.instructionTypeCanThrow() && code.metadata().mayHaveMonitorInstruction()) {
+ return false;
+ }
+ // Constants that are used by invoke range are not canonicalized to be compliant with the
+ // optimization splitRangeInvokeConstant that gives the register allocator more freedom in
+ // assigning register to ranged invokes which can greatly reduce the number of register
+ // needed (and thereby code size as well). Thus no need to do a transformation that should
+ // be removed later by another optimization.
+ if (constantUsedByInvokeRange(instruction)) {
+ return false;
+ }
+ return true;
+ }
+
+ public boolean isConstant(Instruction instruction) {
// Interested only in instructions types that can be canonicalized, i.e., ConstClass,
// ConstNumber, (DexItemBased)?ConstString, InstanceGet and StaticGet.
switch (instruction.opcode()) {
@@ -460,20 +481,6 @@
if (instruction.outValue().hasLocalInfo()) {
return false;
}
- // Do not canonicalize throwing instructions if there are monitor operations in the code.
- // That could lead to unbalanced locking and could lead to situations where OOM exceptions
- // could leave a synchronized method without unlocking the monitor.
- if (instruction.instructionTypeCanThrow() && code.metadata().mayHaveMonitorInstruction()) {
- return false;
- }
- // Constants that are used by invoke range are not canonicalized to be compliant with the
- // optimization splitRangeInvokeConstant that gives the register allocator more freedom in
- // assigning register to ranged invokes which can greatly reduce the number of register
- // needed (and thereby code size as well). Thus no need to do a transformation that should
- // be removed later by another optimization.
- if (constantUsedByInvokeRange(instruction)) {
- return false;
- }
return true;
}
