Merge "Use 'info' level diagnostics for issues that are typically not actionable."
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java
index d587efe..3d2cc25 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -320,17 +320,28 @@
timing.end();
}
+ GraphLense graphLense = GraphLense.getIdentityLense();
+
if (options.proguardConfiguration.isAccessModificationAllowed()) {
- ClassAndMemberPublicizer.run(application, appInfo.dexItemFactory);
+ graphLense = ClassAndMemberPublicizer.run(
+ executorService, timing, application, appInfo, rootSet, graphLense);
// We can now remove visibility bridges. Note that we do not need to update the
// invoke-targets here, as the existing invokes will simply dispatch to the now
// visible super-method. MemberRebinding, if run, will then dispatch it correctly.
application = new VisibilityBridgeRemover(appInfo, application).run();
}
- GraphLense graphLense = GraphLense.getIdentityLense();
-
if (appInfo.hasLiveness()) {
+ if (options.proguardConfiguration.hasApplyMappingFile()) {
+ SeedMapper seedMapper =
+ SeedMapper.seedMapperFromFile(options.proguardConfiguration.getApplyMappingFile());
+ timing.begin("apply-mapping");
+ graphLense =
+ new ProguardMapApplier(appInfo.withLiveness(), graphLense, seedMapper).run(timing);
+ application = application.asDirect().rewrittenWithLense(graphLense);
+ appInfo = appInfo.withLiveness().rewrittenWithLense(application.asDirect(), graphLense);
+ timing.end();
+ }
graphLense = new MemberRebindingAnalysis(appInfo.withLiveness(), graphLense).run();
// Class merging requires inlining.
if (options.enableClassMerging && options.enableInlining) {
@@ -339,20 +350,11 @@
new VerticalClassMerger(application, appInfo.withLiveness(), graphLense, timing);
graphLense = classMerger.run();
timing.end();
-
+ application = application.asDirect().rewrittenWithLense(graphLense);
appInfo = appInfo.withLiveness()
- .prunedCopyFrom(application, classMerger.getRemovedClasses());
+ .prunedCopyFrom(application, classMerger.getRemovedClasses())
+ .rewrittenWithLense(application.asDirect(), graphLense);
}
- if (options.proguardConfiguration.hasApplyMappingFile()) {
- SeedMapper seedMapper =
- SeedMapper.seedMapperFromFile(options.proguardConfiguration.getApplyMappingFile());
- timing.begin("apply-mapping");
- graphLense =
- new ProguardMapApplier(appInfo.withLiveness(), graphLense, seedMapper).run(timing);
- timing.end();
- }
- application = application.asDirect().rewrittenWithLense(graphLense);
- appInfo = appInfo.withLiveness().rewrittenWithLense(application.asDirect(), graphLense);
// Collect switch maps and ordinals maps.
appInfo = new SwitchMapCollector(appInfo.withLiveness(), options).run();
appInfo = new EnumOrdinalMapCollector(appInfo.withLiveness(), options).run();
diff --git a/src/main/java/com/android/tools/r8/graph/DexClass.java b/src/main/java/com/android/tools/r8/graph/DexClass.java
index 2c188da..64c85f3 100644
--- a/src/main/java/com/android/tools/r8/graph/DexClass.java
+++ b/src/main/java/com/android/tools/r8/graph/DexClass.java
@@ -13,6 +13,7 @@
import com.google.common.collect.Iterators;
import java.util.Arrays;
import java.util.List;
+import java.util.Set;
import java.util.function.Consumer;
public abstract class DexClass extends DexItem {
@@ -158,6 +159,32 @@
return result;
}
+ public void virtualizeMethods(Set<DexEncodedMethod> privateInstanceMethods) {
+ int vLen = virtualMethods.length;
+ int dLen = directMethods.length;
+ int mLen = privateInstanceMethods.size();
+ assert mLen <= dLen;
+
+ DexEncodedMethod[] newDirectMethods = new DexEncodedMethod[dLen - mLen];
+ int index = 0;
+ for (int i = 0; i < dLen; i++) {
+ DexEncodedMethod encodedMethod = directMethods[i];
+ if (!privateInstanceMethods.contains(encodedMethod)) {
+ newDirectMethods[index++] = encodedMethod;
+ }
+ }
+ assert index == dLen - mLen;
+ setDirectMethods(newDirectMethods);
+
+ DexEncodedMethod[] newVirtualMethods = new DexEncodedMethod[vLen + mLen];
+ System.arraycopy(virtualMethods, 0, newVirtualMethods, 0, vLen);
+ index = vLen;
+ for (DexEncodedMethod encodedMethod : privateInstanceMethods) {
+ newVirtualMethods[index++] = encodedMethod;
+ }
+ setVirtualMethods(newVirtualMethods);
+ }
+
/**
* For all annotations on the class and all annotations on its methods and fields apply the
* specified consumer.
diff --git a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
index 362fc2c..845eecd 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -47,6 +47,7 @@
import com.android.tools.r8.utils.InternalOptions;
import com.google.common.collect.ImmutableList;
import java.util.Arrays;
+import java.util.BitSet;
import java.util.Collections;
import java.util.List;
import java.util.function.Consumer;
@@ -159,6 +160,10 @@
return isVirtualMethod() && !accessFlags.isAbstract();
}
+ public boolean isPublicized() {
+ return optimizationInfo.isPublicized();
+ }
+
public boolean isPublicMethod() {
return accessFlags.isPublic();
}
@@ -668,6 +673,7 @@
private boolean neverReturnsNormally = false;
private boolean returnsConstant = false;
private long returnedConstant = 0;
+ private boolean publicized = false;
private boolean forceInline = false;
private boolean useIdentifierNameString = false;
private boolean checksNullReceiverBeforeAnySideEffect = false;
@@ -677,6 +683,7 @@
private ClassInlinerEligibility classInlinerEligibility = null;
private TrivialInitializer trivialInitializerInfo = null;
private ParameterUsagesInfo parametersUsages = null;
+ private BitSet kotlinNotNullParamHints = null;
private OptimizationInfo() {
// Intentionally left empty.
@@ -687,6 +694,7 @@
neverReturnsNull = template.neverReturnsNull;
returnsConstant = template.returnsConstant;
returnedConstant = template.returnedConstant;
+ publicized = template.publicized;
forceInline = template.forceInline;
useIdentifierNameString = template.useIdentifierNameString;
checksNullReceiverBeforeAnySideEffect = template.checksNullReceiverBeforeAnySideEffect;
@@ -700,6 +708,14 @@
return parametersUsages == null ? null : parametersUsages.getParameterUsage(parameter);
}
+ public BitSet getKotlinNotNullParamHints() {
+ return kotlinNotNullParamHints;
+ }
+
+ public void setKotlinNotNullParamHints(BitSet hints) {
+ this.kotlinNotNullParamHints = hints;
+ }
+
public boolean returnsArgument() {
return returnedArgument != -1;
}
@@ -742,6 +758,10 @@
return returnedConstant;
}
+ public boolean isPublicized() {
+ return publicized;
+ }
+
public boolean forceInline() {
return forceInline;
}
@@ -782,6 +802,10 @@
forceInline = true;
}
+ private void markPublicized() {
+ publicized = true;
+ }
+
private void markUseIdentifierNameString() {
useIdentifierNameString = true;
}
@@ -843,6 +867,10 @@
ensureMutableOI().setParameterUsages(parametersUsages);
}
+ synchronized public void setKotlinNotNullParamHints(BitSet hints) {
+ ensureMutableOI().setKotlinNotNullParamHints(hints);
+ }
+
synchronized public void setTrivialInitializer(TrivialInitializer info) {
ensureMutableOI().setTrivialInitializer(info);
}
@@ -851,6 +879,10 @@
ensureMutableOI().markForceInline();
}
+ synchronized public void markPublicized() {
+ ensureMutableOI().markPublicized();
+ }
+
synchronized public void markUseIdentifierNameString() {
ensureMutableOI().markUseIdentifierNameString();
}
diff --git a/src/main/java/com/android/tools/r8/graph/DexMethodHandle.java b/src/main/java/com/android/tools/r8/graph/DexMethodHandle.java
index cd3c093..bfdc1b7 100644
--- a/src/main/java/com/android/tools/r8/graph/DexMethodHandle.java
+++ b/src/main/java/com/android/tools/r8/graph/DexMethodHandle.java
@@ -177,7 +177,8 @@
case INVOKE_SUPER:
return Type.SUPER;
default:
- throw new Unreachable("DexMethodHandle with unexpected type: " + this);
+ throw new Unreachable(
+ "Conversion to invoke type with unexpected method handle: " + this);
}
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/CheckCast.java b/src/main/java/com/android/tools/r8/ir/code/CheckCast.java
index 1a43128..51e1891 100644
--- a/src/main/java/com/android/tools/r8/ir/code/CheckCast.java
+++ b/src/main/java/com/android/tools/r8/ir/code/CheckCast.java
@@ -35,6 +35,11 @@
this.type = type;
}
+ @Override
+ boolean computeNeverNull() {
+ return object().isNeverNull();
+ }
+
public DexType getType() {
return type;
}
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 cb5aad0..72728a8 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
@@ -138,13 +138,10 @@
if (predecessors.size() <= 1) {
continue;
}
- // If any of the edges to the block are critical, we need to insert new blocks on each
- // containing the move-exception instruction which must remain the first instruction.
- if (block.entry() instanceof MoveException) {
- nextBlockNumber = block.splitCriticalExceptionEdges(
- nextBlockNumber, valueNumberGenerator, newBlocks::add);
- continue;
- }
+
+ // Exceptional edges are given unique header blocks and can have at most one predecessor.
+ assert !block.entry().isMoveException();
+
for (int predIndex = 0; predIndex < predecessors.size(); predIndex++) {
BasicBlock pred = predecessors.get(predIndex);
if (!pred.hasOneNormalExit()) {
@@ -164,6 +161,27 @@
blocks.addAll(newBlocks);
}
+ public boolean verifySplitCriticalEdges() {
+ for (BasicBlock block : blocks) {
+ // If there are multiple incoming edges, check each has a split block.
+ List<BasicBlock> predecessors = block.getPredecessors();
+ if (predecessors.size() > 1) {
+ for (BasicBlock predecessor : predecessors) {
+ assert predecessor.hasOneNormalExit();
+ assert predecessor.getSuccessors().get(0) == block;
+ }
+ }
+ // If there are outgoing exceptional edges, check that each has a split block.
+ if (block.hasCatchHandlers()) {
+ for (BasicBlock handler : block.getCatchHandlers().getUniqueTargets()) {
+ assert handler.getPredecessors().size() == 1;
+ assert handler.getPredecessors().get(0) == block;
+ }
+ }
+ }
+ return true;
+ }
+
/**
* Trace blocks and attempt to put fallthrough blocks immediately after the block that
* falls through. When we fail to do that we create a new fallthrough block with an explicit
diff --git a/src/main/java/com/android/tools/r8/ir/code/Instruction.java b/src/main/java/com/android/tools/r8/ir/code/Instruction.java
index 5d37ea6..62e518d 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Instruction.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Instruction.java
@@ -68,6 +68,12 @@
this.position = position;
}
+ boolean computeNeverNull() {
+ // By default just return the flag from the out value since it never changes.
+ assert outValue != null;
+ return outValue.isNeverNull();
+ }
+
public final void forceSetPosition(Position position) {
this.position = position;
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/Invoke.java b/src/main/java/com/android/tools/r8/ir/code/Invoke.java
index 819bb4b..fc5ee05 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Invoke.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Invoke.java
@@ -12,6 +12,7 @@
import com.android.tools.r8.graph.AppInfo;
import com.android.tools.r8.graph.DexItem;
import com.android.tools.r8.graph.DexMethod;
+import com.android.tools.r8.graph.DexMethodHandle.MethodHandleType;
import com.android.tools.r8.graph.DexProto;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.ir.analysis.type.TypeLatticeElement;
@@ -30,7 +31,29 @@
NEW_ARRAY,
MULTI_NEW_ARRAY,
CUSTOM,
- POLYMORPHIC
+ POLYMORPHIC;
+
+ public MethodHandleType toMethodHandle(DexMethod targetMethod) {
+ switch (this) {
+ case STATIC:
+ return MethodHandleType.INVOKE_STATIC;
+ case VIRTUAL:
+ return MethodHandleType.INVOKE_INSTANCE;
+ case DIRECT:
+ if (targetMethod.name.toString().equals("<init>")) {
+ return MethodHandleType.INVOKE_CONSTRUCTOR;
+ } else {
+ return MethodHandleType.INVOKE_DIRECT;
+ }
+ case INTERFACE:
+ return MethodHandleType.INVOKE_INTERFACE;
+ case SUPER:
+ return MethodHandleType.INVOKE_SUPER;
+ default:
+ throw new Unreachable(
+ "Conversion to method handle with unexpected invoke type: " + this);
+ }
+ }
}
public Invoke(Value result, List<Value> arguments) {
diff --git a/src/main/java/com/android/tools/r8/ir/code/Move.java b/src/main/java/com/android/tools/r8/ir/code/Move.java
index f27153d..b095bb4 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Move.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Move.java
@@ -29,6 +29,11 @@
}
}
+ @Override
+ boolean computeNeverNull() {
+ return src().isNeverNull();
+ }
+
public Value dest() {
return outValue;
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/Phi.java b/src/main/java/com/android/tools/r8/ir/code/Phi.java
index 51850a0..ed09ee7 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Phi.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Phi.java
@@ -57,7 +57,6 @@
// period of time to break cycles. When the cycle has been resolved they are completed
// exactly once by adding the operands.
assert operands.isEmpty();
- boolean canBeNull = false;
if (block.getPredecessors().size() == 0) {
throwUndefinedValueError();
}
@@ -66,13 +65,10 @@
// Since this read has been delayed we must provide the local info for the value.
Value operand = builder.readRegister(register, type, pred, edgeType, getLocalInfo());
operand.constrainType(type);
- canBeNull |= operand.canBeNull();
appendOperand(operand);
}
- if (!canBeNull) {
- markNeverNull();
- }
removeTrivialPhi();
+ recomputeNeverNull();
}
public void addOperands(List<Value> operands) {
@@ -84,20 +80,24 @@
// period of time to break cycles. When the cycle has been resolved they are completed
// exactly once by adding the operands.
assert this.operands.isEmpty();
- boolean canBeNull = false;
if (operands.size() == 0) {
throwUndefinedValueError();
}
for (Value operand : operands) {
- canBeNull |= operand.canBeNull();
appendOperand(operand);
}
- if (!canBeNull) {
- markNeverNull();
- }
if (removeTrivialPhi) {
removeTrivialPhi();
}
+ recomputeNeverNull();
+ }
+
+ // Implementation assumes that canBeNull may change to neverNull, but
+ // not other way around. This will need to be revised later.
+ void recomputeNeverNull() {
+ if (canBeNull() && operands.stream().allMatch(Value::isNeverNull)) {
+ markNeverNull();
+ }
}
public void addDebugValue(Value value) {
@@ -131,7 +131,10 @@
public void removeOperand(int index) {
operands.get(index).removePhiUser(this);
- operands.remove(index);
+ Value value = operands.remove(index);
+ if (value.canBeNull()) {
+ recomputeNeverNull();
+ }
}
public void removeOperandsByIndex(List<Integer> operandsToRemove) {
@@ -147,6 +150,7 @@
current = i + 1;
}
operands.addAll(copy.subList(current, copy.size()));
+ recomputeNeverNull();
}
public void replaceOperandAt(int predIndex, Value newValue) {
@@ -154,6 +158,9 @@
operands.set(predIndex, newValue);
newValue.addPhiUser(this);
current.removePhiUser(this);
+ if (current.canBeNull() && newValue.isNeverNull()) {
+ recomputeNeverNull();
+ }
}
void replaceOperand(Value current, Value newValue) {
@@ -163,6 +170,9 @@
newValue.addPhiUser(this);
}
}
+ if (current.canBeNull() && newValue.isNeverNull()) {
+ recomputeNeverNull();
+ }
}
void replaceDebugValue(Value current, Value newValue) {
diff --git a/src/main/java/com/android/tools/r8/ir/code/Value.java b/src/main/java/com/android/tools/r8/ir/code/Value.java
index 05dd801..815a38f 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Value.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Value.java
@@ -628,6 +628,17 @@
public void markNeverNull() {
assert !neverNull;
neverNull = true;
+
+ // Propagate never null flag change.
+ for (Instruction user : users) {
+ Value value = user.outValue;
+ if (value != null && value.canBeNull() && user.computeNeverNull()) {
+ value.markNeverNull();
+ }
+ }
+ for (Phi phi : phiUsers) {
+ phi.recomputeNeverNull();
+ }
}
/**
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/CfSourceCode.java b/src/main/java/com/android/tools/r8/ir/conversion/CfSourceCode.java
index 309e1d0..4cbebcf 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/CfSourceCode.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/CfSourceCode.java
@@ -586,7 +586,7 @@
}
@Override
- public int getMoveExceptionRegister() {
+ public int getMoveExceptionRegister(int instructionIndex) {
return CfState.Slot.STACK_OFFSET;
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/DexSourceCode.java b/src/main/java/com/android/tools/r8/ir/conversion/DexSourceCode.java
index 31ee706..b59794b 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/DexSourceCode.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/DexSourceCode.java
@@ -21,12 +21,12 @@
import com.android.tools.r8.code.InvokeSuperRange;
import com.android.tools.r8.code.InvokeVirtual;
import com.android.tools.r8.code.InvokeVirtualRange;
+import com.android.tools.r8.code.MoveException;
import com.android.tools.r8.code.MoveResult;
import com.android.tools.r8.code.MoveResultObject;
import com.android.tools.r8.code.MoveResultWide;
import com.android.tools.r8.code.SwitchPayload;
import com.android.tools.r8.code.Throw;
-import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.DebugLocalInfo;
import com.android.tools.r8.graph.DexCode;
import com.android.tools.r8.graph.DexCode.Try;
@@ -185,14 +185,19 @@
}
@Override
- public int getMoveExceptionRegister() {
- // No register, move-exception is manually entered during construction.
+ public int getMoveExceptionRegister(int instructionIndex) {
+ Instruction instruction = code.instructions[instructionIndex];
+ if (instruction instanceof MoveException) {
+ MoveException moveException = (MoveException) instruction;
+ return moveException.AA;
+ }
return -1;
}
@Override
public Position getDebugPositionAtOffset(int offset) {
- throw new Unreachable();
+ DexDebugEntry entry = getDebugEntryAtOffset(offset);
+ return entry == null ? preamblePosition : getCanonicalPositionAppendCaller(entry);
}
@Override
@@ -225,23 +230,30 @@
}
}
+ private DexDebugEntry getDebugEntryAtOffset(int offset) {
+ DexDebugEntry current = null;
+ if (debugEntries != null) {
+ for (DexDebugEntry entry : debugEntries) {
+ if (entry.address > offset) {
+ break;
+ }
+ current = entry;
+ }
+ }
+ return current;
+ }
+
private void updateDebugPosition(int instructionIndex, IRBuilder builder) {
if (debugEntries == null || debugEntries.isEmpty()) {
return;
}
- DexDebugEntry current = null;
int offset = instructionOffset(instructionIndex);
- for (DexDebugEntry entry : debugEntries) {
- if (entry.address > offset) {
- break;
- }
- current = entry;
- }
- if (current == null) {
+ DexDebugEntry entry = getDebugEntryAtOffset(offset);
+ if (entry == null) {
currentPosition = preamblePosition;
} else {
- currentPosition = getCanonicalPositionAppendCaller(current);
- if (current.lineEntry && current.address == offset) {
+ currentPosition = getCanonicalPositionAppendCaller(entry);
+ if (entry.lineEntry && entry.address == offset) {
builder.addDebugPosition(currentPosition);
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java b/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
index 5be54f5..963cf41 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
@@ -93,6 +93,8 @@
import it.unimi.dsi.fastutil.ints.IntIterator;
import it.unimi.dsi.fastutil.ints.IntList;
import it.unimi.dsi.fastutil.ints.IntSet;
+import it.unimi.dsi.fastutil.objects.Reference2IntMap;
+import it.unimi.dsi.fastutil.objects.Reference2IntOpenHashMap;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
@@ -142,6 +144,13 @@
}
}
+ private static class SplitBlockWorklistItem extends WorklistItem {
+
+ public SplitBlockWorklistItem(BasicBlock block) {
+ super(block, -1);
+ }
+ }
+
/**
* Representation of lists of values that can be used as keys in maps. A list of
* values is equal to another list of values if it contains exactly the same values
@@ -224,6 +233,10 @@
return all;
}
+ boolean hasJustOneNormalExit() {
+ return normalSuccessors.size() == 1 && exceptionalSuccessors.isEmpty();
+ }
+
BlockInfo split(
int blockStartOffset, int fallthroughOffset, Int2ReferenceMap<BlockInfo> targets) {
BlockInfo fallthroughInfo = new BlockInfo();
@@ -279,6 +292,7 @@
// Mapping from instruction offsets to basic-block targets.
private final Int2ReferenceSortedMap<BlockInfo> targets = new Int2ReferenceAVLTreeMap<>();
+ private final Reference2IntMap<BasicBlock> offsets = new Reference2IntOpenHashMap<>();
// Worklist of reachable blocks.
private final Queue<Integer> traceBlocksWorklist = new LinkedList<>();
@@ -296,7 +310,6 @@
private final LinkedList<BasicBlock> blocks = new LinkedList<>();
private BasicBlock currentBlock = null;
- private final List<BasicBlock.Pair> needGotoToCatchBlocks = new ArrayList<>();
final private ValueNumberGenerator valueNumberGenerator;
private final DexEncodedMethod method;
private final AppInfo appInfo;
@@ -365,7 +378,9 @@
source.setUp();
// Create entry block (at a non-targetable address).
- targets.put(INITIAL_BLOCK_OFFSET, new BlockInfo());
+ BlockInfo initialBlockInfo = new BlockInfo();
+ targets.put(INITIAL_BLOCK_OFFSET, initialBlockInfo);
+ offsets.put(initialBlockInfo.block, INITIAL_BLOCK_OFFSET);
// Process reachable code paths starting from instruction 0.
int instCount = source.instructionCount();
@@ -411,9 +426,6 @@
// Check that the last block is closed and does not fall off the end.
assert currentBlock == null;
- // Handle where a catch handler hits the same block as the fallthrough.
- handleFallthroughToCatchBlock();
-
// Verify that we have properly filled all blocks
// Must be after handle-catch (which has delayed edges),
// but before handle-exit (which does not maintain predecessor counts).
@@ -434,9 +446,8 @@
// Package up the IR code.
IRCode ir = new IRCode(options, method, blocks, valueNumberGenerator, hasDebugPositions);
- // Split critical edges to make sure that we have a place to insert phi moves if
- // necessary.
- ir.splitCriticalEdges();
+ // Verify critical edges are split so we have a place to insert phi moves if necessary.
+ assert ir.verifySplitCriticalEdges();
for (BasicBlock block : blocks) {
block.deduplicatePhis();
@@ -550,6 +561,12 @@
// Process synthesized move-exception block specially.
if (item instanceof MoveExceptionWorklistItem) {
processMoveExceptionItem((MoveExceptionWorklistItem) item);
+ closeCurrentBlockGuaranteedNotToNeedEdgeSplitting();
+ continue;
+ }
+ // Split blocks are just pending close.
+ if (item instanceof SplitBlockWorklistItem) {
+ closeCurrentBlockGuaranteedNotToNeedEdgeSplitting();
continue;
}
// Build IR for each dex instruction in the block.
@@ -571,20 +588,20 @@
private void processMoveExceptionItem(MoveExceptionWorklistItem moveExceptionItem) {
// TODO(zerny): Link with outer try-block handlers, if any. b/65203529
- int moveExceptionDest = source.getMoveExceptionRegister();
- assert moveExceptionDest >= 0;
int targetIndex = source.instructionIndex(moveExceptionItem.targetOffset);
- Value out = writeRegister(moveExceptionDest, ValueType.OBJECT, ThrowingInfo.NO_THROW, null);
- Position position = source.getDebugPositionAtOffset(moveExceptionItem.targetOffset);
- MoveException moveException = new MoveException(out);
- moveException.setPosition(position);
- currentBlock.add(moveException);
+ int moveExceptionDest = source.getMoveExceptionRegister(targetIndex);
+ if (moveExceptionDest >= 0) {
+ Value out = writeRegister(moveExceptionDest, ValueType.OBJECT, ThrowingInfo.NO_THROW, null);
+ Position position = source.getDebugPositionAtOffset(moveExceptionItem.targetOffset);
+ MoveException moveException = new MoveException(out);
+ moveException.setPosition(position);
+ currentBlock.add(moveException);
+ }
Goto exit = new Goto();
currentBlock.add(exit);
BasicBlock targetBlock = getTarget(moveExceptionItem.targetOffset);
currentBlock.link(targetBlock);
addToWorklist(targetBlock, targetIndex);
- closeCurrentBlock();
}
// Helper to resolve switch payloads and build switch instructions (dex code only).
@@ -976,11 +993,8 @@
public void addGoto(int targetOffset) {
addInstruction(new Goto());
BasicBlock targetBlock = getTarget(targetOffset);
- if (currentBlock.hasCatchSuccessor(targetBlock)) {
- needGotoToCatchBlocks.add(new BasicBlock.Pair(currentBlock, targetBlock));
- } else {
- currentBlock.link(targetBlock);
- }
+ assert !currentBlock.hasCatchSuccessor(targetBlock);
+ currentBlock.link(targetBlock);
addToWorklist(targetBlock, source.instructionIndex(targetOffset));
closeCurrentBlock();
}
@@ -1292,25 +1306,20 @@
}
public void addMoveException(int dest) {
- Value out = writeRegister(dest, ValueType.OBJECT, ThrowingInfo.NO_THROW);
- assert !out.hasLocalInfo();
- MoveException instruction = new MoveException(out);
- assert !instruction.instructionTypeCanThrow();
- if (currentBlock.getInstructions().size() == 1 && currentBlock.entry().isDebugPosition()) {
- InstructionListIterator it = currentBlock.listIterator();
- Instruction entry = it.next();
- assert entry.getPosition().equals(source.getCurrentPosition());
- attachLocalValues(instruction);
- it.replaceCurrentInstruction(instruction);
- return;
+ assert !currentBlock.getPredecessors().isEmpty();
+ assert currentBlock.getPredecessors().stream().allMatch(b -> b.entry().isMoveException());
+ assert verifyValueIsMoveException(readRegister(dest, ValueType.OBJECT));
+ }
+
+ private static boolean verifyValueIsMoveException(Value value) {
+ if (value.isPhi()) {
+ for (Value operand : value.asPhi().getOperands()) {
+ assert operand.definition.isMoveException();
+ }
+ } else {
+ assert value.definition.isMoveException();
}
- if (!currentBlock.getInstructions().isEmpty()) {
- throw new CompilationError("Invalid MoveException instruction encountered. "
- + "The MoveException instruction is not the first instruction in the block in "
- + method.qualifiedName()
- + ".");
- }
- addInstruction(instruction);
+ return true;
}
public void addMoveResult(int dest) {
@@ -1513,7 +1522,7 @@
public void addThrow(int value) {
Value in = readRegister(value, ValueType.OBJECT);
addInstruction(new Throw(in));
- closeCurrentBlock();
+ closeCurrentBlockGuaranteedNotToNeedEdgeSplitting();
}
public void addOr(NumericType type, int dest, int left, int right) {
@@ -1805,24 +1814,47 @@
if (!throwingInstructionInCurrentBlock) {
return;
}
- BasicBlock block = new BasicBlock();
+ // Note that when splitting the block in two we must update the CFG information so that we can
+ // correctly identify if the normal exits of the constructed block must be split once it is
+ // closed.
+ int currentBlockOffset = getOffset(currentBlock);
+ BlockInfo currentBlockInfo = getBlockInfo(currentBlockOffset);
+
+ BlockInfo info = new BlockInfo();
+ BasicBlock block = info.block;
block.setNumber(nextBlockNumber++);
blocks.add(block);
- block.incrementUnfilledPredecessorCount();
+
+ // Compute some unused offset for the new block and link it in the CFG.
int freshOffset = INITIAL_BLOCK_OFFSET - 1;
while (targets.containsKey(freshOffset)) {
freshOffset--;
}
- targets.put(freshOffset, null);
+ targets.put(freshOffset, info);
+ offsets.put(block, freshOffset);
+
+ // Copy over the exceptional successors.
for (int offset : source.getCurrentCatchHandlers().getUniqueTargets()) {
+ info.addExceptionalSuccessor(offset);
BlockInfo target = targets.get(offset);
assert !target.block.isSealed();
target.block.incrementUnfilledPredecessorCount();
target.addExceptionalPredecessor(freshOffset);
}
+
+ // Move all normal successors to the new block.
+ currentBlockInfo.normalSuccessors.forEach(info::addNormalSuccessor);
+ currentBlockInfo.normalSuccessors.clear();
+
+ // Link the two blocks.
addInstruction(new Goto());
currentBlock.link(block);
- closeCurrentBlock();
+ block.incrementUnfilledPredecessorCount();
+ currentBlockInfo.addNormalSuccessor(freshOffset);
+ info.addNormalPredecessor(currentBlockOffset);
+
+ // The new block can only have a single predecessor and so we don't need to split between them.
+ closeCurrentBlockGuaranteedNotToNeedEdgeSplitting();
setCurrentBlock(block);
}
@@ -1843,29 +1875,19 @@
assert !throwingInstructionInCurrentBlock;
throwingInstructionInCurrentBlock = true;
List<BasicBlock> targets = new ArrayList<>(catchHandlers.getAllTargets().size());
- int moveExceptionDest = source.getMoveExceptionRegister();
- if (moveExceptionDest < 0) {
- for (int targetOffset : catchHandlers.getAllTargets()) {
- BasicBlock target = getTarget(targetOffset);
- addToWorklist(target, source.instructionIndex(targetOffset));
- targets.add(target);
+ // Construct unique move-exception header blocks for each unique target.
+ Map<BasicBlock, BasicBlock> moveExceptionHeaders =
+ new IdentityHashMap<>(catchHandlers.getUniqueTargets().size());
+ for (int targetOffset : catchHandlers.getAllTargets()) {
+ BasicBlock target = getTarget(targetOffset);
+ BasicBlock header = moveExceptionHeaders.get(target);
+ if (header == null) {
+ header = new BasicBlock();
+ header.incrementUnfilledPredecessorCount();
+ moveExceptionHeaders.put(target, header);
+ ssaWorklist.add(new MoveExceptionWorklistItem(header, targetOffset));
}
- } else {
- // If there is a well-defined move-exception destination register (eg, compiling from
- // Java-bytecode) then we construct move-exception header blocks for each unique target.
- Map<BasicBlock, BasicBlock> moveExceptionHeaders =
- new IdentityHashMap<>(catchHandlers.getUniqueTargets().size());
- for (int targetOffset : catchHandlers.getAllTargets()) {
- BasicBlock target = getTarget(targetOffset);
- BasicBlock header = moveExceptionHeaders.get(target);
- if (header == null) {
- header = new BasicBlock();
- header.incrementUnfilledPredecessorCount();
- moveExceptionHeaders.put(target, header);
- ssaWorklist.add(new MoveExceptionWorklistItem(header, targetOffset));
- }
- targets.add(header);
- }
+ targets.add(header);
}
currentBlock.linkCatchSuccessors(catchHandlers.getGuards(), targets);
}
@@ -1907,6 +1929,7 @@
info = new BlockInfo();
}
targets.put(offset, info);
+ offsets.put(info.block, offset);
}
return info;
}
@@ -1980,11 +2003,23 @@
// Private block helpers.
+ private BlockInfo getBlockInfo(int offset) {
+ return targets.get(offset);
+ }
+
+ private BlockInfo getBlockInfo(BasicBlock block) {
+ return getBlockInfo(getOffset(block));
+ }
+
private BasicBlock getTarget(int offset) {
return targets.get(offset).block;
}
- private void closeCurrentBlock() {
+ private int getOffset(BasicBlock block) {
+ return offsets.getInt(block);
+ }
+
+ private void closeCurrentBlockGuaranteedNotToNeedEdgeSplitting() {
// TODO(zerny): To ensure liveness of locals throughout the entire block, we might want to
// insert reads before closing the block. It is unclear if we can rely on a local-end to ensure
// liveness in all blocks where the local should be live.
@@ -1994,48 +2029,41 @@
throwingInstructionInCurrentBlock = false;
}
+ private void closeCurrentBlock() {
+ assert currentBlock != null;
+ BlockInfo info = getBlockInfo(currentBlock);
+ if (!info.hasJustOneNormalExit()) {
+ // Exceptional edges are always split on construction, so no need to split edges to them.
+ for (int successorOffset : info.normalSuccessors) {
+ BlockInfo successorInfo = getBlockInfo(successorOffset);
+ if (successorInfo.predecessorCount() > 1) {
+ BasicBlock splitBlock = createSplitEdgeBlock(currentBlock, successorInfo.block);
+ ssaWorklist.add(new SplitBlockWorklistItem(splitBlock));
+ }
+ }
+ }
+ closeCurrentBlockGuaranteedNotToNeedEdgeSplitting();
+ }
+
+ private static BasicBlock createSplitEdgeBlock(BasicBlock source, BasicBlock target) {
+ BasicBlock splitBlock = new BasicBlock();
+ splitBlock.add(new Goto());
+ splitBlock.incrementUnfilledPredecessorCount();
+ splitBlock.getPredecessors().add(source);
+ splitBlock.getSuccessors().add(target);
+ source.replaceSuccessor(target, splitBlock);
+ target.replacePredecessor(source, splitBlock);
+ return splitBlock;
+ }
+
private void closeCurrentBlockWithFallThrough(BasicBlock nextBlock) {
assert currentBlock != null;
addInstruction(new Goto());
- if (currentBlock.hasCatchSuccessor(nextBlock)) {
- needGotoToCatchBlocks.add(new BasicBlock.Pair(currentBlock, nextBlock));
- } else {
- currentBlock.link(nextBlock);
- }
+ assert !currentBlock.hasCatchSuccessor(nextBlock);
+ currentBlock.link(nextBlock);
closeCurrentBlock();
}
- private void handleFallthroughToCatchBlock() {
- // When a catch handler for a block goes to the same block as the fallthrough for that
- // block the graph only has one edge there. In these cases we add an additional block so the
- // catch edge goes through that and then make the fallthrough go through a new direct edge.
- for (BasicBlock.Pair pair : needGotoToCatchBlocks) {
- BasicBlock source = pair.first;
- BasicBlock target = pair.second;
-
- // New block with one unfilled predecessor.
- BasicBlock newBlock = BasicBlock.createGotoBlock(nextBlockNumber++, target);
- blocks.add(newBlock);
- newBlock.incrementUnfilledPredecessorCount();
-
- // Link blocks.
- source.replaceSuccessor(target, newBlock);
- newBlock.getPredecessors().add(source);
- source.getSuccessors().add(target);
- target.getPredecessors().add(newBlock);
-
- // Check that the successor indexes are correct.
- assert source.hasCatchSuccessor(newBlock);
- assert !source.hasCatchSuccessor(target);
-
- // Mark the filled predecessors to the blocks.
- if (source.isFilled()) {
- newBlock.filledPredecessor(this);
- }
- target.filledPredecessor(this);
- }
- }
-
/**
* Change to control-flow graph to avoid repeated phi operands when all the same values
* flow in from multiple predecessors.
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
index 479af89..590b0c5 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
@@ -65,6 +65,7 @@
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.ListMultimap;
import java.util.ArrayList;
+import java.util.BitSet;
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
@@ -630,6 +631,11 @@
printC1VisualizerHeader(method);
printMethod(code, "Initial IR (SSA)");
+ if (method.getCode() != null && method.getCode().isJarCode() &&
+ appInfo.definitionFor(method.method.holder).hasKotlinInfo()) {
+ computeKotlinNotNullParamHints(feedback, method, code);
+ }
+
if (options.canHaveArtStringNewInitBug()) {
CodeRewriter.ensureDirectStringNewToInit(code);
}
@@ -655,7 +661,8 @@
}
if (memberValuePropagation != null) {
- memberValuePropagation.rewriteWithConstantValues(code, method.method.holder);
+ memberValuePropagation.rewriteWithConstantValues(
+ code, method.method.holder, isProcessedConcurrently);
}
if (options.enableSwitchMapRemoval && appInfo.hasLiveness()) {
codeRewriter.removeSwitchMaps(code);
@@ -775,10 +782,44 @@
}
codeRewriter.identifyParameterUsages(method, code, feedback);
+ if (options.canHaveNumberConversionRegisterAllocationBug()) {
+ codeRewriter.workaroundNumberConversionRegisterAllocationBug(code);
+ }
+
printMethod(code, "Optimized IR (SSA)");
finalizeIR(method, code, feedback);
}
+ private void computeKotlinNotNullParamHints(
+ OptimizationFeedback feedback, DexEncodedMethod method, IRCode code) {
+ // Try to infer Kotlin non-null parameter check to use it as a hint.
+ List<Value> arguments = code.collectArguments(true);
+ BitSet paramsCheckedForNull = new BitSet();
+ DexMethod checkParameterIsNotNull =
+ code.options.itemFactory.kotlin.intrinsics.checkParameterIsNotNull;
+ for (int index = 0; index < arguments.size(); index++) {
+ Value argument = arguments.get(index);
+ for (Instruction user : argument.uniqueUsers()) {
+ // To enforce parameter non-null requirement Kotlin uses intrinsic:
+ // kotlin.jvm.internal.Intrinsics.checkParameterIsNotNull(param, message)
+ //
+ // with the following we simply look if the parameter is ever passed
+ // to the mentioned intrinsic as the first argument. We do it only for
+ // code coming from Java classfile, since after the method is rewritten
+ // by R8 this call gets inlined.
+ if (!user.isInvokeStatic() ||
+ user.asInvokeMethod().getInvokedMethod() != checkParameterIsNotNull ||
+ user.inValues().indexOf(argument) != 0) {
+ continue;
+ }
+ paramsCheckedForNull.set(index);
+ }
+ }
+ if (paramsCheckedForNull.length() > 0) {
+ feedback.setKotlinNotNullParamHints(method, paramsCheckedForNull);
+ }
+ }
+
private void finalizeIR(DexEncodedMethod method, IRCode code, OptimizationFeedback feedback) {
code.traceBlocks();
if (options.isGeneratingClassFiles()) {
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/JarSourceCode.java b/src/main/java/com/android/tools/r8/ir/conversion/JarSourceCode.java
index 5c25fba..53bbb7e 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/JarSourceCode.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/JarSourceCode.java
@@ -594,7 +594,12 @@
}
@Override
- public int getMoveExceptionRegister() {
+ public int getMoveExceptionRegister(int instructionIndex) {
+ return getMoveExceptionRegister();
+ }
+
+ // In classfiles the register is always on top of stack.
+ private int getMoveExceptionRegister() {
return state.startOfStack;
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java b/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java
index 6b016b9..9a88674 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java
@@ -78,13 +78,13 @@
DexProto newMethodProto =
appInfo.dexItemFactory.applyClassMappingToProto(
callSite.methodProto, graphLense::lookupType, protoFixupCache);
- DexMethodHandle newBootstrapMethod = rewriteDexMethodHandle(method,
- callSite.bootstrapMethod);
+ DexMethodHandle newBootstrapMethod =
+ rewriteDexMethodHandle(callSite.bootstrapMethod, method);
List<DexValue> newArgs = callSite.bootstrapArgs.stream().map(
(arg) -> {
if (arg instanceof DexValueMethodHandle) {
return new DexValueMethodHandle(
- rewriteDexMethodHandle(method, ((DexValueMethodHandle) arg).value));
+ rewriteDexMethodHandle(((DexValueMethodHandle) arg).value, method));
}
return arg;
})
@@ -226,16 +226,17 @@
}
private DexMethodHandle rewriteDexMethodHandle(
- DexEncodedMethod method, DexMethodHandle methodHandle) {
+ DexMethodHandle methodHandle, DexEncodedMethod context) {
if (methodHandle.isMethodHandle()) {
DexMethod invokedMethod = methodHandle.asMethod();
+ MethodHandleType oldType = methodHandle.type;
GraphLenseLookupResult lenseLookup =
- graphLense.lookupMethod(invokedMethod, method, methodHandle.type.toInvokeType());
+ graphLense.lookupMethod(invokedMethod, context, oldType.toInvokeType());
DexMethod actualTarget = lenseLookup.getMethod();
+ MethodHandleType newType = lenseLookup.getType().toMethodHandle(actualTarget);
if (actualTarget != invokedMethod) {
- MethodHandleType newType = methodHandle.type;
DexClass clazz = appInfo.definitionFor(actualTarget.holder);
- if (clazz != null && (newType.isInvokeInterface() || newType.isInvokeInstance())) {
+ if (clazz != null && (oldType.isInvokeInterface() || oldType.isInvokeInstance())) {
newType =
lenseLookup.getType() == Type.INTERFACE
? MethodHandleType.INVOKE_INTERFACE
@@ -243,6 +244,9 @@
}
return new DexMethodHandle(newType, actualTarget);
}
+ if (oldType != newType) {
+ return new DexMethodHandle(newType, actualTarget);
+ }
} else {
DexField field = methodHandle.asField();
DexField actualField = graphLense.lookupField(field);
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedback.java b/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedback.java
index 7ed34ba..ad6d66d 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedback.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedback.java
@@ -9,6 +9,7 @@
import com.android.tools.r8.graph.DexEncodedMethod.ParameterUsagesInfo;
import com.android.tools.r8.graph.DexEncodedMethod.TrivialInitializer;
import com.android.tools.r8.ir.optimize.Inliner.Constraint;
+import java.util.BitSet;
public interface OptimizationFeedback {
void methodReturnsArgument(DexEncodedMethod method, int argument);
@@ -21,4 +22,5 @@
void setClassInlinerEligibility(DexEncodedMethod method, ClassInlinerEligibility eligibility);
void setTrivialInitializer(DexEncodedMethod method, TrivialInitializer info);
void setParameterUsages(DexEncodedMethod method, ParameterUsagesInfo parameterUsagesInfo);
+ void setKotlinNotNullParamHints(DexEncodedMethod method, BitSet hints);
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedbackDirect.java b/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedbackDirect.java
index 44588ab..326fa03 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedbackDirect.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedbackDirect.java
@@ -9,6 +9,7 @@
import com.android.tools.r8.graph.DexEncodedMethod.ParameterUsagesInfo;
import com.android.tools.r8.graph.DexEncodedMethod.TrivialInitializer;
import com.android.tools.r8.ir.optimize.Inliner.Constraint;
+import java.util.BitSet;
public class OptimizationFeedbackDirect implements OptimizationFeedback {
@@ -62,4 +63,9 @@
public void setParameterUsages(DexEncodedMethod method, ParameterUsagesInfo parameterUsagesInfo) {
method.setParameterUsages(parameterUsagesInfo);
}
+
+ @Override
+ public void setKotlinNotNullParamHints(DexEncodedMethod method, BitSet hints) {
+ method.setKotlinNotNullParamHints(hints);
+ }
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedbackIgnore.java b/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedbackIgnore.java
index c2d6ede..359c90d 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedbackIgnore.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedbackIgnore.java
@@ -9,6 +9,7 @@
import com.android.tools.r8.graph.DexEncodedMethod.ParameterUsagesInfo;
import com.android.tools.r8.graph.DexEncodedMethod.TrivialInitializer;
import com.android.tools.r8.ir.optimize.Inliner.Constraint;
+import java.util.BitSet;
public class OptimizationFeedbackIgnore implements OptimizationFeedback {
@@ -45,4 +46,8 @@
@Override
public void setParameterUsages(DexEncodedMethod method, ParameterUsagesInfo parameterUsagesInfo) {
}
+
+ @Override
+ public void setKotlinNotNullParamHints(DexEncodedMethod method, BitSet hints) {
+ }
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/SourceCode.java b/src/main/java/com/android/tools/r8/ir/conversion/SourceCode.java
index e95790a..bf2b51f 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/SourceCode.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/SourceCode.java
@@ -59,7 +59,8 @@
void resolveAndBuildNewArrayFilledData(int arrayRef, int payloadOffset, IRBuilder builder);
CatchHandlers<Integer> getCurrentCatchHandlers();
- int getMoveExceptionRegister();
+
+ int getMoveExceptionRegister(int instructionIndex);
// For debugging/verification purpose.
boolean verifyRegister(int register);
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 7ea7f57..5066f87 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
@@ -63,6 +63,7 @@
import com.android.tools.r8.ir.code.InvokeDirect;
import com.android.tools.r8.ir.code.InvokeMethod;
import com.android.tools.r8.ir.code.InvokeNewArray;
+import com.android.tools.r8.ir.code.InvokeStatic;
import com.android.tools.r8.ir.code.InvokeVirtual;
import com.android.tools.r8.ir.code.MemberType;
import com.android.tools.r8.ir.code.NewArrayEmpty;
@@ -755,7 +756,9 @@
//
// (1) as a receiver of reads/writes of instance fields of the holder class
// (2) as a return value
- // (3) as a receiver of a call to the superclass initializer
+ // (3) as a receiver of a call to the superclass initializer. Note that we don't
+ // check what is passed to superclass initializer as arguments, only check
+ // that it is not the instance being initialized.
//
boolean instanceInitializer = method.isInstanceInitializer();
if (method.accessFlags.isNative() ||
@@ -787,23 +790,25 @@
(insn.isInstancePut() && insn.asInstancePut().object() == receiver)) {
DexField field = insn.asFieldInstruction().getField();
if (field.clazz == clazz.type && clazz.lookupInstanceField(field) != null) {
- // Since class inliner currently only supports classes directly extending
- // java.lang.Object, we don't need to worry about fields defined in superclasses.
+ // Require only accessing instance fields of the *current* class.
continue;
}
return;
}
- // If this is an instance initializer allow one call
- // to java.lang.Object.<init>() on 'this'.
- if (instanceInitializer && insn.isInvokeDirect()) {
+ // If this is an instance initializer allow one call to superclass instance initializer.
+ if (insn.isInvokeDirect()) {
InvokeDirect invokedDirect = insn.asInvokeDirect();
- if (invokedDirect.getInvokedMethod() == dexItemFactory.objectMethods.constructor &&
- invokedDirect.getReceiver() == receiver &&
- !seenSuperInitCall) {
+ DexMethod invokedMethod = invokedDirect.getInvokedMethod();
+ if (dexItemFactory.isConstructor(invokedMethod) &&
+ invokedMethod.holder == clazz.superType &&
+ invokedDirect.inValues().lastIndexOf(receiver) == 0 &&
+ !seenSuperInitCall &&
+ instanceInitializer) {
seenSuperInitCall = true;
continue;
}
+ // We don't support other direct calls yet.
return;
}
@@ -870,7 +875,7 @@
// This method defines trivial instance initializer as follows:
//
- // ** The initializer may only call the initializer of the base class, which
+ // ** The initializer may call the initializer of the base class, which
// itself must be trivial.
//
// ** java.lang.Object.<init>() is considered trivial.
@@ -3004,4 +3009,61 @@
}
}
}
+
+ // See comment for InternalOptions.canHaveNumberConversionRegisterAllocationBug().
+ public void workaroundNumberConversionRegisterAllocationBug(IRCode code) {
+ final Supplier<DexMethod> javaLangDoubleisNaN = Suppliers.memoize(() ->
+ dexItemFactory.createMethod(
+ dexItemFactory.createString("Ljava/lang/Double;"),
+ dexItemFactory.createString("isNaN"),
+ dexItemFactory.booleanDescriptor,
+ new DexString[]{dexItemFactory.doubleDescriptor}));
+
+ ListIterator<BasicBlock> blocks = code.listIterator();
+ while (blocks.hasNext()) {
+ BasicBlock block = blocks.next();
+ InstructionListIterator it = block.listIterator();
+ while (it.hasNext()) {
+ Instruction instruction = it.next();
+ if (instruction.isArithmeticBinop() || instruction.isNeg()) {
+ for (Value value : instruction.inValues()) {
+ // Insert a call to Double.isNaN on each value which come from a number conversion
+ // to double and flows into an arithmetic instruction. This seems to break the traces
+ // in the Dalvik JIT and avoid the bug where the generated ARM code can clobber float
+ // values in a single-precision registers with double values written to
+ // double-precision registers. See b/77496850 for examples.
+ if (!value.isPhi()
+ && value.definition.isNumberConversion()
+ && value.definition.asNumberConversion().to == NumericType.DOUBLE) {
+ Value newValue = code.createValue(
+ instruction.outValue().outType(), instruction.getLocalInfo());
+ InvokeStatic invokeIsNaN =
+ new InvokeStatic(javaLangDoubleisNaN.get(), newValue, ImmutableList.of(value));
+ invokeIsNaN.setPosition(instruction.getPosition());
+
+ // Insert the invoke before the current instruction.
+ it.previous();
+ BasicBlock blockWithInvokeNaN =
+ block.hasCatchHandlers() ? it.split(code, blocks) : block;
+ if (blockWithInvokeNaN != block) {
+ // If we split, add the invoke at the end of the original block.
+ it = block.listIterator(block.getInstructions().size());
+ it.previous();
+ it.add(invokeIsNaN);
+ // Continue iteration in the split block.
+ block = blockWithInvokeNaN;
+ it = block.listIterator();
+ } else {
+ // Otherwise, add it to the current block.
+ it.add(invokeIsNaN);
+ }
+ // Skip over the instruction causing the invoke to be inserted.
+ Instruction temp = it.next();
+ assert temp == instruction;
+ }
+ }
+ }
+ }
+ }
+ }
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java b/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
index b05fd4f..cc7f571 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
@@ -14,11 +14,14 @@
import com.android.tools.r8.ir.code.InvokeMethodWithReceiver;
import com.android.tools.r8.ir.code.InvokePolymorphic;
import com.android.tools.r8.ir.code.InvokeStatic;
+import com.android.tools.r8.ir.code.Value;
import com.android.tools.r8.ir.conversion.CallSiteInformation;
import com.android.tools.r8.ir.optimize.Inliner.InlineAction;
import com.android.tools.r8.ir.optimize.Inliner.Reason;
import com.android.tools.r8.logging.Log;
import com.android.tools.r8.utils.InternalOptions;
+import java.util.BitSet;
+import java.util.List;
import java.util.ListIterator;
import java.util.function.Predicate;
@@ -218,13 +221,33 @@
if (reason == Reason.SIMPLE) {
// If we are looking for a simple method, only inline if actually simple.
Code code = candidate.getCode();
- if (!code.estimatedSizeForInliningAtMost(inliningInstructionLimit)) {
+ int instructionLimit = computeInstructionLimit(invoke, candidate);
+ if (!code.estimatedSizeForInliningAtMost(instructionLimit)) {
return false;
}
}
return true;
}
+ private int computeInstructionLimit(InvokeMethod invoke, DexEncodedMethod candidate) {
+ int instructionLimit = inliningInstructionLimit;
+ BitSet hints = candidate.getOptimizationInfo().getKotlinNotNullParamHints();
+ if (hints != null) {
+ List<Value> arguments = invoke.inValues();
+ if (invoke.isInvokeMethodWithReceiver()) {
+ arguments = arguments.subList(1, arguments.size());
+ }
+ for (int index = 0; index < arguments.size(); index++) {
+ Value argument = arguments.get(index);
+ if (argument.isNeverNull() && hints.get(index)) {
+ // 5-4 instructions per parameter check are expected to be removed.
+ instructionLimit += 4;
+ }
+ }
+ }
+ return instructionLimit;
+ }
+
@Override
public InlineAction computeForInvokeWithReceiver(
InvokeMethodWithReceiver invoke, DexType invocationContext) {
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java b/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java
index cdfa2b1..f92eb80 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java
@@ -7,6 +7,8 @@
import com.android.tools.r8.graph.DexClass;
import com.android.tools.r8.graph.DexEncodedField;
import com.android.tools.r8.graph.DexEncodedMethod;
+import com.android.tools.r8.graph.DexEncodedMethod.TrivialInitializer;
+import com.android.tools.r8.graph.DexEncodedMethod.TrivialInitializer.TrivialClassInitializer;
import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexItem;
import com.android.tools.r8.graph.DexMethod;
@@ -23,6 +25,7 @@
import com.android.tools.r8.ir.code.ValueType;
import com.android.tools.r8.shaking.Enqueuer.AppInfoWithLiveness;
import com.android.tools.r8.shaking.ProguardMemberRule;
+import java.util.function.Predicate;
public class MemberValuePropagation {
@@ -113,7 +116,8 @@
* <p>
* Also assigns value ranges to values where possible.
*/
- public void rewriteWithConstantValues(IRCode code, DexType callingContext) {
+ public void rewriteWithConstantValues(
+ IRCode code, DexType callingContext, Predicate<DexEncodedMethod> isProcessedConcurrently) {
InstructionIterator iterator = code.instructionIterator();
while (iterator.hasNext()) {
Instruction current = iterator.next();
@@ -155,7 +159,7 @@
if (!invokeReplaced && invoke.outValue() != null) {
DexEncodedMethod target = invoke.computeSingleTarget(appInfo);
if (target != null) {
- if (target.getOptimizationInfo().neverReturnsNull()) {
+ if (target.getOptimizationInfo().neverReturnsNull() && invoke.outValue().canBeNull()) {
invoke.outValue().markNeverNull();
}
if (target.getOptimizationInfo().returnsConstant()) {
@@ -182,12 +186,11 @@
} else if (current.isStaticGet()) {
StaticGet staticGet = current.asStaticGet();
DexField field = staticGet.getField();
- Instruction replacement = null;
DexEncodedField target = appInfo.lookupStaticTarget(field.getHolder(), field);
ProguardMemberRuleLookup lookup = null;
if (target != null) {
// Check if a this value is known const.
- replacement = target.valueAsConstInstruction(appInfo, staticGet.dest());
+ Instruction replacement = target.valueAsConstInstruction(appInfo, staticGet.dest());
if (replacement == null) {
lookup = lookupMemberRule(target);
if (lookup != null) {
@@ -207,6 +210,37 @@
} else {
iterator.replaceCurrentInstruction(replacement);
}
+ } else if (staticGet.dest() != null) {
+ // In case the class holder of this static field satisfying following criteria:
+ // -- cannot trigger other static initializer except for its own
+ // -- is final
+ // -- has a class initializer which is classified as trivial
+ // (see CodeRewriter::computeClassInitializerInfo) and
+ // initializes the field being accessed
+ //
+ // ... and the field itself is not pinned by keep rules (in which case it might
+ // be updated outside the class constructor, e.g. via reflections), it is safe
+ // to assume that the static-get instruction reads the value it initialized value
+ // in class initializer and is never null.
+ //
+ DexClass holderDefinition = appInfo.definitionFor(field.getHolder());
+ if (holderDefinition != null &&
+ holderDefinition.accessFlags.isFinal() &&
+ !appInfo.canTriggerStaticInitializer(field.getHolder(), true)) {
+
+ Value outValue = staticGet.dest();
+ DexEncodedMethod classInitializer = holderDefinition.getClassInitializer();
+ if (classInitializer != null && !isProcessedConcurrently.test(classInitializer)) {
+ TrivialInitializer info =
+ classInitializer.getOptimizationInfo().getTrivialInitializerInfo();
+ if (info != null &&
+ ((TrivialClassInitializer) info).field == field &&
+ !appInfo.isPinned(field) &&
+ outValue.canBeNull()) {
+ outValue.markNeverNull();
+ }
+ }
+ }
}
}
} else if (current.isStaticPut()) {
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/Outliner.java b/src/main/java/com/android/tools/r8/ir/optimize/Outliner.java
index 6cb8f6e..e749479 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/Outliner.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/Outliner.java
@@ -1099,7 +1099,7 @@
}
@Override
- public int getMoveExceptionRegister() {
+ public int getMoveExceptionRegister(int instructionIndex) {
throw new Unreachable();
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java
index b28158a..219b5b1 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java
@@ -16,6 +16,7 @@
import com.android.tools.r8.ir.optimize.InliningOracle;
import com.android.tools.r8.shaking.Enqueuer.AppInfoWithLiveness;
import com.google.common.collect.Streams;
+import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
@@ -127,31 +128,51 @@
.filter(insn -> insn.isNewInstance() || insn.isStaticGet())
.collect(Collectors.toList());
- for (Instruction root : roots) {
- InlineCandidateProcessor processor =
- new InlineCandidateProcessor(factory, appInfo,
- type -> isClassEligible(appInfo, type),
- isProcessedConcurrently, method, root);
+ // We loop inlining iterations until there was no inlining, but still use same set
+ // of roots to avoid infinite inlining. Looping makes possible for some roots to
+ // become eligible after other roots are inlined.
- // Assess eligibility and compute inlining of direct calls and extra methods needed.
- if (!processor.isInstanceEligible() ||
- !processor.isClassAndUsageEligible() ||
- !processor.areInstanceUsersEligible(defaultOracle)) {
- continue;
+ boolean repeat;
+ do {
+ repeat = false;
+
+ Iterator<Instruction> rootsIterator = roots.iterator();
+ while (rootsIterator.hasNext()) {
+ Instruction root = rootsIterator.next();
+ InlineCandidateProcessor processor =
+ new InlineCandidateProcessor(factory, appInfo,
+ type -> isClassEligible(appInfo, type),
+ isProcessedConcurrently, method, root);
+
+ // Assess eligibility of instance and class.
+ if (!processor.isInstanceEligible() ||
+ !processor.isClassAndUsageEligible()) {
+ // This root will never be inlined.
+ rootsIterator.remove();
+ continue;
+ }
+
+ // Assess users eligibility and compute inlining of direct calls and extra methods needed.
+ if (!processor.areInstanceUsersEligible(defaultOracle)) {
+ // This root may succeed if users change in future.
+ continue;
+ }
+
+ // Is inlining allowed.
+ if (processor.getEstimatedCombinedSizeForInlining() >= totalMethodInstructionLimit) {
+ continue;
+ }
+
+ // Inline the class instance.
+ processor.processInlining(code, inliner);
+
+ // Restore normality.
+ code.removeAllTrivialPhis();
+ assert code.isConsistentSSA();
+ rootsIterator.remove();
+ repeat = true;
}
-
- // Is inlining allowed.
- if (processor.getEstimatedCombinedSizeForInlining() >= totalMethodInstructionLimit) {
- continue;
- }
-
- // Inline the class instance.
- processor.processInlining(code, inliner);
-
- // Restore normality.
- code.removeAllTrivialPhis();
- assert code.isConsistentSSA();
- }
+ } while (repeat);
}
private boolean isClassEligible(AppInfo appInfo, DexType clazz) {
@@ -167,7 +188,6 @@
// Class is eligible for this optimization. Eligibility implementation:
// - is not an abstract class or interface
- // - directly extends java.lang.Object
// - does not declare finalizer
// - does not trigger any static initializers except for its own
private boolean computeClassEligible(AppInfo appInfo, DexType clazz) {
@@ -177,11 +197,6 @@
return false;
}
- // Must directly extend Object.
- if (definition.superType != factory.objectType) {
- return false;
- }
-
// Class must not define finalizer.
for (DexEncodedMethod method : definition.virtualMethods()) {
if (method.method.name == factory.finalizeMethodName &&
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
index 9c553bd..4d6e3b1 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
@@ -14,6 +14,7 @@
import com.android.tools.r8.graph.DexEncodedMethod.ParameterUsagesInfo.SingleCallOfArgumentMethod;
import com.android.tools.r8.graph.DexEncodedMethod.TrivialInitializer;
import com.android.tools.r8.graph.DexEncodedMethod.TrivialInitializer.TrivialClassInitializer;
+import com.android.tools.r8.graph.DexEncodedMethod.TrivialInitializer.TrivialInstanceInitializer;
import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexMethod;
@@ -21,6 +22,7 @@
import com.android.tools.r8.ir.code.BasicBlock;
import com.android.tools.r8.ir.code.ConstNumber;
import com.android.tools.r8.ir.code.IRCode;
+import com.android.tools.r8.ir.code.If;
import com.android.tools.r8.ir.code.InstanceGet;
import com.android.tools.r8.ir.code.Instruction;
import com.android.tools.r8.ir.code.Invoke.Type;
@@ -114,6 +116,10 @@
}
if (isNewInstance()) {
+ // NOTE: if the eligible class does not directly extend java.lang.Object,
+ // we also have to guarantee that it is initialized with initializer classified as
+ // TrivialInstanceInitializer. This will be checked in areInstanceUsersEligible(...).
+
// There must be no static initializer on the class itself.
return !eligibleClassDefinition.hasClassInitializer();
}
@@ -238,6 +244,16 @@
}
}
+ // Allow some IF instructions.
+ if (user.isIf()) {
+ If ifInsn = user.asIf();
+ If.Type type = ifInsn.getType();
+ if (ifInsn.isZeroTest() && (type == If.Type.EQ || type == If.Type.NE)) {
+ // Allow ==/!= null tests, we know that the instance is a non-null value.
+ continue;
+ }
+ }
+
return false; // Not eligible.
}
return true;
@@ -256,7 +272,8 @@
replaceUsagesAsUnusedArgument(code);
forceInlineExtraMethodInvocations(inliner);
forceInlineDirectMethodInvocations(inliner);
- removeSuperClassInitializerAndFieldReads(code);
+ removeMiscUsages(code);
+ removeFieldReads(code);
removeFieldWrites();
removeInstruction(root);
}
@@ -308,19 +325,58 @@
}
}
- // Remove call to superclass initializer, replace field reads with appropriate
- // values, insert phis when needed.
- private void removeSuperClassInitializerAndFieldReads(IRCode code) {
- Map<DexField, FieldValueHelper> fieldHelpers = new IdentityHashMap<>();
+ // Remove miscellaneous users before handling field reads.
+ private void removeMiscUsages(IRCode code) {
+ boolean needToRemoveUnreachableBlocks = false;
for (Instruction user : eligibleInstance.uniqueUsers()) {
// Remove the call to superclass constructor.
if (root.isNewInstance() &&
user.isInvokeDirect() &&
- user.asInvokeDirect().getInvokedMethod() == factory.objectMethods.constructor) {
+ factory.isConstructor(user.asInvokeDirect().getInvokedMethod()) &&
+ user.asInvokeDirect().getInvokedMethod().holder == eligibleClassDefinition.superType) {
removeInstruction(user);
continue;
}
+ if (user.isIf()) {
+ If ifInsn = user.asIf();
+ assert ifInsn.isZeroTest()
+ : "Unexpected usage in non-zero-test IF instruction: " + user;
+ BasicBlock block = user.getBlock();
+ If.Type type = ifInsn.getType();
+ assert type == If.Type.EQ || type == If.Type.NE
+ : "Unexpected type in zero-test IF instruction: " + user;
+ BasicBlock newBlock = type == If.Type.EQ
+ ? ifInsn.fallthroughBlock() : ifInsn.getTrueTarget();
+ BasicBlock blockToRemove = type == If.Type.EQ
+ ? ifInsn.getTrueTarget() : ifInsn.fallthroughBlock();
+ assert newBlock != blockToRemove;
+
+ block.replaceSuccessor(blockToRemove, newBlock);
+ blockToRemove.removePredecessor(block);
+ assert block.exit().isGoto();
+ assert block.exit().asGoto().getTarget() == newBlock;
+ needToRemoveUnreachableBlocks = true;
+ continue;
+ }
+
+ if (user.isInstanceGet() || user.isInstancePut()) {
+ // Leave field reads/writes until next steps.
+ continue;
+ }
+
+ throw new Unreachable("Unexpected usage left after method inlining: " + user);
+ }
+
+ if (needToRemoveUnreachableBlocks) {
+ code.removeUnreachableBlocks();
+ }
+ }
+
+ // Replace field reads with appropriate values, insert phis when needed.
+ private void removeFieldReads(IRCode code) {
+ Map<DexField, FieldValueHelper> fieldHelpers = new IdentityHashMap<>();
+ for (Instruction user : eligibleInstance.uniqueUsers()) {
if (user.isInstanceGet()) {
// Replace a field read with appropriate value.
replaceFieldRead(code, user.asInstanceGet(), fieldHelpers);
@@ -385,6 +441,19 @@
return null;
}
+ // If the superclass of the initializer is NOT java.lang.Object, the super class
+ // initializer being called must be classified as TrivialInstanceInitializer.
+ //
+ // NOTE: since we already classified the class as eligible, it does not have
+ // any class initializers in superclass chain or in superinterfaces, see
+ // details in ClassInliner::computeClassEligible(...).
+ if (eligibleClassDefinition.superType != factory.objectType) {
+ TrivialInitializer info = definition.getOptimizationInfo().getTrivialInitializerInfo();
+ if (!(info instanceof TrivialInstanceInitializer)) {
+ return null;
+ }
+ }
+
if (!definition.isInliningCandidate(method, Reason.SIMPLE, appInfo)) {
// We won't be able to inline it here.
diff --git a/src/main/java/com/android/tools/r8/ir/regalloc/RegisterMoveScheduler.java b/src/main/java/com/android/tools/r8/ir/regalloc/RegisterMoveScheduler.java
index d1fe04c..031d161 100644
--- a/src/main/java/com/android/tools/r8/ir/regalloc/RegisterMoveScheduler.java
+++ b/src/main/java/com/android/tools/r8/ir/regalloc/RegisterMoveScheduler.java
@@ -14,6 +14,8 @@
import com.android.tools.r8.ir.code.Move;
import com.android.tools.r8.ir.code.Position;
import com.android.tools.r8.ir.code.Value;
+import it.unimi.dsi.fastutil.ints.IntArraySet;
+import it.unimi.dsi.fastutil.ints.IntSet;
import java.util.ArrayList;
import java.util.Deque;
import java.util.HashMap;
@@ -59,6 +61,8 @@
}
public void schedule() {
+ assert everyDestinationOnlyWrittenOnce();
+
// Worklist of moves that are ready to be inserted.
Deque<RegisterMove> worklist = new LinkedList<>();
@@ -196,4 +200,13 @@
iterator.remove();
return move;
}
+
+ private boolean everyDestinationOnlyWrittenOnce() {
+ IntSet destinations = new IntArraySet(moveSet.size());
+ for (RegisterMove move : moveSet) {
+ boolean changed = destinations.add(move.dst);
+ assert changed;
+ }
+ return true;
+ }
}
diff --git a/src/main/java/com/android/tools/r8/ir/regalloc/SpillMoveSet.java b/src/main/java/com/android/tools/r8/ir/regalloc/SpillMoveSet.java
index 1ea788d..cebcc8c 100644
--- a/src/main/java/com/android/tools/r8/ir/regalloc/SpillMoveSet.java
+++ b/src/main/java/com/android/tools/r8/ir/regalloc/SpillMoveSet.java
@@ -61,7 +61,46 @@
assert to.getSplitParent() == from.getSplitParent();
BasicBlock atEntryToBlock = blockStartMap.get(i + 1);
if (atEntryToBlock == null) {
- addInMove(i, to, from);
+ Value value = from.getValue();
+ if (value.definition != null
+ && value.definition.isMoveException()
+ && to.getStart() == value.definition.asMoveException().getNumber() + 1) {
+ // Consider the following IR code.
+ // 40: ...
+ // 42: v0 <- move-exception
+ // 44: ...
+ // ...
+ // 50: ... v0 ... // 4-bit constrained use of v0
+ //
+ // Initially, the liveness interval of v0 is [42; 50[. If the method has overlapping move-
+ // exception intervals (and the register allocator needs more than 16 registers), then the
+ // intervals of v0 will be split immediately after its definition. Therefore, v0 will have
+ // two liveness intervals: I1=[42; 43[ and I2=[43;50[.
+ //
+ // When allocating a register for the interval I2, we may need to spill an existing value v1
+ // that is currently active. As a result, we will split the liveness interval of v1 before
+ // the start of I2 (i.e., at position 43). The live intervals of v1 after the split
+ // therefore becomes J1=[x, y[, J2=[41, 43[, J3=[43, z[.
+ //
+ // If the registers assigned to J1 and J2 are different, we will create an in-move at
+ // position 43 in resolveControlFlow() (not position 41, since the first instruction of the
+ // target block is a move-exception instruction). If the the registers assigned to I1 and I2
+ // are also different, then an in-move will also be created at position 43 by the call to
+ // addSpillOrRestoreMove() in insertMoves().
+ //
+ // If the registers of I2 and J2 are the same (which is a valid assignment), then we will
+ // do parallel move scheduling for the following two in-moves:
+ // move X, reg(I2)
+ // move X, reg(J2)
+ //
+ // Therefore, there is a risk that we end up with the value that has been spilled instead of
+ // the exception object in register X at position 44. To avoid this situation, we schedule
+ // the move of the exception object (in this case, "move X, reg(I2)") as an out-move, such
+ // that it always gets inserted *after* the resolution moves of the current block.
+ addOutMove(i, to, from);
+ } else {
+ addInMove(i, to, from);
+ }
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/synthetic/SyntheticSourceCode.java b/src/main/java/com/android/tools/r8/ir/synthetic/SyntheticSourceCode.java
index 80a439a..c64d8ed 100644
--- a/src/main/java/com/android/tools/r8/ir/synthetic/SyntheticSourceCode.java
+++ b/src/main/java/com/android/tools/r8/ir/synthetic/SyntheticSourceCode.java
@@ -209,7 +209,7 @@
}
@Override
- public int getMoveExceptionRegister() {
+ public int getMoveExceptionRegister(int instructionIndex) {
throw new Unreachable();
}
diff --git a/src/main/java/com/android/tools/r8/kotlin/Kotlin.java b/src/main/java/com/android/tools/r8/kotlin/Kotlin.java
index 0af41b1..05c7b92 100644
--- a/src/main/java/com/android/tools/r8/kotlin/Kotlin.java
+++ b/src/main/java/com/android/tools/r8/kotlin/Kotlin.java
@@ -80,6 +80,9 @@
public final DexType type = factory.createType("Lkotlin/jvm/internal/Intrinsics;");
public final DexMethod throwParameterIsNullException = factory.createMethod(type,
factory.createProto(factory.voidType, factory.stringType), "throwParameterIsNullException");
+ public final DexMethod checkParameterIsNotNull = factory.createMethod(type,
+ factory.createProto(factory.voidType, factory.objectType, factory.stringType),
+ "checkParameterIsNotNull");
public final DexMethod throwNpe = factory.createMethod(
type, factory.createProto(factory.voidType), "throwNpe");
}
diff --git a/src/main/java/com/android/tools/r8/naming/ProguardMapApplier.java b/src/main/java/com/android/tools/r8/naming/ProguardMapApplier.java
index c159b9d..f5d48fd 100644
--- a/src/main/java/com/android/tools/r8/naming/ProguardMapApplier.java
+++ b/src/main/java/com/android/tools/r8/naming/ProguardMapApplier.java
@@ -149,27 +149,25 @@
}
});
- // We need to handle a lib class that extends another lib class where some members are not
- // overridden, resulting in absence of definitions. References to those members need to be
- // redirected via lense as well.
- if (clazz.isLibraryClass()) {
- classNaming.forAllFieldNaming(memberNaming -> {
- if (!appliedMemberNaming.contains(memberNaming)) {
- DexField pretendedOriginalField =
- ((FieldSignature) memberNaming.getOriginalSignature())
- .toDexField(appInfo.dexItemFactory, from);
- applyFieldMapping(pretendedOriginalField, memberNaming);
- }
- });
- classNaming.forAllMethodNaming(memberNaming -> {
- if (!appliedMemberNaming.contains(memberNaming)) {
- DexMethod pretendedOriginalMethod =
- ((MethodSignature) memberNaming.getOriginalSignature())
- .toDexMethod(appInfo.dexItemFactory, from);
- applyMethodMapping(pretendedOriginalMethod, memberNaming);
- }
- });
- }
+ // We need to handle a class that extends another class where some members are not overridden,
+ // resulting in absence of definitions. References to those members need to be redirected via
+ // the lense as well.
+ classNaming.forAllFieldNaming(memberNaming -> {
+ if (!appliedMemberNaming.contains(memberNaming)) {
+ DexField pretendedOriginalField =
+ ((FieldSignature) memberNaming.getOriginalSignature())
+ .toDexField(appInfo.dexItemFactory, from);
+ applyFieldMapping(pretendedOriginalField, memberNaming);
+ }
+ });
+ classNaming.forAllMethodNaming(memberNaming -> {
+ if (!appliedMemberNaming.contains(memberNaming)) {
+ DexMethod pretendedOriginalMethod =
+ ((MethodSignature) memberNaming.getOriginalSignature())
+ .toDexMethod(appInfo.dexItemFactory, from);
+ applyMethodMapping(pretendedOriginalMethod, memberNaming);
+ }
+ });
}
private void applyFieldMapping(DexField originalField, MemberNaming memberNaming) {
diff --git a/src/main/java/com/android/tools/r8/optimize/ClassAndMemberPublicizer.java b/src/main/java/com/android/tools/r8/optimize/ClassAndMemberPublicizer.java
index 297dd57..eaeb1ee 100644
--- a/src/main/java/com/android/tools/r8/optimize/ClassAndMemberPublicizer.java
+++ b/src/main/java/com/android/tools/r8/optimize/ClassAndMemberPublicizer.java
@@ -3,65 +3,179 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.optimize;
+import com.android.tools.r8.graph.AppInfo;
import com.android.tools.r8.graph.DexApplication;
import com.android.tools.r8.graph.DexClass;
import com.android.tools.r8.graph.DexEncodedMethod;
-import com.android.tools.r8.graph.DexItemFactory;
+import com.android.tools.r8.graph.DexMethod;
+import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.graph.GraphLense;
import com.android.tools.r8.graph.MethodAccessFlags;
+import com.android.tools.r8.optimize.PublicizerLense.PublicizedLenseBuilder;
+import com.android.tools.r8.shaking.RootSetBuilder.RootSet;
+import com.android.tools.r8.utils.MethodSignatureEquivalence;
+import com.android.tools.r8.utils.ThreadUtils;
+import com.android.tools.r8.utils.Timing;
+import com.google.common.base.Equivalence;
+import com.google.common.base.Equivalence.Wrapper;
+import com.google.common.collect.Sets;
+import java.util.ArrayList;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
public final class ClassAndMemberPublicizer {
- private final DexItemFactory factory;
private final DexApplication application;
+ private final AppInfo appInfo;
+ private final RootSet rootSet;
+ private final GraphLense previuosLense;
+ private final PublicizedLenseBuilder lenseBuilder;
- private ClassAndMemberPublicizer(DexApplication application, DexItemFactory factory) {
+ private final Equivalence<DexMethod> equivalence = MethodSignatureEquivalence.get();
+ // TODO(b/72109068): finer-grained naming spaces, e.g., per-tree.
+ private final Set<Wrapper<DexMethod>> methodPool = Sets.newConcurrentHashSet();
+
+ private ClassAndMemberPublicizer(
+ DexApplication application,
+ AppInfo appInfo,
+ RootSet rootSet,
+ GraphLense previousLense) {
this.application = application;
- this.factory = factory;
+ this.appInfo = appInfo;
+ this.rootSet = rootSet;
+ this.previuosLense = previousLense;
+ lenseBuilder = PublicizerLense.createBuilder();
}
/**
- * Marks all package private and protected methods and fields as public. Also makes
- * all private static methods public.
+ * Marks all package private and protected methods and fields as public.
+ * Makes all private static methods public.
+ * Makes private instance methods public final instance methods, if possible.
* <p>
* This will destructively update the DexApplication passed in as argument.
*/
- public static DexApplication run(DexApplication application, DexItemFactory factory) {
- return new ClassAndMemberPublicizer(application, factory).run();
+ public static GraphLense run(
+ ExecutorService executorService,
+ Timing timing,
+ DexApplication application,
+ AppInfo appInfo,
+ RootSet rootSet,
+ GraphLense previousLense) throws ExecutionException {
+ return new ClassAndMemberPublicizer(application, appInfo, rootSet, previousLense)
+ .run(executorService, timing);
}
- private DexApplication run() {
- for (DexClass clazz : application.classes()) {
+ private GraphLense run(ExecutorService executorService, Timing timing)
+ throws ExecutionException {
+ // Phase 1: Collect methods to check if private instance methods don't have conflicts.
+ timing.begin("Phase 1: collectMethods");
+ try {
+ List<Future<?>> futures = new ArrayList<>();
+ // TODO(b/72109068): finer-grained naming spaces will need a different class visiting.
+ application.classes().forEach(clazz ->
+ futures.add(executorService.submit(collectMethodPerClass(clazz))));
+ ThreadUtils.awaitFutures(futures);
+ } finally {
+ timing.end();
+ }
+
+ // Phase 2: Visit classes and promote class/member to public if possible.
+ timing.begin("Phase 2: promoteToPublic");
+ DexType.forAllInterfaces(appInfo.dexItemFactory, this::publicizeType);
+ publicizeType(appInfo.dexItemFactory.objectType);
+ timing.end();
+
+ return lenseBuilder.build(appInfo, previuosLense);
+ }
+
+ private Runnable collectMethodPerClass(DexClass clazz) {
+ return () -> {
+ clazz.forEachMethod(encodedMethod -> {
+ // We will add private instance methods when we promote them.
+ if (!encodedMethod.isPrivateMethod() || encodedMethod.isStaticMethod()) {
+ methodPool.add(equivalence.wrap(encodedMethod.method));
+ }
+ });
+ };
+ }
+
+ private void publicizeType(DexType type) {
+ DexClass clazz = application.definitionFor(type);
+ if (clazz != null && clazz.isProgramClass()) {
clazz.accessFlags.promoteToPublic();
- clazz.forEachMethod(this::publicizeMethod);
clazz.forEachField(field -> field.accessFlags.promoteToPublic());
+ Set<DexEncodedMethod> privateInstanceEncodedMethods = new LinkedHashSet<>();
+ clazz.forEachMethod(encodedMethod -> {
+ if (publicizeMethod(clazz, encodedMethod)) {
+ privateInstanceEncodedMethods.add(encodedMethod);
+ }
+ });
+ if (!privateInstanceEncodedMethods.isEmpty()) {
+ clazz.virtualizeMethods(privateInstanceEncodedMethods);
+ }
}
- return application;
+
+ // TODO(b/72109068): Can process sub types in parallel.
+ type.forAllExtendsSubtypes(this::publicizeType);
}
- private void publicizeMethod(DexEncodedMethod method) {
- MethodAccessFlags accessFlags = method.accessFlags;
+ private boolean publicizeMethod(DexClass holder, DexEncodedMethod encodedMethod) {
+ MethodAccessFlags accessFlags = encodedMethod.accessFlags;
if (accessFlags.isPublic()) {
- return;
+ return false;
}
- if (factory.isClassConstructor(method.method)) {
- return;
+ if (appInfo.dexItemFactory.isClassConstructor(encodedMethod.method)) {
+ return false;
}
if (!accessFlags.isPrivate()) {
accessFlags.unsetProtected();
accessFlags.setPublic();
- return;
+ return false;
}
assert accessFlags.isPrivate();
- if (factory.isConstructor(method.method)) {
- // TODO: b/72211928
- return;
+ if (appInfo.dexItemFactory.isConstructor(encodedMethod.method)) {
+ // TODO(b/72211928)
+ return false;
}
if (!accessFlags.isStatic()) {
- // TODO: b/72109068
- return;
+ // If this method is mentioned in keep rules, do not transform (rule applications changed).
+ if (rootSet.noShrinking.containsKey(encodedMethod)) {
+ return false;
+ }
+
+ // We can't publicize private instance methods in interfaces or methods that are copied from
+ // interfaces to lambda-desugared classes because this will be added as a new default method.
+ // TODO(b/72109068): It might be possible to transform it into static methods, though.
+ if (holder.isInterface() || accessFlags.isSynthetic()) {
+ return false;
+ }
+
+ Wrapper<DexMethod> key = equivalence.wrap(encodedMethod.method);
+ if (methodPool.contains(key)) {
+ // We can't do anything further because even renaming is not allowed due to the keep rule.
+ if (rootSet.noObfuscation.contains(encodedMethod)) {
+ return false;
+ }
+ // TODO(b/72109068): Renaming will enable more private instance methods to be publicized.
+ return false;
+ }
+ methodPool.add(key);
+ lenseBuilder.add(encodedMethod.method);
+ accessFlags.unsetPrivate();
+ accessFlags.setFinal();
+ accessFlags.setPublic();
+ // Although the current method became public, it surely has the single virtual target.
+ encodedMethod.method.setSingleVirtualMethodCache(
+ encodedMethod.method.getHolder(), encodedMethod);
+ encodedMethod.markPublicized();
+ return true;
}
// For private static methods we can just relax the access to private, since
@@ -70,5 +184,6 @@
// does not take into account access of the static methods.
accessFlags.unsetPrivate();
accessFlags.setPublic();
+ return false;
}
}
diff --git a/src/main/java/com/android/tools/r8/optimize/MemberRebindingAnalysis.java b/src/main/java/com/android/tools/r8/optimize/MemberRebindingAnalysis.java
index da7a787..9efd5c8 100644
--- a/src/main/java/com/android/tools/r8/optimize/MemberRebindingAnalysis.java
+++ b/src/main/java/com/android/tools/r8/optimize/MemberRebindingAnalysis.java
@@ -116,7 +116,6 @@
private void computeMethodRebinding(Set<DexMethod> methods,
Function<DexMethod, DexEncodedMethod> lookupTarget) {
for (DexMethod method : methods) {
- method = lense.lookupMethod(method);
// We can safely ignore array types, as the corresponding methods are defined in a library.
if (!method.getHolder().isClassType()) {
continue;
@@ -150,7 +149,7 @@
target = bridgeMethod;
}
}
- builder.map(method, validTargetFor(target.method, method));
+ builder.map(method, lense.lookupMethod(validTargetFor(target.method, method)));
}
}
}
@@ -189,7 +188,6 @@
BiFunction<DexClass, DexField, DexEncodedField> lookupTargetOnClass) {
for (Map.Entry<DexField, Set<DexEncodedMethod>> entry : fields.entrySet()) {
DexField field = entry.getKey();
- field = lense.lookupField(field);
DexEncodedField target = lookup.apply(field.getHolder(), field);
// Rebind to the lowest library class or program class. Do not rebind accesses to fields that
// are not visible from the access context.
@@ -197,7 +195,8 @@
if (target != null && target.field != field
&& contexts.stream().allMatch(context ->
isVisibleFromOriginalContext(context.method.getHolder(), target))) {
- builder.map(field, validTargetFor(target.field, field, lookupTargetOnClass));
+ builder.map(field,
+ lense.lookupField(validTargetFor(target.field, field, lookupTargetOnClass)));
}
}
}
diff --git a/src/main/java/com/android/tools/r8/optimize/PublicizerLense.java b/src/main/java/com/android/tools/r8/optimize/PublicizerLense.java
new file mode 100644
index 0000000..835135e
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/optimize/PublicizerLense.java
@@ -0,0 +1,68 @@
+// Copyright (c) 2018, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+package com.android.tools.r8.optimize;
+
+import com.android.tools.r8.graph.AppInfo;
+import com.android.tools.r8.graph.DexClass;
+import com.android.tools.r8.graph.DexEncodedMethod;
+import com.android.tools.r8.graph.DexMethod;
+import com.android.tools.r8.graph.GraphLense;
+import com.android.tools.r8.graph.GraphLense.NestedGraphLense;
+import com.android.tools.r8.ir.code.Invoke.Type;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import java.util.Set;
+
+final class PublicizerLense extends NestedGraphLense {
+ private final AppInfo appInfo;
+ private final Set<DexMethod> publicizedMethods;
+
+ PublicizerLense(
+ AppInfo appInfo, GraphLense previousLense, Set<DexMethod> publicizedMethods) {
+ // This lense does not map any DexItem's at all.
+ // It will just tweak invoke type for publicized methods from invoke-direct to invoke-virtual.
+ super(ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of(),
+ previousLense, appInfo.dexItemFactory);
+ this.appInfo = appInfo;
+ this.publicizedMethods = publicizedMethods;
+ }
+
+ @Override
+ public GraphLenseLookupResult lookupMethod(
+ DexMethod method, DexEncodedMethod context, Type type) {
+ GraphLenseLookupResult previous = previousLense.lookupMethod(method, context, type);
+ method = previous.getMethod();
+ type = previous.getType();
+ if (type == Type.DIRECT && publicizedMethods.contains(method)) {
+ DexClass holderClass = appInfo.definitionFor(method.holder);
+ if (holderClass != null) {
+ DexEncodedMethod actualEncodedTarget = holderClass.lookupVirtualMethod(method);
+ if (actualEncodedTarget != null
+ && actualEncodedTarget.isPublicized()) {
+ return new GraphLenseLookupResult(method, Type.VIRTUAL);
+ }
+ }
+ }
+ return super.lookupMethod(method, context, type);
+ }
+
+ static PublicizedLenseBuilder createBuilder() {
+ return new PublicizedLenseBuilder();
+ }
+
+ static class PublicizedLenseBuilder {
+ private final ImmutableSet.Builder<DexMethod> methodSetBuilder = ImmutableSet.builder();
+
+ private PublicizedLenseBuilder() {
+ }
+
+ public GraphLense build(AppInfo appInfo, GraphLense previousLense) {
+ return new PublicizerLense(appInfo, previousLense, methodSetBuilder.build());
+ }
+
+ public void add(DexMethod publicizedMethod) {
+ methodSetBuilder.add(publicizedMethod);
+ }
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/shaking/AnnotationRemover.java b/src/main/java/com/android/tools/r8/shaking/AnnotationRemover.java
index 28ca9fc..459fe7a 100644
--- a/src/main/java/com/android/tools/r8/shaking/AnnotationRemover.java
+++ b/src/main/java/com/android/tools/r8/shaking/AnnotationRemover.java
@@ -139,10 +139,20 @@
}
private void stripAttributes(DexProgramClass clazz) {
- if (!keep.enclosingMethod && clazz.getEnclosingMethod() != null) {
+ // If [clazz] is mentioned by a keep rule, it could be used for reflection, and we therefore
+ // need to keep the enclosing method and inner classes attributes, if requested. In Proguard
+ // compatibility mode we keep these attributes independent of whether the given class is kept.
+ if (appInfo.isPinned(clazz.type) || options.forceProguardCompatibility) {
+ if (!keep.enclosingMethod) {
+ clazz.clearEnclosingMethod();
+ }
+ if (!keep.innerClasses) {
+ clazz.clearInnerClasses();
+ }
+ } else {
+ // These attributes are only relevant for reflection, and this class is not used for
+ // reflection. (Note that clearing these attributes can enable more vertical class merging.)
clazz.clearEnclosingMethod();
- }
- if (!keep.innerClasses && !clazz.getInnerClasses().isEmpty()) {
clazz.clearInnerClasses();
}
}
diff --git a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
index d42c68b..baacb1e 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -113,6 +113,11 @@
private final Set<DexItem> identifierNameStrings = Sets.newIdentityHashSet();
/**
+ * Set of method signatures used in invoke-super instructions that either cannot be resolved or
+ * resolve to a private method (leading to an IllegalAccessError).
+ */
+ private final Set<DexMethod> brokenSuperInvokes = Sets.newIdentityHashSet();
+ /**
* This map keeps a view of all virtual methods that are reachable from virtual invokes. A method
* is reachable even if no live subtypes exist, so this is not sufficient for inclusion in the
* live set.
@@ -1018,6 +1023,7 @@
DexEncodedMethod resolutionTarget = appInfo.resolveMethod(method.holder, method)
.asResultOfResolve();
if (resolutionTarget == null) {
+ brokenSuperInvokes.add(method);
reportMissingMethod(method);
return;
}
@@ -1029,6 +1035,9 @@
reportMissingMethod(method);
return;
}
+ if (target.accessFlags.isPrivate()) {
+ brokenSuperInvokes.add(method);
+ }
assert !superInvokeDependencies.containsKey(from) || !superInvokeDependencies.get(from)
.contains(target);
if (Log.ENABLED) {
@@ -1518,6 +1527,11 @@
*/
public final SortedSet<DexMethod> staticInvokes;
/**
+ * Set of method signatures used in invoke-super instructions that either cannot be resolved or
+ * resolve to a private method (leading to an IllegalAccessError).
+ */
+ public final SortedSet<DexMethod> brokenSuperInvokes;
+ /**
* Set of all items that have to be kept independent of whether they are used.
*/
final Set<DexItem> pinnedItems;
@@ -1583,6 +1597,8 @@
this.superInvokes = joinInvokedMethods(enqueuer.superInvokes, TargetWithContext::getTarget);
this.directInvokes = joinInvokedMethods(enqueuer.directInvokes);
this.staticInvokes = joinInvokedMethods(enqueuer.staticInvokes);
+ this.brokenSuperInvokes =
+ ImmutableSortedSet.copyOf(DexMethod::slowCompareTo, enqueuer.brokenSuperInvokes);
this.noSideEffects = enqueuer.rootSet.noSideEffects;
this.assumedValues = enqueuer.rootSet.assumedValues;
this.alwaysInline = enqueuer.rootSet.alwaysInline;
@@ -1621,6 +1637,7 @@
this.superInvokes = previous.superInvokes;
this.directInvokes = previous.directInvokes;
this.staticInvokes = previous.staticInvokes;
+ this.brokenSuperInvokes = previous.brokenSuperInvokes;
this.protoLiteFields = previous.protoLiteFields;
this.alwaysInline = previous.alwaysInline;
this.identifierNameStrings = previous.identifierNameStrings;
@@ -1657,6 +1674,7 @@
this.superInvokes = rewriteMethodsConservatively(previous.superInvokes, lense);
this.directInvokes = rewriteMethodsConservatively(previous.directInvokes, lense);
this.staticInvokes = rewriteMethodsConservatively(previous.staticInvokes, lense);
+ this.brokenSuperInvokes = rewriteMethodsConservatively(previous.brokenSuperInvokes, lense);
this.prunedTypes = rewriteItems(previous.prunedTypes, lense::lookupType);
// TODO(herhut): Migrate these to Descriptors, as well.
assert lense.assertNotModified(previous.noSideEffects.keySet());
@@ -1703,6 +1721,7 @@
this.superInvokes = previous.superInvokes;
this.directInvokes = previous.directInvokes;
this.staticInvokes = previous.staticInvokes;
+ this.brokenSuperInvokes = previous.brokenSuperInvokes;
this.protoLiteFields = previous.protoLiteFields;
this.alwaysInline = previous.alwaysInline;
this.identifierNameStrings = previous.identifierNameStrings;
diff --git a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
index c5d2144..6f935ee 100644
--- a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
@@ -6,7 +6,6 @@
import com.android.tools.r8.errors.CompilationError;
import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.AppInfo.ResolutionResult;
-import com.android.tools.r8.graph.DefaultUseRegistry;
import com.android.tools.r8.graph.DexAnnotationSet;
import com.android.tools.r8.graph.DexApplication;
import com.android.tools.r8.graph.DexClass;
@@ -77,7 +76,6 @@
ILLEGAL_ACCESS,
NO_SIDE_EFFECTS,
PINNED_SOURCE,
- PINNED_TARGET,
RESOLUTION_FOR_FIELDS_MAY_CHANGE,
RESOLUTION_FOR_METHODS_MAY_CHANGE,
STATIC_INITIALIZERS,
@@ -109,9 +107,6 @@
case PINNED_SOURCE:
message = "it should be kept";
break;
- case PINNED_TARGET:
- message = "its target should be kept";
- break;
case RESOLUTION_FOR_FIELDS_MAY_CHANGE:
message = "it could affect field resolution";
break;
@@ -181,37 +176,24 @@
extractPinnedItems(appInfo.alwaysInline, pinnedTypes, AbortReason.ALWAYS_INLINE);
extractPinnedItems(appInfo.noSideEffects.keySet(), pinnedTypes, AbortReason.NO_SIDE_EFFECTS);
- for (DexProgramClass clazz : classes) {
- for (DexEncodedMethod method : clazz.methods()) {
- // Avoid merging two types if this could remove a NoSuchMethodError, as illustrated by the
- // following example. (Alternatively, it would be possible to merge A and B and rewrite the
- // "invoke-super A.m" instruction into "invoke-super Object.m" to preserve the error. This
- // situation should generally not occur in practice, though.)
- //
- // class A {}
- // class B extends A {
- // public void m() {}
- // }
- // class C extends A {
- // public void m() {
- // invoke-super "A.m" <- should yield NoSuchMethodError, cannot merge A and B
- // }
- // }
- if (!method.isStaticMethod()) {
- method.registerCodeReferences(
- new DefaultUseRegistry() {
- @Override
- public boolean registerInvokeSuper(DexMethod target) {
- DexClass targetClass = appInfo.definitionFor(target.getHolder());
- if (targetClass != null
- && targetClass.isProgramClass()
- && targetClass.lookupVirtualMethod(target) == null) {
- pinnedTypes.add(target.getHolder());
- }
- return true;
- }
- });
- }
+ // Avoid merging two types if this could remove a NoSuchMethodError, as illustrated by the
+ // following example. (Alternatively, it would be possible to merge A and B and rewrite the
+ // "invoke-super A.m" instruction into "invoke-super Object.m" to preserve the error. This
+ // situation should generally not occur in practice, though.)
+ //
+ // class A {}
+ // class B extends A {
+ // public void m() {}
+ // }
+ // class C extends A {
+ // public void m() {
+ // invoke-super "A.m" <- should yield NoSuchMethodError, cannot merge A and B
+ // }
+ // }
+ for (DexMethod signature : appInfo.brokenSuperInvokes) {
+ DexClass targetClass = appInfo.definitionFor(signature.holder);
+ if (targetClass != null && targetClass.isProgramClass()) {
+ pinnedTypes.add(signature.holder);
}
}
return pinnedTypes;
@@ -469,13 +451,6 @@
DexClass targetClass = appInfo.definitionFor(clazz.type.getSingleSubtype());
assert !mergedClasses.containsKey(targetClass.type);
- if (appInfo.isPinned(targetClass.type)) {
- // We have to keep the target class intact, so we cannot merge it.
- if (Log.ENABLED) {
- AbortReason.PINNED_TARGET.printLogMessageForClass(clazz);
- }
- continue;
- }
if (clazz.hasClassInitializer() && targetClass.hasClassInitializer()) {
// TODO(herhut): Handle class initializers.
if (Log.ENABLED) {
@@ -1179,7 +1154,7 @@
}
}
- private static class CollisionDetector {
+ private class CollisionDetector {
private static final int NOT_FOUND = 1 << (Integer.SIZE - 1);
@@ -1204,27 +1179,30 @@
}
boolean mayCollide() {
+ timing.begin("collision detection");
fillSeenPositions(invokes);
+ boolean result = false;
// If the type is not used in methods at all, there cannot be any conflict.
- if (seenPositions.isEmpty()) {
- return false;
- }
- for (DexMethod method : invokes) {
- Int2IntMap positionsMap = seenPositions.get(method.name);
- if (positionsMap != null) {
- int arity = method.getArity();
- int previous = positionsMap.get(arity);
- if (previous != NOT_FOUND) {
- assert previous != 0;
- int positions = computePositionsFor(method.proto, source, sourceProtoCache,
- substituions);
- if ((positions & previous) != 0) {
- return true;
+ if (!seenPositions.isEmpty()) {
+ for (DexMethod method : invokes) {
+ Int2IntMap positionsMap = seenPositions.get(method.name);
+ if (positionsMap != null) {
+ int arity = method.getArity();
+ int previous = positionsMap.get(arity);
+ if (previous != NOT_FOUND) {
+ assert previous != 0;
+ int positions =
+ computePositionsFor(method.proto, source, sourceProtoCache, substituions);
+ if ((positions & previous) != 0) {
+ result = true;
+ break;
+ }
}
}
}
}
- return false;
+ timing.end();
+ return result;
}
private void fillSeenPositions(Collection<DexMethod> invokes) {
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 834f62e..eb8ee0e 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -631,4 +631,12 @@
public boolean canHaveArtStringNewInitBug() {
return minApiLevel <= AndroidApiLevel.P.getLevel();
}
+
+ // Dalvik tracing JIT may perform invalid optimizations when int/float values are converted to
+ // double and used in arithmetic operations.
+ //
+ // See b/77496850.
+ public boolean canHaveNumberConversionRegisterAllocationBug() {
+ return minApiLevel <= AndroidApiLevel.K.getLevel();
+ }
}
diff --git a/src/main/java/com/android/tools/r8/utils/Timing.java b/src/main/java/com/android/tools/r8/utils/Timing.java
index ff2aa68..e8cf59b 100644
--- a/src/main/java/com/android/tools/r8/utils/Timing.java
+++ b/src/main/java/com/android/tools/r8/utils/Timing.java
@@ -13,6 +13,8 @@
// Finally a report is printed by:
// t.report();
+import java.util.LinkedHashMap;
+import java.util.Map;
import java.util.Stack;
public class Timing {
@@ -27,23 +29,28 @@
static class Node {
final String title;
- final Stack<Node> sons = new Stack<>();
- final long start_time;
- long stop_time;
+ final Map<String, Node> children = new LinkedHashMap<>();
+ long duration = 0;
+ long start_time;
Node(String title) {
this.title = title;
this.start_time = System.nanoTime();
- this.stop_time = -1;
+ }
+
+ void restart() {
+ assert start_time == -1;
+ start_time = System.nanoTime();
}
void end() {
- stop_time = System.nanoTime();
+ duration += System.nanoTime() - start_time;
+ start_time = -1;
assert duration() >= 0;
}
long duration() {
- return stop_time - start_time;
+ return duration;
}
@Override
@@ -66,15 +73,22 @@
System.out.print("- ");
}
System.out.println(toString(top));
- sons.forEach(p -> { p.report(depth + 1, top); });
+ children.values().forEach(p -> p.report(depth + 1, top));
}
}
public void begin(String title) {
- Node n = new Node(title);
- stack.peek().sons.add(n);
- stack.push(n);
+ Node parent = stack.peek();
+ Node child;
+ if (parent.children.containsKey(title)) {
+ child = parent.children.get(title);
+ child.restart();
+ } else {
+ child = new Node(title);
+ parent.children.put(title, child);
+ }
+ stack.push(child);
}
public void end() {
diff --git a/src/test/examples/classmerging/PinnedParameterTypesTest.java b/src/test/examples/classmerging/PinnedParameterTypesTest.java
new file mode 100644
index 0000000..bdfe3b7
--- /dev/null
+++ b/src/test/examples/classmerging/PinnedParameterTypesTest.java
@@ -0,0 +1,47 @@
+// Copyright (c) 2018, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package classmerging;
+
+import java.lang.reflect.Method;
+
+public class PinnedParameterTypesTest {
+
+ public static void main(String[] args) throws Exception {
+ for (Method method : TestClass.class.getMethods()) {
+ if (method.getName().equals("method")) {
+ Class<?> parameterType = method.getParameterTypes()[0];
+
+ // Should print classmerging.PinnedParameterTypesTest$Interface when -keepparameternames is
+ // used.
+ System.out.println(parameterType.getName());
+
+ method.invoke(null, new InterfaceImpl());
+ break;
+ }
+ }
+ }
+
+ public interface Interface {
+
+ void foo();
+ }
+
+ public static class InterfaceImpl implements Interface {
+
+ @Override
+ public void foo() {
+ System.out.println("In InterfaceImpl.foo()");
+ }
+ }
+
+ public static class TestClass {
+
+ // This method has been kept explicitly by a keep rule. Therefore, since -keepparameternames is
+ // used, Interface must not be merged into InterfaceImpl.
+ public static void method(Interface obj) {
+ obj.foo();
+ }
+ }
+}
diff --git a/src/test/examples/classmerging/keep-rules.txt b/src/test/examples/classmerging/keep-rules.txt
index fa48747..516e39d 100644
--- a/src/test/examples/classmerging/keep-rules.txt
+++ b/src/test/examples/classmerging/keep-rules.txt
@@ -19,6 +19,12 @@
-keep public class classmerging.RewritePinnedMethodTest {
public static void main(...);
}
+-keep public class classmerging.PinnedParameterTypesTest {
+ public static void main(...);
+}
+-keep public class classmerging.PinnedParameterTypesTest$TestClass {
+ public static void method(...);
+}
-keep public class classmerging.SimpleInterfaceAccessTest {
public static void main(...);
}
diff --git a/src/test/java/com/android/tools/r8/R8RunArtTestsTest.java b/src/test/java/com/android/tools/r8/R8RunArtTestsTest.java
index 2032866..7328c6e 100644
--- a/src/test/java/com/android/tools/r8/R8RunArtTestsTest.java
+++ b/src/test/java/com/android/tools/r8/R8RunArtTestsTest.java
@@ -1009,6 +1009,11 @@
);
private static List<String> requireClassInliningToBeDisabled = ImmutableList.of(
+ // Test is registered to be failing (failingRunWithArt), it fails only on 4.0.4
+ // (because of a bug in this version of dalvik) but succeeds on later versions.
+ // If class inliner us enabled it actually works fine on 4.0.4 since the class
+ // instantiation is properly inlined.
+ "301-abstract-protected",
// Test depends on exception produced for missing method or similar cases, but
// after class inlining removes class instantiations and references the exception
// is not produced.
diff --git a/src/test/java/com/android/tools/r8/accessrelaxation/AccessRelaxationTest.java b/src/test/java/com/android/tools/r8/accessrelaxation/AccessRelaxationTest.java
index 34b92b0..027b7e8 100644
--- a/src/test/java/com/android/tools/r8/accessrelaxation/AccessRelaxationTest.java
+++ b/src/test/java/com/android/tools/r8/accessrelaxation/AccessRelaxationTest.java
@@ -4,35 +4,89 @@
package com.android.tools.r8.accessrelaxation;
+import static com.android.tools.r8.utils.DexInspectorMatchers.isPresent;
+import static com.android.tools.r8.utils.DexInspectorMatchers.isPublic;
+import static org.hamcrest.CoreMatchers.not;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
import com.android.tools.r8.DexIndexedConsumer;
import com.android.tools.r8.R8Command;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.VmTestRunner;
+import com.android.tools.r8.accessrelaxation.privateinstance.Base;
+import com.android.tools.r8.accessrelaxation.privateinstance.Sub1;
+import com.android.tools.r8.accessrelaxation.privateinstance.Sub2;
+import com.android.tools.r8.accessrelaxation.privateinstance.TestMain;
import com.android.tools.r8.accessrelaxation.privatestatic.A;
import com.android.tools.r8.accessrelaxation.privatestatic.B;
import com.android.tools.r8.accessrelaxation.privatestatic.BB;
import com.android.tools.r8.accessrelaxation.privatestatic.C;
+import com.android.tools.r8.naming.MemberNaming.MethodSignature;
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.utils.AndroidApp;
+import com.android.tools.r8.utils.DexInspector;
+import com.android.tools.r8.utils.DexInspector.ClassSubject;
+import com.android.tools.r8.utils.DexInspector.MethodSubject;
import com.google.common.collect.ImmutableList;
import org.junit.Test;
+import org.junit.runner.RunWith;
+@RunWith(VmTestRunner.class)
public class AccessRelaxationTest extends TestBase {
- @Test
- public void testStaticMethodRelaxation() throws Exception {
+ private static final String STRING = "java.lang.String";
+
+ private static R8Command.Builder loadProgramFiles(Package p, Class... classes) throws Exception {
R8Command.Builder builder = R8Command.builder();
- builder.addProgramFiles(ToolHelper.getClassFilesForTestPackage(A.class.getPackage()));
+ builder.addProgramFiles(ToolHelper.getClassFilesForTestPackage(p));
+ for (Class clazz : classes) {
+ builder.addProgramFiles(ToolHelper.getClassFileForTestClass(clazz));
+ }
builder.setProgramConsumer(DexIndexedConsumer.emptyConsumer());
builder.setMinApiLevel(ToolHelper.getMinApiLevelForDexVm().getLevel());
+ return builder;
+ }
+
+ private void compareJvmAndArt(AndroidApp app, Class mainClass) throws Exception {
+ // Run on Jvm.
+ String jvmOutput = runOnJava(mainClass);
+
+ // Run on Art to check generated code against verifier.
+ String artOutput = runOnArt(app, mainClass);
+
+ String adjustedArtOutput = artOutput.replace(
+ "java.lang.IncompatibleClassChangeError", "java.lang.IllegalAccessError");
+ assertEquals(jvmOutput, adjustedArtOutput);
+ }
+
+ private static void assertPublic(
+ DexInspector dexInspector, Class clazz, MethodSignature signature) {
+ ClassSubject classSubject = dexInspector.clazz(clazz);
+ assertThat(classSubject, isPresent());
+ MethodSubject methodSubject = classSubject.method(signature);
+ assertThat(methodSubject, isPublic());
+ }
+
+ private static void assertNotPublic(
+ DexInspector dexInspector, Class clazz, MethodSignature signature) {
+ ClassSubject classSubject = dexInspector.clazz(clazz);
+ assertThat(classSubject, isPresent());
+ MethodSubject methodSubject = classSubject.method(signature);
+ assertThat(methodSubject, not(isPublic()));
+ }
+
+ @Test
+ public void testStaticMethodRelaxation() throws Exception {
+ Class mainClass = C.class;
+ R8Command.Builder builder = loadProgramFiles(mainClass.getPackage());
// Note: we use '-checkdiscard' to indirectly check that the access relaxation is
// done which leads to inlining of all pB*** methods so they are removed. Without
// access relaxation inlining is not performed and method are kept.
builder.addProguardConfiguration(
ImmutableList.of(
- "-keep class " + C.class.getCanonicalName() + "{",
+ "-keep class " + mainClass.getCanonicalName() + "{",
" public static void main(java.lang.String[]);",
"}",
"",
@@ -57,15 +111,71 @@
Origin.unknown());
AndroidApp app = ToolHelper.runR8(builder.build());
+ compareJvmAndArt(app, mainClass);
- // Run on Jvm.
- String jvmOutput = runOnJava(C.class);
+ DexInspector dexInspector = new DexInspector(app);
+ assertPublic(dexInspector, A.class,
+ new MethodSignature("baz", STRING, ImmutableList.of()));
+ assertPublic(dexInspector, A.class,
+ new MethodSignature("bar", STRING, ImmutableList.of()));
+ assertPublic(dexInspector, A.class,
+ new MethodSignature("bar", STRING, ImmutableList.of("int")));
+ assertPublic(dexInspector, A.class,
+ new MethodSignature("blah", STRING, ImmutableList.of("int")));
- // Run on Art to check generated code against verifier.
- String artOutput = runOnArt(app, C.class);
+ assertPublic(dexInspector, B.class,
+ new MethodSignature("blah", STRING, ImmutableList.of("int")));
- String adjustedArtOutput = artOutput.replace(
- "java.lang.IncompatibleClassChangeError", "java.lang.IllegalAccessError");
- assertEquals(jvmOutput, adjustedArtOutput);
+ assertPublic(dexInspector, BB.class,
+ new MethodSignature("blah", STRING, ImmutableList.of("int")));
+ }
+
+ @Test
+ public void testInstanceMethodRelaxation() throws Exception {
+ Class mainClass = TestMain.class;
+ R8Command.Builder builder = loadProgramFiles(mainClass.getPackage());
+
+ builder.addProguardConfiguration(
+ ImmutableList.of(
+ "-keep class " + mainClass.getCanonicalName() + "{",
+ " public static void main(java.lang.String[]);",
+ "}",
+ "",
+ "-checkdiscard class " + Base.class.getCanonicalName() + "{",
+ " *** p*();",
+ "}",
+ "",
+ "-checkdiscard class " + Sub1.class.getCanonicalName() + "{",
+ " *** p*();",
+ "}",
+ "",
+ "-checkdiscard class " + Sub2.class.getCanonicalName() + "{",
+ " *** p*();",
+ "}",
+ "",
+ "-dontobfuscate",
+ "-allowaccessmodification"
+ ),
+ Origin.unknown());
+
+ AndroidApp app = ToolHelper.runR8(builder.build());
+ compareJvmAndArt(app, mainClass);
+
+ DexInspector dexInspector = new DexInspector(app);
+ assertPublic(dexInspector, Base.class,
+ new MethodSignature("foo", STRING, ImmutableList.of()));
+
+ // Base#foo?() can't be publicized due to Itf<1>#foo<1>().
+ assertNotPublic(dexInspector, Base.class,
+ new MethodSignature("foo1", STRING, ImmutableList.of()));
+ assertNotPublic(dexInspector, Base.class,
+ new MethodSignature("foo2", STRING, ImmutableList.of()));
+
+ // Sub1#bar1(int) can't be publicized due to Base#bar1(int).
+ assertNotPublic(dexInspector, Sub1.class,
+ new MethodSignature("bar1", STRING, ImmutableList.of("int")));
+
+ assertPublic(dexInspector, Sub2.class,
+ new MethodSignature("bar2", STRING, ImmutableList.of("int")));
}
}
diff --git a/src/test/java/com/android/tools/r8/accessrelaxation/InvokeTypeConversionTest.java b/src/test/java/com/android/tools/r8/accessrelaxation/InvokeTypeConversionTest.java
new file mode 100644
index 0000000..1f97c11
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/accessrelaxation/InvokeTypeConversionTest.java
@@ -0,0 +1,147 @@
+// Copyright (c) 2018, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+package com.android.tools.r8.accessrelaxation;
+
+import static com.android.tools.r8.utils.DexInspectorMatchers.isPresent;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+import com.android.tools.r8.R8Command;
+import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.ToolHelper.DexVm.Version;
+import com.android.tools.r8.ToolHelper.ProcessResult;
+import com.android.tools.r8.VmTestRunner;
+import com.android.tools.r8.code.InvokeDirect;
+import com.android.tools.r8.code.InvokeVirtual;
+import com.android.tools.r8.graph.DexCode;
+import com.android.tools.r8.graph.DexEncodedMethod;
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.smali.SmaliBuilder;
+import com.android.tools.r8.smali.SmaliBuilder.MethodSignature;
+import com.android.tools.r8.smali.SmaliTestBase;
+import com.android.tools.r8.utils.AndroidApp;
+import com.android.tools.r8.utils.DexInspector;
+import com.android.tools.r8.utils.DexInspector.ClassSubject;
+import com.google.common.collect.ImmutableList;
+import java.util.List;
+import java.util.function.Consumer;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+@RunWith(VmTestRunner.class)
+public class InvokeTypeConversionTest extends SmaliTestBase {
+ private final String CLASS_NAME = "Example";
+ private MethodSignature main;
+
+ private SmaliBuilder buildTestClass(String invokeLine) {
+ SmaliBuilder builder = new SmaliBuilder(CLASS_NAME);
+ builder.addDefaultConstructor();
+ builder.addPrivateInstanceMethod("int", "foo", ImmutableList.of(), 1,
+ " const/4 v0, 0",
+ " return v0");
+ builder.addPrivateStaticMethod("int", "bar", ImmutableList.of(), 1,
+ " const/4 v0, 0",
+ " return v0");
+ main = builder.addMainMethod(2,
+ "new-instance v1, L" + CLASS_NAME + ";",
+ "invoke-direct { v1 }, L" + CLASS_NAME + ";-><init>()V",
+ invokeLine,
+ "move-result v1",
+ "invoke-static { v1 }, Ljava/lang/Integer;->valueOf(I)Ljava/lang/Integer;",
+ "move-result-object v1",
+ "sget-object v0, Ljava/lang/System;->out:Ljava/io/PrintStream;",
+ "invoke-virtual { v0, v1 }, Ljava/io/PrintStream;->print(Ljava/lang/Object;)V",
+ "return-void");
+ return builder;
+ }
+
+ private void run(
+ SmaliBuilder builder,
+ String expectedException,
+ Consumer<DexInspector> inspectorConsumer) throws Exception {
+ AndroidApp app = buildApplication(builder);
+ List<String> pgConfigs = ImmutableList.of(
+ keepMainProguardConfiguration(CLASS_NAME),
+ "-printmapping",
+ "-dontobfuscate",
+ "-allowaccessmodification");
+ R8Command.Builder command = ToolHelper.prepareR8CommandBuilder(app);
+ command.addProguardConfiguration(pgConfigs, Origin.unknown());
+ AndroidApp processedApp = ToolHelper.runR8(command.build(), o -> {
+ o.enableInlining = false;
+ });
+ ProcessResult artResult = runOnArtRaw(processedApp, CLASS_NAME);
+ if (expectedException == null) {
+ assertEquals(0, artResult.exitCode);
+ assertEquals("0", artResult.stdout);
+ } else {
+ assertEquals(1, artResult.exitCode);
+ assertThat(artResult.stderr, containsString(expectedException));
+ }
+ DexInspector inspector = new DexInspector(processedApp);
+ inspectorConsumer.accept(inspector);
+ }
+
+ // The following test checks invoke-direct, which refers to the private static method, is *not*
+ // rewritten by publicizer lense, resulting in IncompatibleClassChangeError, which is expected.
+ //
+ // class Example {
+ // private int foo() { return 0; }
+ // private static int bar() { return 0; }
+ // public static void main(String[] args) {
+ // Example instance = new Example();
+ // "invoke-direct instance, Example->bar()" <- should yield IncompatibleClassChangeError
+ // ...
+ // }
+ // }
+ @Test
+ public void invokeDirectToAlreadyStaticMethod() throws Exception {
+ SmaliBuilder builder = buildTestClass(
+ "invoke-direct { v1 }, L" + CLASS_NAME + ";->bar()I");
+ String expectedError =
+ ToolHelper.getDexVm().getVersion().isOlderThanOrEqual(Version.V4_4_4)
+ ? "VerifyError" : "IncompatibleClassChangeError";
+ run(builder, expectedError, dexInspector -> {
+ ClassSubject clazz = dexInspector.clazz(CLASS_NAME);
+ assertThat(clazz, isPresent());
+ DexEncodedMethod method = getMethod(dexInspector, main);
+ assertNotNull(method);
+ DexCode code = method.getCode().asDexCode();
+ // The given invoke line is remained as-is.
+ assertTrue(code.instructions[2] instanceof InvokeDirect);
+ });
+ }
+
+ // The following test checks invoke-direct, which refers to the private instance method, *is*
+ // rewritten by publicizer lense, as the target method will be publicized.
+ //
+ // class Example {
+ // private int foo() { return 0; }
+ // private static int bar() { return 0; }
+ // public static void main(String[] args) {
+ // Example instance = new Example();
+ // instance.foo(); // which was "invoke-direct instance, Example->foo()"
+ // // will be "invoke-virtual instance, Example->foo()"
+ // ...
+ // }
+ // }
+ @Test
+ public void invokeDirectToPublicizedMethod() throws Exception {
+ SmaliBuilder builder = buildTestClass(
+ "invoke-direct { v1 }, L" + CLASS_NAME + ";->foo()I");
+ run(builder, null, dexInspector -> {
+ ClassSubject clazz = dexInspector.clazz(CLASS_NAME);
+ assertThat(clazz, isPresent());
+ DexEncodedMethod method = getMethod(dexInspector, main);
+ assertNotNull(method);
+ DexCode code = method.getCode().asDexCode();
+ // The given invoke line is changed to invoke-virtual
+ assertTrue(code.instructions[2] instanceof InvokeVirtual);
+ });
+ }
+
+}
\ No newline at end of file
diff --git a/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/Base.java b/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/Base.java
new file mode 100644
index 0000000..fd1fd1c
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/Base.java
@@ -0,0 +1,48 @@
+// Copyright (c) 2018, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+package com.android.tools.r8.accessrelaxation.privateinstance;
+
+public class Base {
+
+ // NOTE: here and below 'synchronized' is supposed to disable inlining of this method.
+ private synchronized String foo() {
+ return "Base::foo()";
+ }
+
+ public String pFoo() {
+ return foo();
+ }
+
+ private synchronized String foo1() {
+ return "Base::foo1()";
+ }
+
+ public String pFoo1() {
+ return foo1();
+ }
+
+ private synchronized String foo2() {
+ return "Base::foo2()";
+ }
+
+ public String pFoo2() {
+ return foo2();
+ }
+
+ private synchronized String bar1(int i) {
+ throw new AssertionError("Sub1#bar1(int) will not use this signature.");
+ }
+
+ public void dump() {
+ System.out.println(foo());
+ System.out.println(foo1());
+ System.out.println(foo2());
+ try {
+ bar1(0);
+ } catch (AssertionError e) {
+ // expected
+ }
+ }
+
+}
diff --git a/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/Itf1.java b/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/Itf1.java
new file mode 100644
index 0000000..bc44973
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/Itf1.java
@@ -0,0 +1,12 @@
+// Copyright (c) 2018, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+package com.android.tools.r8.accessrelaxation.privateinstance;
+
+public interface Itf1 {
+ String foo1();
+
+ default String foo1(int i) {
+ return "Itf1::foo1(" + i + ") >> " + foo1();
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/Itf2.java b/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/Itf2.java
new file mode 100644
index 0000000..233a327
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/Itf2.java
@@ -0,0 +1,12 @@
+// Copyright (c) 2018, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+package com.android.tools.r8.accessrelaxation.privateinstance;
+
+public interface Itf2 {
+ String foo2();
+
+ default String foo2(int i) {
+ return "Itf2::foo2(" + i + ") >> " + foo2();
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/Sub1.java b/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/Sub1.java
new file mode 100644
index 0000000..4cbbf11
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/Sub1.java
@@ -0,0 +1,32 @@
+// Copyright (c) 2018, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+package com.android.tools.r8.accessrelaxation.privateinstance;
+
+public class Sub1 extends Base implements Itf1 {
+
+ @Override
+ public String foo1() {
+ return "Sub1::foo1()";
+ }
+
+ private synchronized String bar1(int i) {
+ return "Sub1::bar1(" + i + ")";
+ }
+
+ public String pBar1() {
+ return bar1(1);
+ }
+
+ @Override
+ public void dump() {
+ System.out.println(foo1());
+ System.out.println(foo1(0));
+ try {
+ System.out.println(bar1(0));
+ } catch (Throwable e) {
+ System.out.println(e.getClass().getName());
+ }
+ }
+
+}
diff --git a/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/Sub2.java b/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/Sub2.java
new file mode 100644
index 0000000..8c4e01e
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/Sub2.java
@@ -0,0 +1,28 @@
+// Copyright (c) 2018, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+package com.android.tools.r8.accessrelaxation.privateinstance;
+
+public class Sub2 extends Base implements Itf2 {
+
+ @Override
+ public String foo2() {
+ return "Sub2::foo2()";
+ }
+
+ private synchronized String bar2(int i) {
+ return "Sub2::bar2(" + i + ")";
+ }
+
+ public String pBar2() {
+ return bar2(2);
+ }
+
+ @Override
+ public void dump() {
+ System.out.println(foo2());
+ System.out.println(foo2(0));
+ System.out.println(bar2(0));
+ }
+
+}
diff --git a/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/TestMain.java b/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/TestMain.java
new file mode 100644
index 0000000..bfd322b
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/TestMain.java
@@ -0,0 +1,13 @@
+// Copyright (c) 2018, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+package com.android.tools.r8.accessrelaxation.privateinstance;
+
+public class TestMain {
+
+ public static void main(String[] args) {
+ new Base().dump();
+ new Sub1().dump();
+ new Sub2().dump();
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java b/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java
index 4da4821..59accb3 100644
--- a/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java
@@ -45,8 +45,7 @@
import org.junit.runner.RunWith;
// TODO(christofferqa): Add tests to check that statically typed invocations on method handles
-// continue to work after class merging. Rewriting of method handles should be carried out by
-// LensCodeRewriter.rewriteDexMethodHandle.
+// continue to work after class merging.
@RunWith(VmTestRunner.class)
public class ClassMergingTest extends TestBase {
@@ -207,6 +206,29 @@
}
@Test
+ public void testPinnedParameterTypes() throws Exception {
+ String main = "classmerging.PinnedParameterTypesTest";
+ Path[] programFiles =
+ new Path[] {
+ CF_DIR.resolve("PinnedParameterTypesTest.class"),
+ CF_DIR.resolve("PinnedParameterTypesTest$Interface.class"),
+ CF_DIR.resolve("PinnedParameterTypesTest$InterfaceImpl.class"),
+ CF_DIR.resolve("PinnedParameterTypesTest$TestClass.class")
+ };
+ Set<String> preservedClassNames =
+ ImmutableSet.of(
+ "classmerging.PinnedParameterTypesTest",
+ "classmerging.PinnedParameterTypesTest$Interface",
+ "classmerging.PinnedParameterTypesTest$InterfaceImpl",
+ "classmerging.PinnedParameterTypesTest$TestClass");
+ runTest(
+ main,
+ programFiles,
+ preservedClassNames::contains,
+ getProguardConfig(EXAMPLE_KEEP, "-keepparameternames"));
+ }
+
+ @Test
public void testSuperCallWasDetected() throws Exception {
String main = "classmerging.SuperCallRewritingTest";
Path[] programFiles =
diff --git a/src/test/java/com/android/tools/r8/ir/SplitBlockTest.java b/src/test/java/com/android/tools/r8/ir/SplitBlockTest.java
index e7344a4..0cf2f53 100644
--- a/src/test/java/com/android/tools/r8/ir/SplitBlockTest.java
+++ b/src/test/java/com/android/tools/r8/ir/SplitBlockTest.java
@@ -196,7 +196,7 @@
public void runCatchHandlerTest(boolean codeThrows, boolean twoGuards) throws Exception {
final int secondBlockInstructions = 4;
- final int initialBlockCount = 5;
+ final int initialBlockCount = 6;
// Try split between all instructions in second block.
for (int i = 1; i < secondBlockInstructions; i++) {
TestApplication test = codeWithCatchHandlers(codeThrows, twoGuards);
@@ -235,7 +235,7 @@
public void runCatchHandlerSplitThreeTest(boolean codeThrows, boolean twoGuards)
throws Exception {
final int secondBlockInstructions = 4;
- final int initialBlockCount = 5;
+ final int initialBlockCount = 6;
// Try split out all instructions in second block.
for (int i = 1; i < secondBlockInstructions - 1; i++) {
TestApplication test = codeWithCatchHandlers(codeThrows, twoGuards);
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/CheckCastRemovalTest.java b/src/test/java/com/android/tools/r8/ir/optimize/CheckCastRemovalTest.java
index be7ee92..f894322 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/CheckCastRemovalTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/CheckCastRemovalTest.java
@@ -89,7 +89,7 @@
"-keep class B { *; }",
"-keep class C { *; }",
"-dontshrink");
- AndroidApp app = compileWithR8(builder, pgConfigs, null);
+ AndroidApp app = compileWithR8(builder, pgConfigs, opts -> opts.enableClassInlining = false);
DexEncodedMethod method = getMethod(app, CLASS_NAME, main);
assertNotNull(method);
diff --git a/src/test/java/com/android/tools/r8/kotlin/AbstractR8KotlinTestBase.java b/src/test/java/com/android/tools/r8/kotlin/AbstractR8KotlinTestBase.java
index f99934e..fd78cb7 100644
--- a/src/test/java/com/android/tools/r8/kotlin/AbstractR8KotlinTestBase.java
+++ b/src/test/java/com/android/tools/r8/kotlin/AbstractR8KotlinTestBase.java
@@ -231,7 +231,7 @@
}
// Build classpath for compilation (and java execution)
- assert classpath.isEmpty();
+ classpath.clear();
classpath.add(getKotlinJarFile(folder));
classpath.add(getJavaJarFile(folder));
classpath.addAll(extraClasspath);
diff --git a/src/test/java/com/android/tools/r8/kotlin/KotlinClassInlinerTest.java b/src/test/java/com/android/tools/r8/kotlin/KotlinClassInlinerTest.java
index 46af30d..a690429 100644
--- a/src/test/java/com/android/tools/r8/kotlin/KotlinClassInlinerTest.java
+++ b/src/test/java/com/android/tools/r8/kotlin/KotlinClassInlinerTest.java
@@ -7,7 +7,9 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import com.android.tools.r8.ToolHelper.KotlinTargetVersion;
import com.android.tools.r8.code.NewInstance;
import com.android.tools.r8.code.SgetObject;
import com.android.tools.r8.graph.DexClass;
@@ -16,14 +18,26 @@
import com.android.tools.r8.naming.MemberNaming.MethodSignature;
import com.android.tools.r8.utils.DexInspector;
import com.android.tools.r8.utils.DexInspector.ClassSubject;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.Sets;
+import java.util.Collection;
import java.util.Set;
import java.util.function.Predicate;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.junit.Test;
+import org.junit.runners.Parameterized.Parameters;
public class KotlinClassInlinerTest extends AbstractR8KotlinTestBase {
+ @Parameters(name = "allowAccessModification: {0} target: {1}")
+ public static Collection<Object[]> data() {
+ ImmutableList.Builder<Object[]> builder = new ImmutableList.Builder<>();
+ for (KotlinTargetVersion targetVersion : KotlinTargetVersion.values()) {
+ builder.add(new Object[]{Boolean.TRUE, targetVersion});
+ }
+ return builder.build();
+ }
+
private static boolean isLambda(DexClass clazz) {
return !clazz.type.getPackageDescriptor().startsWith("kotlin") &&
(isKStyleLambdaOrGroup(clazz) || isJStyleLambdaOrGroup(clazz));
@@ -49,7 +63,17 @@
@Test
public void testJStyleLambdas() throws Exception {
final String mainClassName = "class_inliner_lambda_j_style.MainKt";
- runTest("class_inliner_lambda_j_style", mainClassName, (app) -> {
+ runTest("class_inliner_lambda_j_style", mainClassName, false, (app) -> {
+ DexInspector inspector = new DexInspector(app);
+ assertTrue(
+ inspector.clazz("class_inliner_lambda_j_style.MainKt$testStateful$1").isPresent());
+ assertTrue(
+ inspector.clazz("class_inliner_lambda_j_style.MainKt$testStateful2$1").isPresent());
+ assertTrue(
+ inspector.clazz("class_inliner_lambda_j_style.MainKt$testStateful3$1").isPresent());
+ });
+
+ runTest("class_inliner_lambda_j_style", mainClassName, true, (app) -> {
DexInspector inspector = new DexInspector(app);
Predicate<DexType> lambdaCheck = createLambdaCheck(inspector);
ClassSubject clazz = inspector.clazz(mainClassName);
@@ -81,6 +105,40 @@
});
}
+ @Test
+ public void testKStyleLambdas() throws Exception {
+ final String mainClassName = "class_inliner_lambda_k_style.MainKt";
+ runTest("class_inliner_lambda_k_style", mainClassName, false, (app) -> {
+ DexInspector inspector = new DexInspector(app);
+ assertTrue(inspector.clazz(
+ "class_inliner_lambda_k_style.MainKt$testKotlinSequencesStateless$1").isPresent());
+ assertTrue(inspector.clazz(
+ "class_inliner_lambda_k_style.MainKt$testKotlinSequencesStateful$1").isPresent());
+ });
+
+ runTest("class_inliner_lambda_k_style", mainClassName, true, (app) -> {
+ DexInspector inspector = new DexInspector(app);
+ Predicate<DexType> lambdaCheck = createLambdaCheck(inspector);
+ ClassSubject clazz = inspector.clazz(mainClassName);
+
+ assertEquals(
+ Sets.newHashSet(),
+ collectAccessedLambdaTypes(lambdaCheck, clazz,
+ "testKotlinSequencesStateless", "kotlin.sequences.Sequence"));
+
+ assertFalse(inspector.clazz(
+ "class_inliner_lambda_k_style.MainKt$testKotlinSequencesStateless$1").isPresent());
+
+ assertEquals(
+ Sets.newHashSet(),
+ collectAccessedLambdaTypes(lambdaCheck, clazz,
+ "testKotlinSequencesStateful", "int", "int", "kotlin.sequences.Sequence"));
+
+ assertFalse(inspector.clazz(
+ "class_inliner_lambda_k_style.MainKt$testKotlinSequencesStateful$1").isPresent());
+ });
+ }
+
private Set<String> collectAccessedLambdaTypes(
Predicate<DexType> isLambdaType, ClassSubject clazz, String methodName, String... params) {
assertNotNull(clazz);
@@ -97,14 +155,13 @@
.collect(Collectors.toSet());
}
- @Override
protected void runTest(String folder, String mainClass,
- AndroidAppInspector inspector) throws Exception {
+ boolean enabled, AndroidAppInspector inspector) throws Exception {
runTest(
folder, mainClass, null,
options -> {
options.enableInlining = true;
- options.enableClassInlining = true;
+ options.enableClassInlining = enabled;
options.enableLambdaMerging = false;
}, inspector);
}
diff --git a/src/test/java/com/android/tools/r8/kotlin/KotlinLambdaMergingTest.java b/src/test/java/com/android/tools/r8/kotlin/KotlinLambdaMergingTest.java
index ee9065f..e1431a6 100644
--- a/src/test/java/com/android/tools/r8/kotlin/KotlinLambdaMergingTest.java
+++ b/src/test/java/com/android/tools/r8/kotlin/KotlinLambdaMergingTest.java
@@ -14,12 +14,14 @@
import com.android.tools.r8.ir.optimize.lambda.CaptureSignature;
import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.DexInspector;
+import com.android.tools.r8.utils.InternalOptions;
import com.google.common.collect.Lists;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.ExecutionException;
+import java.util.function.Consumer;
import org.junit.Test;
public class KotlinLambdaMergingTest extends AbstractR8KotlinTestBase {
@@ -29,6 +31,13 @@
"-keepattributes InnerClasses,EnclosingMethod\n";
private static final String KEEP_SIGNATURE_INNER_ENCLOSING =
"-keepattributes Signature,InnerClasses,EnclosingMethod\n";
+ private Consumer<InternalOptions> optionsModifier =
+ opts -> {
+ opts.enableClassInlining = false;
+ // Ensure that enclosing method and inner class attributes are kept even on classes that are
+ // not explicitly mentioned by a keep rule.
+ opts.forceProguardCompatibility = true;
+ };
abstract static class LambdaOrGroup {
abstract boolean match(DexClass clazz);
@@ -252,7 +261,7 @@
@Test
public void testTrivialKs() throws Exception {
final String mainClassName = "lambdas_kstyle_trivial.MainKt";
- runTest("lambdas_kstyle_trivial", mainClassName, (app) -> {
+ runTest("lambdas_kstyle_trivial", mainClassName, optionsModifier, (app) -> {
Verifier verifier = new Verifier(app);
String pkg = "lambdas_kstyle_trivial";
@@ -293,7 +302,7 @@
@Test
public void testCapturesKs() throws Exception {
final String mainClassName = "lambdas_kstyle_captures.MainKt";
- runTest("lambdas_kstyle_captures", mainClassName, (app) -> {
+ runTest("lambdas_kstyle_captures", mainClassName, optionsModifier, (app) -> {
Verifier verifier = new Verifier(app);
String pkg = "lambdas_kstyle_captures";
String grpPkg = allowAccessModification ? "" : pkg;
@@ -318,7 +327,7 @@
@Test
public void testGenericsNoSignatureKs() throws Exception {
final String mainClassName = "lambdas_kstyle_generics.MainKt";
- runTest("lambdas_kstyle_generics", mainClassName, (app) -> {
+ runTest("lambdas_kstyle_generics", mainClassName, optionsModifier, (app) -> {
Verifier verifier = new Verifier(app);
String pkg = "lambdas_kstyle_generics";
String grpPkg = allowAccessModification ? "" : pkg;
@@ -339,55 +348,57 @@
@Test
public void testInnerClassesAndEnclosingMethodsKs() throws Exception {
final String mainClassName = "lambdas_kstyle_generics.MainKt";
- runTest("lambdas_kstyle_generics", mainClassName, KEEP_INNER_AND_ENCLOSING, (app) -> {
- Verifier verifier = new Verifier(app);
- String pkg = "lambdas_kstyle_generics";
- String grpPkg = allowAccessModification ? "" : pkg;
+ runTest("lambdas_kstyle_generics", mainClassName,
+ KEEP_INNER_AND_ENCLOSING, optionsModifier, (app) -> {
+ Verifier verifier = new Verifier(app);
+ String pkg = "lambdas_kstyle_generics";
+ String grpPkg = allowAccessModification ? "" : pkg;
- verifier.assertLambdaGroups(
- kstyle(grpPkg, 1, 3), // Group for Any
- kstyle(grpPkg, "L", 1), // Group for Beta // First
- kstyle(grpPkg, "L", 1), // Group for Beta // Second
- kstyle(grpPkg, "LS", 1), // Group for Gamma // First
- kstyle(grpPkg, "LS", 1), // Group for Gamma // Second
- kstyle(grpPkg, 1, 2) // Group for int
- );
+ verifier.assertLambdaGroups(
+ kstyle(grpPkg, 1, 3), // Group for Any
+ kstyle(grpPkg, "L", 1), // Group for Beta // First
+ kstyle(grpPkg, "L", 1), // Group for Beta // Second
+ kstyle(grpPkg, "LS", 1), // Group for Gamma // First
+ kstyle(grpPkg, "LS", 1), // Group for Gamma // Second
+ kstyle(grpPkg, 1, 2) // Group for int
+ );
- verifier.assertLambdas(
- new Lambda(pkg, "MainKt$main$4", 1)
- );
- });
+ verifier.assertLambdas(
+ new Lambda(pkg, "MainKt$main$4", 1)
+ );
+ });
}
@Test
public void testGenericsSignatureInnerEnclosingKs() throws Exception {
final String mainClassName = "lambdas_kstyle_generics.MainKt";
- runTest("lambdas_kstyle_generics", mainClassName, KEEP_SIGNATURE_INNER_ENCLOSING, (app) -> {
- Verifier verifier = new Verifier(app);
- String pkg = "lambdas_kstyle_generics";
- String grpPkg = allowAccessModification ? "" : pkg;
+ runTest("lambdas_kstyle_generics", mainClassName,
+ KEEP_SIGNATURE_INNER_ENCLOSING, optionsModifier, (app) -> {
+ Verifier verifier = new Verifier(app);
+ String pkg = "lambdas_kstyle_generics";
+ String grpPkg = allowAccessModification ? "" : pkg;
- verifier.assertLambdaGroups(
- kstyle(grpPkg, 1, 3), // Group for Any
- kstyle(grpPkg, "L", 1), // Group for Beta in First
- kstyle(grpPkg, "L", 1), // Group for Beta in Second
- kstyle(grpPkg, "LS", 1), // Group for Gamma<String> in First
- kstyle(grpPkg, "LS", 1), // Group for Gamma<Integer> in First
- kstyle(grpPkg, "LS", 1), // Group for Gamma<String> in Second
- kstyle(grpPkg, "LS", 1), // Group for Gamma<Integer> in Second
- kstyle(grpPkg, 1, 2) // Group for int
- );
+ verifier.assertLambdaGroups(
+ kstyle(grpPkg, 1, 3), // Group for Any
+ kstyle(grpPkg, "L", 1), // Group for Beta in First
+ kstyle(grpPkg, "L", 1), // Group for Beta in Second
+ kstyle(grpPkg, "LS", 1), // Group for Gamma<String> in First
+ kstyle(grpPkg, "LS", 1), // Group for Gamma<Integer> in First
+ kstyle(grpPkg, "LS", 1), // Group for Gamma<String> in Second
+ kstyle(grpPkg, "LS", 1), // Group for Gamma<Integer> in Second
+ kstyle(grpPkg, 1, 2) // Group for int
+ );
- verifier.assertLambdas(
- new Lambda(pkg, "MainKt$main$4", 1)
- );
- });
+ verifier.assertLambdas(
+ new Lambda(pkg, "MainKt$main$4", 1)
+ );
+ });
}
@Test
public void testTrivialJs() throws Exception {
final String mainClassName = "lambdas_jstyle_trivial.MainKt";
- runTest("lambdas_jstyle_trivial", mainClassName, (app) -> {
+ runTest("lambdas_jstyle_trivial", mainClassName, optionsModifier, (app) -> {
Verifier verifier = new Verifier(app);
String pkg = "lambdas_jstyle_trivial";
String grp = allowAccessModification ? "" : pkg;
@@ -435,7 +446,7 @@
@Test
public void testSingleton() throws Exception {
final String mainClassName = "lambdas_singleton.MainKt";
- runTest("lambdas_singleton", mainClassName, (app) -> {
+ runTest("lambdas_singleton", mainClassName, optionsModifier, (app) -> {
Verifier verifier = new Verifier(app);
String pkg = "lambdas_singleton";
String grp = allowAccessModification ? "" : pkg;
diff --git a/src/test/java/com/android/tools/r8/maindexlist/MainDexListTests.java b/src/test/java/com/android/tools/r8/maindexlist/MainDexListTests.java
index 4214a67..101bf06 100644
--- a/src/test/java/com/android/tools/r8/maindexlist/MainDexListTests.java
+++ b/src/test/java/com/android/tools/r8/maindexlist/MainDexListTests.java
@@ -766,7 +766,7 @@
}
@Override
- public int getMoveExceptionRegister() {
+ public int getMoveExceptionRegister(int instructionIndex) {
throw new Unreachable();
}
diff --git a/src/test/java/com/android/tools/r8/naming/MinifierTest.java b/src/test/java/com/android/tools/r8/naming/MinifierTest.java
index 4b42880..8a5975ea 100644
--- a/src/test/java/com/android/tools/r8/naming/MinifierTest.java
+++ b/src/test/java/com/android/tools/r8/naming/MinifierTest.java
@@ -98,10 +98,10 @@
// method naming001.A.m should be kept, according to the keep rule.
assertEquals("m", naming.lookupName(m).toSourceString());
+ // method naming001.A.privateFunc is transformed to a public method, and then kept.
DexMethod p = dexItemFactory.createMethod(
a, dexItemFactory.createProto(dexItemFactory.voidType), "privateFunc");
- // method naming001.A.privateFunc would be renamed.
- assertNotEquals("privateFunc", naming.lookupName(p).toSourceString());
+ assertEquals("privateFunc", naming.lookupName(p).toSourceString());
}
private static void test001_rule005(DexItemFactory dexItemFactory, NamingLens naming) {
diff --git a/src/test/java/com/android/tools/r8/naming/NamingTestBase.java b/src/test/java/com/android/tools/r8/naming/NamingTestBase.java
index f311c15..6d16184 100644
--- a/src/test/java/com/android/tools/r8/naming/NamingTestBase.java
+++ b/src/test/java/com/android/tools/r8/naming/NamingTestBase.java
@@ -7,6 +7,7 @@
import com.android.tools.r8.graph.AppInfoWithSubtyping;
import com.android.tools.r8.graph.DexApplication;
import com.android.tools.r8.graph.DexItemFactory;
+import com.android.tools.r8.graph.GraphLense;
import com.android.tools.r8.optimize.ClassAndMemberPublicizer;
import com.android.tools.r8.shaking.Enqueuer;
import com.android.tools.r8.shaking.ProguardConfiguration;
@@ -57,7 +58,7 @@
}
@Before
- public void readApp() throws IOException, ExecutionException, ProguardRuleParserException {
+ public void readApp() throws IOException, ExecutionException {
program = ToolHelper.buildApplication(ImmutableList.of(appFileName));
dexItemFactory = program.dexItemFactory;
appInfo = new AppInfoWithSubtyping(program);
@@ -70,13 +71,18 @@
InternalOptions options = new InternalOptions(configuration,
new Reporter(new DefaultDiagnosticsHandler()));
- if (options.proguardConfiguration.isAccessModificationAllowed()) {
- ClassAndMemberPublicizer.run(program, dexItemFactory);
- }
-
ExecutorService executor = ThreadUtils.getExecutorService(1);
+
RootSet rootSet = new RootSetBuilder(appInfo, program, configuration.getRules(), options)
.run(executor);
+
+ if (options.proguardConfiguration.isAccessModificationAllowed()) {
+ ClassAndMemberPublicizer.run(
+ executor, timing, program, appInfo, rootSet, GraphLense.getIdentityLense());
+ rootSet =
+ new RootSetBuilder(appInfo, program, configuration.getRules(), options).run(executor);
+ }
+
Enqueuer enqueuer = new Enqueuer(appInfo, options, options.forceProguardCompatibility);
appInfo = enqueuer.traceApplication(rootSet, executor, timing);
return new Minifier(appInfo.withLiveness(), rootSet, options).run(timing);
diff --git a/src/test/java/com/android/tools/r8/neverreturnsnormally/NeverReturnsNormallyTest.java b/src/test/java/com/android/tools/r8/neverreturnsnormally/NeverReturnsNormallyTest.java
index 1f9c20f..3e46098 100644
--- a/src/test/java/com/android/tools/r8/neverreturnsnormally/NeverReturnsNormallyTest.java
+++ b/src/test/java/com/android/tools/r8/neverreturnsnormally/NeverReturnsNormallyTest.java
@@ -22,6 +22,7 @@
import com.google.common.collect.ImmutableList;
import java.util.Iterator;
import java.util.function.BiConsumer;
+import org.junit.Ignore;
import org.junit.Test;
public class NeverReturnsNormallyTest extends TestBase {
@@ -127,6 +128,7 @@
return instructions.next();
}
+ @Ignore("b/110736241")
@Test
public void test() throws Exception {
runTest(this::validate, CompilationMode.DEBUG);
diff --git a/src/test/java/com/android/tools/r8/regress/b77496850/B77496850.java b/src/test/java/com/android/tools/r8/regress/b77496850/B77496850.java
new file mode 100644
index 0000000..03614b9
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/regress/b77496850/B77496850.java
@@ -0,0 +1,507 @@
+// Copyright (c) 2018, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.regress.b77496850;
+
+import static org.junit.Assert.assertEquals;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.code.InvokeStatic;
+import com.android.tools.r8.dex.Marker.Tool;
+import com.android.tools.r8.graph.DexCode;
+import com.android.tools.r8.graph.DexItemFactory;
+import com.android.tools.r8.graph.DexMethod;
+import com.android.tools.r8.graph.DexString;
+import com.android.tools.r8.utils.AndroidApiLevel;
+import com.android.tools.r8.utils.AndroidApp;
+import com.android.tools.r8.utils.DexInspector;
+import com.android.tools.r8.utils.DexInspector.ClassSubject;
+import com.android.tools.r8.utils.DexInspector.MethodSubject;
+import com.google.common.collect.ImmutableList;
+import org.junit.Test;
+
+public class B77496850 extends TestBase {
+
+ static final String LOGTAG = "";
+
+ // Mock class for the code in PathParser below.
+ public static class Path {
+ void rLineTo(int x, int y) {
+ }
+ void cubicTo(float a, float b, float c, float d, float e, float f) {
+ }
+ }
+
+ // Mock class for the code in PathParser below.
+ public static class Log {
+ static void w(String x, String y) {
+ }
+ }
+
+ // Code copied from Android support library:
+ // https://android.googlesource.com/platform/frameworks/support/+/9791ac540f94c318f6602123d7000bfc55909b81/compat/src/main/java/android/support/v4/graphics/PathParser.java
+ public static class PathParser {
+
+ private static void drawArc(Path p,
+ float x0,
+ float y0,
+ float x1,
+ float y1,
+ float a,
+ float b,
+ float theta,
+ boolean isMoreThanHalf,
+ boolean isPositiveArc) {
+ /* Convert rotation angle from degrees to radians */
+ double thetaD = Math.toRadians(theta);
+ /* Pre-compute rotation matrix entries */
+ double cosTheta = Math.cos(thetaD);
+ double sinTheta = Math.sin(thetaD);
+ /* Transform (x0, y0) and (x1, y1) into unit space */
+ /* using (inverse) rotation, followed by (inverse) scale */
+ double x0p = (x0 * cosTheta + y0 * sinTheta) / a;
+ double y0p = (-x0 * sinTheta + y0 * cosTheta) / b;
+ double x1p = (x1 * cosTheta + y1 * sinTheta) / a;
+ double y1p = (-x1 * sinTheta + y1 * cosTheta) / b;
+ /* Compute differences and averages */
+ double dx = x0p - x1p;
+ double dy = y0p - y1p;
+ double xm = (x0p + x1p) / 2;
+ double ym = (y0p + y1p) / 2;
+ /* Solve for intersecting unit circles */
+ double dsq = dx * dx + dy * dy;
+ if (dsq == 0.0) {
+ Log.w(LOGTAG, " Points are coincident");
+ return; /* Points are coincident */
+ }
+ double disc = 1.0 / dsq - 1.0 / 4.0;
+ if (disc < 0.0) {
+ Log.w(LOGTAG, "Points are too far apart " + dsq);
+ float adjust = (float) (Math.sqrt(dsq) / 1.99999);
+ drawArc(p, x0, y0, x1, y1, a * adjust,
+ b * adjust, theta, isMoreThanHalf, isPositiveArc);
+ return; /* Points are too far apart */
+ }
+ double s = Math.sqrt(disc);
+ double sdx = s * dx;
+ double sdy = s * dy;
+ double cx;
+ double cy;
+ if (isMoreThanHalf == isPositiveArc) {
+ cx = xm - sdy;
+ cy = ym + sdx;
+ } else {
+ cx = xm + sdy;
+ cy = ym - sdx;
+ }
+ double eta0 = Math.atan2((y0p - cy), (x0p - cx));
+ double eta1 = Math.atan2((y1p - cy), (x1p - cx));
+ double sweep = (eta1 - eta0);
+ if (isPositiveArc != (sweep >= 0)) {
+ if (sweep > 0) {
+ sweep -= 2 * Math.PI;
+ } else {
+ sweep += 2 * Math.PI;
+ }
+ }
+ cx *= a;
+ cy *= b;
+ double tcx = cx;
+ cx = cx * cosTheta - cy * sinTheta;
+ cy = tcx * sinTheta + cy * cosTheta;
+ arcToBezier(p, cx, cy, a, b, x0, y0, thetaD, eta0, sweep);
+ }
+
+ /**
+ * Converts an arc to cubic Bezier segments and records them in p.
+ *
+ * @param p The target for the cubic Bezier segments
+ * @param cx The x coordinate center of the ellipse
+ * @param cy The y coordinate center of the ellipse
+ * @param a The radius of the ellipse in the horizontal direction
+ * @param b The radius of the ellipse in the vertical direction
+ * @param e1x E(eta1) x coordinate of the starting point of the arc
+ * @param e1y E(eta2) y coordinate of the starting point of the arc
+ * @param theta The angle that the ellipse bounding rectangle makes with horizontal plane
+ * @param start The start angle of the arc on the ellipse
+ * @param sweep The angle (positive or negative) of the sweep of the arc on the ellipse
+ */
+ private static void arcToBezier(Path p,
+ double cx,
+ double cy,
+ double a,
+ double b,
+ double e1x,
+ double e1y,
+ double theta,
+ double start,
+ double sweep) {
+ // Taken from equations at: http://spaceroots.org/documents/ellipse/node8.html
+ // and http://www.spaceroots.org/documents/ellipse/node22.html
+ // Maximum of 45 degrees per cubic Bezier segment
+ int numSegments = (int) Math.ceil(Math.abs(sweep * 4 / Math.PI));
+ double eta1 = start;
+ double cosTheta = Math.cos(theta);
+ double sinTheta = Math.sin(theta);
+ double cosEta1 = Math.cos(eta1);
+ double sinEta1 = Math.sin(eta1);
+ double ep1x = (-a * cosTheta * sinEta1) - (b * sinTheta * cosEta1);
+ double ep1y = (-a * sinTheta * sinEta1) + (b * cosTheta * cosEta1);
+ double anglePerSegment = sweep / numSegments;
+ for (int i = 0; i < numSegments; i++) {
+ double eta2 = eta1 + anglePerSegment;
+ double sinEta2 = Math.sin(eta2);
+ double cosEta2 = Math.cos(eta2);
+ double e2x = cx + (a * cosTheta * cosEta2) - (b * sinTheta * sinEta2);
+ double e2y = cy + (a * sinTheta * cosEta2) + (b * cosTheta * sinEta2);
+ double ep2x = -a * cosTheta * sinEta2 - b * sinTheta * cosEta2;
+ double ep2y = -a * sinTheta * sinEta2 + b * cosTheta * cosEta2;
+ double tanDiff2 = Math.tan((eta2 - eta1) / 2);
+ double alpha =
+ Math.sin(eta2 - eta1) * (Math.sqrt(4 + (3 * tanDiff2 * tanDiff2)) - 1) / 3;
+ double q1x = e1x + alpha * ep1x;
+ double q1y = e1y + alpha * ep1y;
+ double q2x = e2x - alpha * ep2x;
+ double q2y = e2y - alpha * ep2y;
+ // Adding this no-op call to workaround a proguard related issue.
+ p.rLineTo(0, 0);
+ p.cubicTo((float) q1x,
+ (float) q1y,
+ (float) q2x,
+ (float) q2y,
+ (float) e2x,
+ (float) e2y);
+ eta1 = eta2;
+ e1x = e2x;
+ e1y = e2y;
+ ep1x = ep2x;
+ ep1y = ep2y;
+ }
+ }
+ }
+
+ // Same code as PathParser above, but with exception handlers in the two methods.
+ public static class PathParserWithExceptionHandler {
+
+ private static void drawArc(Path p,
+ float x0,
+ float y0,
+ float x1,
+ float y1,
+ float a,
+ float b,
+ float theta,
+ boolean isMoreThanHalf,
+ boolean isPositiveArc) {
+ try {
+ /* Convert rotation angle from degrees to radians */
+ double thetaD = Math.toRadians(theta);
+ /* Pre-compute rotation matrix entries */
+ double cosTheta = Math.cos(thetaD);
+ double sinTheta = Math.sin(thetaD);
+ /* Transform (x0, y0) and (x1, y1) into unit space */
+ /* using (inverse) rotation, followed by (inverse) scale */
+ double x0p = (x0 * cosTheta + y0 * sinTheta) / a;
+ double y0p = (-x0 * sinTheta + y0 * cosTheta) / b;
+ double x1p = (x1 * cosTheta + y1 * sinTheta) / a;
+ double y1p = (-x1 * sinTheta + y1 * cosTheta) / b;
+ /* Compute differences and averages */
+ double dx = x0p - x1p;
+ double dy = y0p - y1p;
+ double xm = (x0p + x1p) / 2;
+ double ym = (y0p + y1p) / 2;
+ /* Solve for intersecting unit circles */
+ double dsq = dx * dx + dy * dy;
+ if (dsq == 0.0) {
+ Log.w(LOGTAG, " Points are coincident");
+ return; /* Points are coincident */
+ }
+ double disc = 1.0 / dsq - 1.0 / 4.0;
+ if (disc < 0.0) {
+ Log.w(LOGTAG, "Points are too far apart " + dsq);
+ float adjust = (float) (Math.sqrt(dsq) / 1.99999);
+ drawArc(p, x0, y0, x1, y1, a * adjust,
+ b * adjust, theta, isMoreThanHalf, isPositiveArc);
+ return; /* Points are too far apart */
+ }
+ double s = Math.sqrt(disc);
+ double sdx = s * dx;
+ double sdy = s * dy;
+ double cx;
+ double cy;
+ if (isMoreThanHalf == isPositiveArc) {
+ cx = xm - sdy;
+ cy = ym + sdx;
+ } else {
+ cx = xm + sdy;
+ cy = ym - sdx;
+ }
+ double eta0 = Math.atan2((y0p - cy), (x0p - cx));
+ double eta1 = Math.atan2((y1p - cy), (x1p - cx));
+ double sweep = (eta1 - eta0);
+ if (isPositiveArc != (sweep >= 0)) {
+ if (sweep > 0) {
+ sweep -= 2 * Math.PI;
+ } else {
+ sweep += 2 * Math.PI;
+ }
+ }
+ cx *= a;
+ cy *= b;
+ double tcx = cx;
+ cx = cx * cosTheta - cy * sinTheta;
+ cy = tcx * sinTheta + cy * cosTheta;
+ arcToBezier(p, cx, cy, a, b, x0, y0, thetaD, eta0, sweep);
+ } catch (Throwable t) {
+ // Ignore.
+ }
+ }
+
+ /**
+ * Converts an arc to cubic Bezier segments and records them in p.
+ *
+ * @param p The target for the cubic Bezier segments
+ * @param cx The x coordinate center of the ellipse
+ * @param cy The y coordinate center of the ellipse
+ * @param a The radius of the ellipse in the horizontal direction
+ * @param b The radius of the ellipse in the vertical direction
+ * @param e1x E(eta1) x coordinate of the starting point of the arc
+ * @param e1y E(eta2) y coordinate of the starting point of the arc
+ * @param theta The angle that the ellipse bounding rectangle makes with horizontal plane
+ * @param start The start angle of the arc on the ellipse
+ * @param sweep The angle (positive or negative) of the sweep of the arc on the ellipse
+ */
+ private static void arcToBezier(Path p,
+ double cx,
+ double cy,
+ double a,
+ double b,
+ double e1x,
+ double e1y,
+ double theta,
+ double start,
+ double sweep) {
+ try {
+ // Taken from equations at: http://spaceroots.org/documents/ellipse/node8.html
+ // and http://www.spaceroots.org/documents/ellipse/node22.html
+ // Maximum of 45 degrees per cubic Bezier segment
+ int numSegments = (int) Math.ceil(Math.abs(sweep * 4 / Math.PI));
+ double eta1 = start;
+ double cosTheta = Math.cos(theta);
+ double sinTheta = Math.sin(theta);
+ double cosEta1 = Math.cos(eta1);
+ double sinEta1 = Math.sin(eta1);
+ double ep1x = (-a * cosTheta * sinEta1) - (b * sinTheta * cosEta1);
+ double ep1y = (-a * sinTheta * sinEta1) + (b * cosTheta * cosEta1);
+ double anglePerSegment = sweep / numSegments;
+ for (int i = 0; i < numSegments; i++) {
+ double eta2 = eta1 + anglePerSegment;
+ double sinEta2 = Math.sin(eta2);
+ double cosEta2 = Math.cos(eta2);
+ double e2x = cx + (a * cosTheta * cosEta2) - (b * sinTheta * sinEta2);
+ double e2y = cy + (a * sinTheta * cosEta2) + (b * cosTheta * sinEta2);
+ double ep2x = -a * cosTheta * sinEta2 - b * sinTheta * cosEta2;
+ double ep2y = -a * sinTheta * sinEta2 + b * cosTheta * cosEta2;
+ double tanDiff2 = Math.tan((eta2 - eta1) / 2);
+ double alpha =
+ Math.sin(eta2 - eta1) * (Math.sqrt(4 + (3 * tanDiff2 * tanDiff2)) - 1) / 3;
+ double q1x = e1x + alpha * ep1x;
+ double q1y = e1y + alpha * ep1y;
+ double q2x = e2x - alpha * ep2x;
+ double q2y = e2y - alpha * ep2y;
+ // Adding this no-op call to workaround a proguard related issue.
+ p.rLineTo(0, 0);
+ p.cubicTo((float) q1x,
+ (float) q1y,
+ (float) q2x,
+ (float) q2y,
+ (float) e2x,
+ (float) e2y);
+ eta1 = eta2;
+ e1x = e2x;
+ e1y = e2y;
+ ep1x = ep2x;
+ ep1y = ep2y;
+ }
+ } catch (Throwable t) {
+ // Ignore.
+ }
+ }
+
+ }
+
+ // Reproduction from b/77496850.
+ public static class Reproduction {
+ public int test() {
+ int count = 0;
+ for (int i = 0; i < 1000; i++){
+ count += arcToBezier(1.0, 1.0, 2.0);
+ }
+ return count;
+ }
+
+ private static int arcToBezier(double a, double b, double sweep) {
+ int count = 0;
+
+ int numSegments = (int) sweep;
+
+ double cosTheta = 0.5;
+ double sinTheta = 0.5;
+ double cosEta1 = 0.5;
+ double sinEta1 = 0.5;
+ double ep1x = (-a * cosTheta * sinEta1) - (b * sinTheta * cosEta1);
+ double anglePerSegment = sweep / numSegments;
+
+ for (int i = 0; i < numSegments; i++) {
+ count++;
+ }
+ if (numSegments != count) {
+ return 1;
+ }
+ return 0;
+ }
+
+ public static void main(String[] args) {
+ for (int i = 0; i < 100; i++) {
+ System.out.println(new Reproduction().test());
+ }
+ }
+ }
+
+ // Reproduction from b/77496850 with exception handler.
+ public static class ReproductionWithExceptionHandler {
+ public int test() {
+ int count = 0;
+ for (int i = 0; i < 1000; i++){
+ count += arcToBezier(1.0, 1.0, 2.0);
+ }
+ return count;
+ }
+
+ private static int arcToBezier(double a, double b, double sweep) {
+ try {
+ int count = 0;
+
+ int numSegments = (int) sweep;
+
+ double cosTheta = 0.5;
+ double sinTheta = 0.5;
+ double cosEta1 = 0.5;
+ double sinEta1 = 0.5;
+ double ep1x = (-a * cosTheta * sinEta1) - (b * sinTheta * cosEta1);
+ double anglePerSegment = sweep / numSegments;
+
+ for (int i = 0; i < numSegments; i++) {
+ count++;
+ }
+ if (numSegments != count) {
+ return 1;
+ }
+ return 0;
+ } catch (Throwable t) {
+ return 1;
+ }
+ }
+
+ public static void main(String[] args) {
+ for (int i = 0; i < 100; i++) {
+ System.out.println(new Reproduction().test());
+ }
+ }
+ }
+
+ private int countInvokeDoubleIsNan(DexItemFactory factory, DexCode code) {
+ int count = 0;
+ DexMethod doubleIsNaN = factory.createMethod(
+ factory.createString("Ljava/lang/Double;"),
+ factory.createString("isNaN"),
+ factory.booleanDescriptor,
+ new DexString[]{factory.doubleDescriptor});
+ for (int i = 0; i < code.instructions.length; i++) {
+ if (code.instructions[i] instanceof InvokeStatic) {
+ InvokeStatic invoke = (InvokeStatic) code.instructions[i];
+ if (invoke.getMethod() == doubleIsNaN) {
+ count++;
+ }
+ }
+ }
+ return count;
+ }
+
+ private void checkPathParserMethods(AndroidApp app, Class testClass, int a, int b)
+ throws Exception {
+ DexInspector inspector = new DexInspector(app);
+ DexItemFactory factory = inspector.getFactory();
+ ClassSubject clazz = inspector.clazz(testClass);
+ MethodSubject drawArc = clazz.method(
+ "void",
+ "drawArc",
+ ImmutableList.of(
+ getClass().getCanonicalName() + "$Path",
+ "float", "float", "float", "float", "float", "float", "float", "boolean", "boolean"));
+ MethodSubject arcToBezier = clazz.method(
+ "void",
+ "arcToBezier",
+ ImmutableList.of(
+ getClass().getCanonicalName() + "$Path",
+ "double", "double", "double", "double", "double", "double",
+ "double", "double", "double"));
+ assertEquals(a, countInvokeDoubleIsNan(factory, drawArc.getMethod().getCode().asDexCode()));
+ assertEquals(b, countInvokeDoubleIsNan(factory, arcToBezier.getMethod().getCode().asDexCode()));
+ }
+
+ private void runTestPathParser(
+ Tool compiler, Class testClass, AndroidApiLevel apiLevel, int a, int b)
+ throws Exception {
+ AndroidApp app = readClasses(Path.class, Log.class, testClass);
+ if (compiler == Tool.D8) {
+ app = compileWithD8(app, o -> o.minApiLevel = apiLevel.getLevel());
+ } else {
+ assert compiler == Tool.R8;
+ app = compileWithR8(app, "-keep class * { *; }", o -> o.minApiLevel = apiLevel.getLevel());
+ }
+ checkPathParserMethods(app, testClass, a, b);
+ }
+
+ @Test
+ public void testSupportLibraryPathParser() throws Exception{
+ for (Tool tool : Tool.values()) {
+ runTestPathParser(tool, PathParser.class, AndroidApiLevel.K, 14, 1);
+ runTestPathParser(tool, PathParser.class, AndroidApiLevel.L, 0, 0);
+ runTestPathParser(tool, PathParserWithExceptionHandler.class, AndroidApiLevel.K, 14, 1);
+ runTestPathParser(tool, PathParserWithExceptionHandler.class, AndroidApiLevel.L, 0, 0);
+ }
+ }
+
+ private void runTestReproduction(
+ Tool compiler, Class testClass, AndroidApiLevel apiLevel, int expectedInvokeDoubleIsNanCount)
+ throws Exception {
+ AndroidApp app = readClasses(testClass);
+ if (compiler == Tool.D8) {
+ app = compileWithD8(app, o -> o.minApiLevel = apiLevel.getLevel());
+ } else {
+ assert compiler == Tool.R8;
+ app = compileWithR8(app, "-keep class * { *; }", o -> o.minApiLevel = apiLevel.getLevel());
+ }
+ DexInspector inspector = new DexInspector(app);
+ DexItemFactory factory = inspector.getFactory();
+ ClassSubject clazz = inspector.clazz(testClass);
+ MethodSubject arcToBezier = clazz.method(
+ "int", "arcToBezier", ImmutableList.of("double", "double", "double"));
+ assertEquals(
+ expectedInvokeDoubleIsNanCount,
+ countInvokeDoubleIsNan(factory, arcToBezier.getMethod().getCode().asDexCode()));
+ }
+
+ @Test
+ public void testReproduction() throws Exception{
+ for (Tool tool : Tool.values()) {
+ runTestReproduction(tool, Reproduction.class, AndroidApiLevel.K, tool == Tool.D8 ? 1 : 0);
+ runTestReproduction(tool, Reproduction.class, AndroidApiLevel.L, 0);
+ runTestReproduction(
+ tool, ReproductionWithExceptionHandler.class, AndroidApiLevel.K, tool == Tool.D8 ? 1 : 0);
+ runTestReproduction(tool, ReproductionWithExceptionHandler.class, AndroidApiLevel.L, 0);
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/TreeShakingTest.java b/src/test/java/com/android/tools/r8/shaking/TreeShakingTest.java
index c9488b4..b6a1ac6 100644
--- a/src/test/java/com/android/tools/r8/shaking/TreeShakingTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/TreeShakingTest.java
@@ -18,6 +18,7 @@
import com.android.tools.r8.utils.DexInspector.ClassSubject;
import com.android.tools.r8.utils.DexInspector.FoundFieldSubject;
import com.android.tools.r8.utils.DexInspector.FoundMethodSubject;
+import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.ListUtils;
import com.android.tools.r8.utils.TestDescriptionWatcher;
import com.google.common.collect.ImmutableList;
@@ -94,7 +95,8 @@
String programFile,
List<Path> jarLibraries,
MinifyMode minify,
- List<String> keepRulesFiles)
+ List<String> keepRulesFiles,
+ Consumer<InternalOptions> optionsConsumer)
throws Exception {
out = temp.getRoot().toPath().resolve("out.zip");
proguardMap = temp.getRoot().toPath().resolve(DEFAULT_PROGUARD_MAP_FILE);
@@ -125,7 +127,14 @@
throw new Unreachable();
}
ToolHelper.getAppBuilder(builder).addProgramFiles(Paths.get(programFile));
- ToolHelper.runR8(builder.build(), options -> options.enableInlining = inline);
+ ToolHelper.runR8(
+ builder.build(),
+ options -> {
+ options.enableInlining = inline;
+ if (optionsConsumer != null) {
+ optionsConsumer.accept(options);
+ }
+ });
}
protected static void checkSameStructure(DexInspector ref, DexInspector inspector) {
@@ -163,6 +172,16 @@
BiConsumer<DexInspector, DexInspector> dexComparator,
List<String> keepRulesFiles)
throws Exception {
+ runTest(inspection, outputComparator, dexComparator, keepRulesFiles, null);
+ }
+
+ protected void runTest(
+ Consumer<DexInspector> inspection,
+ BiConsumer<String, String> outputComparator,
+ BiConsumer<DexInspector, DexInspector> dexComparator,
+ List<String> keepRulesFiles,
+ Consumer<InternalOptions> optionsConsumer)
+ throws Exception {
String originalDex = ToolHelper.TESTS_BUILD_DIR + name + "/classes.dex";
String programFile;
if (frontend == Frontend.DEX) {
@@ -183,7 +202,8 @@
Paths.get(ToolHelper.EXAMPLES_BUILD_DIR + "shakinglib.jar"));
}
- generateTreeShakedVersion(backend, programFile, jarLibraries, minify, keepRulesFiles);
+ generateTreeShakedVersion(
+ backend, programFile, jarLibraries, minify, keepRulesFiles, optionsConsumer);
if (backend == Backend.CF) {
Path shakinglib = Paths.get(ToolHelper.EXAMPLES_BUILD_DIR, "shakinglib.jar");
diff --git a/src/test/java/com/android/tools/r8/shaking/examples/TreeShakingAnnotationremovalTest.java b/src/test/java/com/android/tools/r8/shaking/examples/TreeShakingAnnotationremovalTest.java
index d18210c..a235df5 100644
--- a/src/test/java/com/android/tools/r8/shaking/examples/TreeShakingAnnotationremovalTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/examples/TreeShakingAnnotationremovalTest.java
@@ -46,7 +46,12 @@
TreeShakingAnnotationremovalTest::annotationRemovalHasNoInnerClassAnnotations,
null,
null,
- ImmutableList.of("src/test/examples/annotationremoval/keep-rules.txt"));
+ ImmutableList.of("src/test/examples/annotationremoval/keep-rules.txt"),
+ options -> {
+ // To ensure that enclosing method and inner class attributes are kept even on classes
+ // that are not explicitly mentioned by a keep rule.
+ options.forceProguardCompatibility = true;
+ });
}
@Test
@@ -55,8 +60,12 @@
TreeShakingAnnotationremovalTest::annotationRemovalHasAllInnerClassAnnotations,
null,
null,
- ImmutableList.of(
- "src/test/examples/annotationremoval/keep-rules-keep-innerannotation.txt"));
+ ImmutableList.of("src/test/examples/annotationremoval/keep-rules-keep-innerannotation.txt"),
+ options -> {
+ // To ensure that enclosing method and inner class attributes are kept even on classes
+ // that are not explicitly mentioned by a keep rule.
+ options.forceProguardCompatibility = true;
+ });
}
private static void annotationRemovalHasNoInnerClassAnnotations(DexInspector inspector) {
diff --git a/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/defaultctor/ImplicitlyKeptDefaultConstructorTest.java b/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/defaultctor/ImplicitlyKeptDefaultConstructorTest.java
index 61b1454..d8615e0 100644
--- a/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/defaultctor/ImplicitlyKeptDefaultConstructorTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/defaultctor/ImplicitlyKeptDefaultConstructorTest.java
@@ -155,8 +155,10 @@
runTest(
mainClass,
ImmutableList.of(mainClass, SuperClass.class, SubClass.class),
- // Prevent SuperClass from being merged into SubClass.
- keepMainProguardConfiguration(mainClass, ImmutableList.of("-keep class **.SuperClass")),
+ // Prevent SuperClass from being merged into SubClass and keep both
+ // SuperClass and SubClass after instantiation is inlined.
+ keepMainProguardConfiguration(mainClass, ImmutableList.of(
+ "-keep class **.SuperClass", "-keep class **.SubClass")),
this::checkAllClassesPresentWithDefaultConstructor);
}
diff --git a/src/test/java/com/android/tools/r8/smali/SmaliBuilder.java b/src/test/java/com/android/tools/r8/smali/SmaliBuilder.java
index e818801..1dffcee 100644
--- a/src/test/java/com/android/tools/r8/smali/SmaliBuilder.java
+++ b/src/test/java/com/android/tools/r8/smali/SmaliBuilder.java
@@ -257,6 +257,12 @@
return addStaticMethod(returnType, name, parameters, locals, buildCode(instructions));
}
+ public MethodSignature addPrivateStaticMethod(
+ String returnType, String name, List<String> parameters, int locals, String... instructions) {
+ return addMethod(
+ "private static", returnType, name, parameters, locals, buildCode(instructions));
+ }
+
public MethodSignature addStaticMethod(
String returnType, String name, List<String> parameters, int locals, String code) {
return addStaticMethod("", returnType, name, parameters, locals, code);
@@ -301,6 +307,11 @@
"public constructor", "void", "<init>", parameters, locals, buildCode(instructions));
}
+ public MethodSignature addPrivateInstanceMethod(
+ String returnType, String name, List<String> parameters, int locals, String... instructions) {
+ return addMethod("private", returnType, name, parameters, locals, buildCode(instructions));
+ }
+
public MethodSignature addInstanceMethod(
String returnType, String name, int locals, String... instructions) {
return addInstanceMethod(returnType, name, ImmutableList.of(), locals, buildCode(instructions));
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 fd32ec7..0b8de12 100644
--- a/src/test/java/com/android/tools/r8/utils/DexInspector.java
+++ b/src/test/java/com/android/tools/r8/utils/DexInspector.java
@@ -715,6 +715,8 @@
public abstract class MemberSubject extends Subject {
+ public abstract boolean isPublic();
+
public abstract boolean isStatic();
public abstract boolean isFinal();
@@ -773,6 +775,11 @@
}
@Override
+ public boolean isPublic() {
+ return false;
+ }
+
+ @Override
public boolean isStatic() {
return false;
}
@@ -849,6 +856,11 @@
}
@Override
+ public boolean isPublic() {
+ return dexMethod.accessFlags.isPublic();
+ }
+
+ @Override
public boolean isStatic() {
return dexMethod.accessFlags.isStatic();
}
@@ -966,6 +978,11 @@
public class AbsentFieldSubject extends FieldSubject {
@Override
+ public boolean isPublic() {
+ return false;
+ }
+
+ @Override
public boolean isStatic() {
return false;
}
@@ -1032,6 +1049,11 @@
}
@Override
+ public boolean isPublic() {
+ return dexField.accessFlags.isPublic();
+ }
+
+ @Override
public boolean isStatic() {
return dexField.accessFlags.isStatic();
}
diff --git a/src/test/java/com/android/tools/r8/utils/DexInspectorMatchers.java b/src/test/java/com/android/tools/r8/utils/DexInspectorMatchers.java
index 8e21bea..1986bbc 100644
--- a/src/test/java/com/android/tools/r8/utils/DexInspectorMatchers.java
+++ b/src/test/java/com/android/tools/r8/utils/DexInspectorMatchers.java
@@ -138,4 +138,24 @@
}
};
}
+
+ public static Matcher<MethodSubject> isPublic() {
+ return new TypeSafeMatcher<MethodSubject>() {
+ @Override
+ public boolean matchesSafely(final MethodSubject method) {
+ return method.isPresent() && method.isPublic();
+ }
+
+ @Override
+ public void describeTo(final Description description) {
+ description.appendText("method public");
+ }
+
+ @Override
+ public void describeMismatchSafely(final MethodSubject method, Description description) {
+ description
+ .appendText("method ").appendValue(method.getOriginalName()).appendText(" was not");
+ }
+ };
+ }
}
diff --git a/src/test/kotlinR8TestResources/class_inliner_lambda_k_style/main.kt b/src/test/kotlinR8TestResources/class_inliner_lambda_k_style/main.kt
new file mode 100644
index 0000000..136f72d
--- /dev/null
+++ b/src/test/kotlinR8TestResources/class_inliner_lambda_k_style/main.kt
@@ -0,0 +1,49 @@
+// Copyright (c) 2018, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package class_inliner_lambda_k_style
+
+private var COUNT = 0
+
+fun next() = "${COUNT++}".padStart(3, '0')
+
+fun main(args: Array<String>) {
+ testKotlinSequencesStateless(produceSequence(10))
+ testKotlinSequencesStateful(5, 2, produceSequence(10))
+}
+
+data class Record(val foo: String, val good: Boolean)
+
+@Synchronized
+fun testKotlinSequencesStateless(strings: Sequence<String>) {
+ useRecord()
+ // Stateless k-style lambda
+ strings.map { Record(it, false) }.forEach { println(it) }
+}
+
+@Synchronized
+fun testKotlinSequencesStateful(a: Int, b: Int, strings: Sequence<String>) {
+ useRecord()
+ // Big stateful k-style lambda
+ val capture = next()
+ strings.map {
+ val x = it.toInt()
+ val y = a + x
+ val z = capture.toInt() + b
+ println("logging $x/$y/$z") // Intentional
+ Record(it, y % z == 0)
+ }.forEach {
+ println(it)
+ }
+}
+
+private fun produceSequence(size: Int): Sequence<String> {
+ var count = size
+ return generateSequence { if (count-- > 0) next() else null }
+}
+
+// Need this to make sure testKotlinSequenceXXX is not processed
+// concurrently with invoke() on lambdas.
+fun useRecord() = useRecord2()
+fun useRecord2() = Record("", true)
diff --git a/src/test/sampleApks/split/AndroidManifest.xml b/src/test/sampleApks/split/AndroidManifest.xml
new file mode 100644
index 0000000..e46cd1e
--- /dev/null
+++ b/src/test/sampleApks/split/AndroidManifest.xml
@@ -0,0 +1,26 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!--
+Copyright (c) 2018, the R8 project authors. Please see the AUTHORS file
+for details. All rights reserved. Use of this source code is governed by a
+BSD-style license that can be found in the LICENSE file.
+-->
+<manifest xmlns:android="http://schemas.android.com/apk/res/android"
+ package="com.android.tools.r8.sample.split"
+ android:versionCode="1"
+ android:versionName="0.1" >
+
+ <uses-sdk android:minSdkVersion="21" />
+
+ <application
+ android:icon="@drawable/icon"
+ android:label="@string/app_name" >
+ <activity
+ android:name=".R8Activity"
+ android:label="@string/app_name" >
+ <intent-filter>
+ <action android:name="android.intent.action.MAIN" />
+ <category android:name="android.intent.category.LAUNCHER" />
+ </intent-filter>
+ </activity>
+ </application>
+</manifest>
diff --git a/src/test/sampleApks/split/assets/README.txt b/src/test/sampleApks/split/assets/README.txt
new file mode 100644
index 0000000..ecb060c
--- /dev/null
+++ b/src/test/sampleApks/split/assets/README.txt
@@ -0,0 +1 @@
+Sample split app from R8 project
diff --git a/src/test/sampleApks/split/res/drawable-mdpi/icon.png b/src/test/sampleApks/split/res/drawable-mdpi/icon.png
new file mode 100644
index 0000000..0799d58
--- /dev/null
+++ b/src/test/sampleApks/split/res/drawable-mdpi/icon.png
Binary files differ
diff --git a/src/test/sampleApks/split/res/layout/main.xml b/src/test/sampleApks/split/res/layout/main.xml
new file mode 100644
index 0000000..7859435
--- /dev/null
+++ b/src/test/sampleApks/split/res/layout/main.xml
@@ -0,0 +1,18 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!--
+Copyright (c) 2018, the R8 project authors. Please see the AUTHORS file
+for details. All rights reserved. Use of this source code is governed by a
+BSD-style license that can be found in the LICENSE file.
+-->
+<LinearLayout
+ xmlns:android="http://schemas.android.com/apk/res/android"
+ android:orientation="vertical" android:layout_width="fill_parent"
+ android:layout_height="fill_parent" android:id="@+id/MainLayout"
+ android:background="@android:color/background_light">
+
+ <Button
+ android:layout_height="wrap_content"
+ android:layout_width="match_parent" android:id="@+id/PressButton"
+ android:layout_margin="2dip"
+ android:text="Do something"/>
+</LinearLayout>
diff --git a/src/test/sampleApks/split/res/values/strings.xml b/src/test/sampleApks/split/res/values/strings.xml
new file mode 100644
index 0000000..ecac20f
--- /dev/null
+++ b/src/test/sampleApks/split/res/values/strings.xml
@@ -0,0 +1,9 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!--
+Copyright (c) 2018, the R8 project authors. Please see the AUTHORS file
+for details. All rights reserved. Use of this source code is governed by a
+BSD-style license that can be found in the LICENSE file.
+-->
+<resources>
+ <string name="app_name">R8 split app</string>
+</resources>
diff --git a/src/test/sampleApks/split/split.spec b/src/test/sampleApks/split/split.spec
new file mode 100644
index 0000000..a06e259
--- /dev/null
+++ b/src/test/sampleApks/split/split.spec
@@ -0,0 +1 @@
+com.android.tools.r8.sample.split.SplitClass:split
diff --git a/src/test/sampleApks/split/split_manifest/AndroidManifest.xml b/src/test/sampleApks/split/split_manifest/AndroidManifest.xml
new file mode 100644
index 0000000..21ea9f3
--- /dev/null
+++ b/src/test/sampleApks/split/split_manifest/AndroidManifest.xml
@@ -0,0 +1,15 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!--
+Copyright (c) 2018, the R8 project authors. Please see the AUTHORS file
+for details. All rights reserved. Use of this source code is governed by a
+BSD-style license that can be found in the LICENSE file.
+-->
+<manifest xmlns:android="http://schemas.android.com/apk/res/android"
+ package="com.android.tools.r8.sample.split"
+ split="featuresplit"
+ android:versionCode="1"
+ android:versionName="0.1" >
+
+ <uses-sdk android:minSdkVersion="21" />
+
+</manifest>
diff --git a/src/test/sampleApks/split/src/com/android/tools/r8/sample/split/R8Activity.java b/src/test/sampleApks/split/src/com/android/tools/r8/sample/split/R8Activity.java
new file mode 100644
index 0000000..a80cf55
--- /dev/null
+++ b/src/test/sampleApks/split/src/com/android/tools/r8/sample/split/R8Activity.java
@@ -0,0 +1,40 @@
+// Copyright (c) 2018, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.sample.split;
+
+import android.app.Activity;
+import android.os.Bundle;
+import com.android.tools.r8.sample.split.R;
+import com.android.tools.r8.sample.split.SplitClass;
+
+public class R8Activity extends Activity {
+ private int res = 0;
+
+ public void onCreate(Bundle savedInstanceState) {
+ super.onCreate(savedInstanceState);
+ setTheme(android.R.style.Theme_Light);
+ setContentView(R.layout.main);
+ // Currently this is split up into 100 iterations to be able to better see
+ // the impact of the jit on later versions of art.
+ long total = 0;
+ for (int i = 0; i < 100; i++) {
+ total += benchmarkCall();
+ }
+ System.out.println("Total: " + total);
+ }
+
+ public long benchmarkCall() {
+ SplitClass split = new SplitClass(3);
+ long start = System.nanoTime();
+ for (int i = 0; i < 1000; i++) {
+ // Ensure no dead code elimination.
+ res = split.calculate(i);
+ }
+ long finish = System.nanoTime();
+ long timeElapsed = finish - start;
+ System.out.println("Took: " + timeElapsed);
+ return timeElapsed;
+ }
+}
diff --git a/src/test/sampleApks/split/src/com/android/tools/r8/sample/split/SplitClass.java b/src/test/sampleApks/split/src/com/android/tools/r8/sample/split/SplitClass.java
new file mode 100644
index 0000000..9b6990d
--- /dev/null
+++ b/src/test/sampleApks/split/src/com/android/tools/r8/sample/split/SplitClass.java
@@ -0,0 +1,21 @@
+// Copyright (c) 2018, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.sample.split;
+
+public class SplitClass {
+ final int initialValue;
+
+ public SplitClass(int initialValue) {
+ this.initialValue = initialValue;
+ }
+
+ public int calculate(int x) {
+ int result = 2;
+ for (int i = 0; i < 42; i++) {
+ result += initialValue + x;
+ }
+ return result;
+ }
+}
diff --git a/third_party/gmail/gmail_android_170604.16.tar.gz.sha1 b/third_party/gmail/gmail_android_170604.16.tar.gz.sha1
index f57ba90..9d9985c 100644
--- a/third_party/gmail/gmail_android_170604.16.tar.gz.sha1
+++ b/third_party/gmail/gmail_android_170604.16.tar.gz.sha1
@@ -1 +1 @@
-161c569821a5c9b4cb8e99de764f3449191af084
\ No newline at end of file
+a6d49ef4fb2094672a6f6be039c971727cc9fd34
\ No newline at end of file
diff --git a/third_party/youtube/youtube.android_12.22.tar.gz.sha1 b/third_party/youtube/youtube.android_12.22.tar.gz.sha1
index 8f6813c..056ff59 100644
--- a/third_party/youtube/youtube.android_12.22.tar.gz.sha1
+++ b/third_party/youtube/youtube.android_12.22.tar.gz.sha1
@@ -1 +1 @@
-73c4880898d734064815d0426d8fe84ee6d075b4
\ No newline at end of file
+57b5c53a80ba010d1faef7da1b643f8c72b3e4e8
\ No newline at end of file
diff --git a/tools/build_sample_apk.py b/tools/build_sample_apk.py
index e1b2deb..90e34ed 100755
--- a/tools/build_sample_apk.py
+++ b/tools/build_sample_apk.py
@@ -13,17 +13,24 @@
import shutil
import subprocess
import sys
+import time
import utils
+import uuid
ANDROID_JAR = 'third_party/android_jar/lib-v{api}/android.jar'
DEFAULT_AAPT = 'aapt' # Assume in path.
DEFAULT_D8 = os.path.join(utils.REPO_ROOT, 'tools', 'd8.py')
+DEFAULT_DEXSPLITTER = os.path.join(utils.REPO_ROOT, 'tools', 'dexsplitter.py')
DEFAULT_JAVAC = 'javac'
SRC_LOCATION = 'src/com/android/tools/r8/sample/{app}/*.java'
DEFAULT_KEYSTORE = os.path.join(os.getenv('HOME'), '.android', 'debug.keystore')
+PACKAGE_PREFIX = 'com.android.tools.r8.sample'
+STANDARD_ACTIVITY = "R8Activity"
+BENCHMARK_ITERATIONS = 100
SAMPLE_APKS = [
- 'simple'
+ 'simple',
+ 'split'
]
def parse_options():
@@ -38,6 +45,18 @@
result.add_option('--keystore',
help='Keystore used for signing',
default=DEFAULT_KEYSTORE)
+ result.add_option('--split',
+ help='Split the app using the split.spec file',
+ default=False, action='store_true')
+ result.add_option('--install',
+ help='Install the app (including featuresplit)',
+ default=False, action='store_true')
+ result.add_option('--benchmark',
+ help='Benchmark the app on the phone with specialized markers',
+ default=False, action='store_true')
+ result.add_option('--benchmark-output-dir',
+ help='Store benchmark results here.',
+ default=None)
result.add_option('--app',
help='Which app to build',
default='simple',
@@ -72,6 +91,19 @@
def get_src_path(app):
return os.path.join(get_sample_dir(app), 'src')
+def get_dex_path(app):
+ return os.path.join(get_bin_path(app), 'classes.dex')
+
+def get_split_path(app, split):
+ return os.path.join(get_bin_path(app), split, 'classes.dex')
+
+def get_package_name(app):
+ return '%s.%s' % (PACKAGE_PREFIX, app)
+
+def get_qualified_activity(app):
+ # The activity specified to adb start is PACKAGE_NAME/.ACTIVITY
+ return '%s/.%s' % (get_package_name(app), STANDARD_ACTIVITY)
+
def run_aapt_pack(aapt, api, app):
with utils.ChangedWorkingDirectory(get_sample_dir(app)):
args = ['package',
@@ -86,6 +118,16 @@
'-G', os.path.join(get_build_dir(app), 'proguard_options')]
run_aapt(aapt, args)
+def run_aapt_split_pack(aapt, api, app):
+ with utils.ChangedWorkingDirectory(get_sample_dir(app)):
+ args = ['package',
+ '-v', '-f',
+ '-I', get_android_jar(api),
+ '-M', 'split_manifest/AndroidManifest.xml',
+ '-S', 'res',
+ '-F', os.path.join(get_bin_path(app), 'split_resources.ap_')]
+ run_aapt(aapt, args)
+
def compile_with_javac(api, app):
with utils.ChangedWorkingDirectory(get_sample_dir(app)):
files = glob.glob(SRC_LOCATION.format(app=app))
@@ -110,28 +152,142 @@
utils.PrintCmd(command)
subprocess.check_call(command)
-def create_temp_apk(app):
+def split(app):
+ split_spec = os.path.join(get_sample_dir(app), 'split.spec')
+ command = [DEFAULT_DEXSPLITTER,
+ '--input', get_dex_path(app),
+ '--output', get_bin_path(app),
+ '--feature-splits', split_spec]
+ utils.PrintCmd(command)
+ subprocess.check_call(command)
+
+def run_adb(args):
+ command = ['adb']
+ command.extend(args)
+ utils.PrintCmd(command)
+ subprocess.check_call(command)
+
+def adb_install(apks):
+ args = [
+ 'install-multiple' if len(apks) > 1 else 'install',
+ '-r',
+ '-d']
+ args.extend(apks)
+ run_adb(args)
+
+def create_temp_apk(app, prefix):
temp_apk_path = os.path.join(get_bin_path(app), '%s.ap_' % app)
- shutil.move(os.path.join(get_bin_path(app), 'resources.ap_'),
- temp_apk_path)
+ shutil.copyfile(os.path.join(get_bin_path(app), '%sresources.ap_' % prefix),
+ temp_apk_path)
return temp_apk_path
-def aapt_add_dex(aapt, app, temp_apk_path):
+def aapt_add_dex(aapt, dex, temp_apk_path):
args = ['add',
'-k', temp_apk_path,
- os.path.join(get_bin_path(app), 'classes.dex')]
+ dex]
run_aapt(aapt, args)
+def kill(app):
+ args = ['shell', 'am', 'force-stop', get_package_name(app)]
+ run_adb(args)
+
+def start_logcat():
+ return subprocess.Popen(['adb', 'logcat'], stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+
+def start(app):
+ args = ['shell', 'am', 'start', '-n', get_qualified_activity(app)]
+ run_adb(args)
+
+def clear_logcat():
+ args = ['logcat', '-c']
+ run_adb(args)
+
+def stop_logcat(popen):
+ popen.terminate()
+ lines = []
+ for l in popen.stdout:
+ if 'System.out' in l:
+ lines.append(l)
+ return lines
+
+def store_or_print_benchmarks(lines, output):
+ single_runs = []
+ total_time = None
+ # We assume that the individual runs are prefixed with 'Took: ' and the total time is
+ # prefixed with 'Total: '. The logcat lines looks like:
+ # 06-28 12:22:00.991 13698 13698 I System.out: Took: 61614
+ for l in lines:
+ if 'Took: ' in l:
+ timing = l.split('Took: ')[1]
+ single_runs.append(timing)
+ if 'Total: ' in l:
+ timing = l.split('Total: ')[1]
+ total_time = timing
+ assert len(single_runs) > 0
+ assert total_time
+ if not output:
+ print 'Individual timings: \n%s' % ''.join(single_runs)
+ print 'Total time: \n%s' % total_time
+ return
+
+ output_dir = os.path.join(output, str(uuid.uuid4()))
+ os.makedirs(output_dir)
+ single_run_file = os.path.join(output_dir, 'single_runs')
+ with open(single_run_file, 'w') as f:
+ f.writelines(single_runs)
+ total_file = os.path.join(output_dir, 'total')
+ with open(total_file, 'w') as f:
+ f.write(total_time)
+ print 'Result stored in %s and %s' % (single_run_file, total_file)
+
+def benchmark(app, output_dir):
+ # Ensure app is not running
+ kill(app)
+ clear_logcat()
+ logcat = start_logcat()
+ start(app)
+ # We could do better here by continiously parsing the logcat for a marker, but
+ # this works nicely with the current setup.
+ time.sleep(3)
+ kill(app)
+ store_or_print_benchmarks(stop_logcat(logcat), output_dir)
+
def Main():
(options, args) = parse_options()
+ apks = []
+ is_split = options.split
run_aapt_pack(options.aapt, options.api, options.app)
+ if is_split:
+ run_aapt_split_pack(options.aapt, options.api, options.app)
compile_with_javac(options.api, options.app)
dex(options.app, options.api)
- temp_apk_path = create_temp_apk(options.app)
- aapt_add_dex(options.aapt, options.app, temp_apk_path)
+ dex_files = { options.app: get_dex_path(options.app)}
+ dex_path = get_dex_path(options.app)
+ if is_split:
+ split(options.app)
+ dex_path = get_split_path(options.app, 'base')
+
+ temp_apk_path = create_temp_apk(options.app, '')
+ aapt_add_dex(options.aapt, dex_path, temp_apk_path)
apk_path = os.path.join(get_bin_path(options.app), '%s.apk' % options.app)
apk_utils.sign(temp_apk_path, apk_path, options.keystore)
- print('Apk available at: %s' % apk_path)
+ apks.append(apk_path)
+
+ if is_split:
+ split_temp_apk_path = create_temp_apk(options.app, 'split_')
+ aapt_add_dex(options.aapt,
+ get_split_path(options.app, 'split'),
+ temp_apk_path)
+ split_apk_path = os.path.join(get_bin_path(options.app), 'featuresplit.apk')
+ apk_utils.sign(temp_apk_path, split_apk_path, options.keystore)
+ apks.append(split_apk_path)
+
+ print('Generated apks available at: %s' % ' '.join(apks))
+ if options.install:
+ adb_install(apks)
+ if options.benchmark:
+ for _ in range(BENCHMARK_ITERATIONS):
+ benchmark(options.app, options.benchmark_output_dir)
if __name__ == '__main__':
sys.exit(Main())
diff --git a/tools/run_bootstrap_benchmark.py b/tools/run_bootstrap_benchmark.py
index 8230c71..bfc62e6 100755
--- a/tools/run_bootstrap_benchmark.py
+++ b/tools/run_bootstrap_benchmark.py
@@ -49,7 +49,7 @@
print "BootstrapR8Dex(CodeSize):", os.path.getsize(d8_r8_output)
dex(PINNED_PGR8_JAR, d8_pg_output)
- print "BootstrapPG(CodeSize):", os.path.getsize(PINNED_PGR8_JAR)
- print "BootstrapPGDex(CodeSize):", os.path.getsize(d8_pg_output)
+ print "BootstrapR8PG(CodeSize):", os.path.getsize(PINNED_PGR8_JAR)
+ print "BootstrapR8PGDex(CodeSize):", os.path.getsize(d8_pg_output)
- sys.exit(0)
\ No newline at end of file
+ sys.exit(0)