Merge "Centralize kotlin package name and prevent relocate"
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index 908c4c5..b500296 100644
--- a/src/main/java/com/android/tools/r8/Version.java
+++ b/src/main/java/com/android/tools/r8/Version.java
@@ -11,7 +11,7 @@
// This field is accessed from release scripts using simple pattern matching.
// Therefore, changing this field could break our release scripts.
- public static final String LABEL = "1.4.20-dev";
+ public static final String LABEL = "1.4.21-dev";
private Version() {
}
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 ab56f30..4934336 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -956,6 +956,7 @@
// This info is used by {@link UninstantiatedTypeOptimization#rewriteInvoke} that replaces an
// invocation with null throwing code if an always-null argument is passed. Also used by Inliner
// to give a credit to null-safe code, e.g., Kotlin's null safe argument.
+ // Note that this bit set takes into account the receiver for instance methods.
private BitSet nonNullParamOrThrow = null;
// Stores information about nullability facts per parameter. If set, that means, the method
// somehow (e.g., null check, such as arg != null, or NPE-throwing instructions such as array
@@ -963,6 +964,7 @@
// guaranteed until the normal exits. That is, if the invocation of this method is finished
// normally, the recorded parameter is definitely not null. These facts are used to propagate
// non-null information through {@link NonNullTracker}.
+ // Note that this bit set takes into account the receiver for instance methods.
private BitSet nonNullParamOnNormalExits = null;
private boolean reachabilitySensitive = false;
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/type/TypeLatticeElement.java b/src/main/java/com/android/tools/r8/ir/analysis/type/TypeLatticeElement.java
index ad8edd3..14aad66 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/type/TypeLatticeElement.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/type/TypeLatticeElement.java
@@ -7,7 +7,6 @@
import com.android.tools.r8.graph.AppInfo;
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexType;
-import com.android.tools.r8.ir.code.NumericType;
import com.android.tools.r8.ir.code.Value;
/**
@@ -346,24 +345,6 @@
return appInfo.dexItemFactory.createReferenceTypeLatticeElement(type, isNullable, appInfo);
}
- public static TypeLatticeElement fromNumericType(NumericType type) {
- switch (type) {
- case BYTE:
- case CHAR:
- case SHORT:
- case INT:
- return INT;
- case LONG:
- return LONG;
- case FLOAT:
- return FLOAT;
- case DOUBLE:
- return DOUBLE;
- default:
- throw new Unreachable("Unexpected numeric type: " + type);
- }
- }
-
public boolean isValueTypeCompatible(TypeLatticeElement other) {
return (isReference() && other.isReference())
|| (isSingle() && other.isSingle())
diff --git a/src/main/java/com/android/tools/r8/ir/code/Binop.java b/src/main/java/com/android/tools/r8/ir/code/Binop.java
index 931a584..54567b5 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Binop.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Binop.java
@@ -10,6 +10,7 @@
import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.AppInfo;
import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.ir.analysis.type.PrimitiveTypeLatticeElement;
import com.android.tools.r8.ir.analysis.type.TypeLatticeElement;
import com.android.tools.r8.ir.optimize.Inliner.ConstraintWithTarget;
import com.android.tools.r8.ir.optimize.InliningConstraints;
@@ -135,7 +136,7 @@
@Override
public TypeLatticeElement evaluate(AppInfo appInfo) {
- return TypeLatticeElement.fromNumericType(type);
+ return PrimitiveTypeLatticeElement.fromNumericType(type);
}
@Override
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java b/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java
index 8f32081..6e5e5d7 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java
@@ -155,12 +155,14 @@
LoadStoreHelper loadStoreHelper = new LoadStoreHelper(code, typeVerificationHelper, appInfo);
loadStoreHelper.insertLoadsAndStores();
// Run optimizations on phis and basic blocks in a fixpoint.
- PhiOptimizations phiOptimizations = new PhiOptimizations();
- boolean reachedFixpoint = false;
- phiOptimizations.optimize(code);
- while (!reachedFixpoint) {
- BasicBlockMuncher.optimize(code);
- reachedFixpoint = !phiOptimizations.optimize(code);
+ if (!options.testing.disallowLoadStoreOptimization) {
+ PhiOptimizations phiOptimizations = new PhiOptimizations();
+ boolean reachedFixpoint = false;
+ phiOptimizations.optimize(code);
+ while (!reachedFixpoint) {
+ BasicBlockMuncher.optimize(code);
+ reachedFixpoint = !phiOptimizations.optimize(code);
+ }
}
registerAllocator = new CfRegisterAllocator(code, options, typeVerificationHelper);
registerAllocator.allocateRegisters();
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 8c16d04..4ecda95 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
@@ -1001,7 +1001,7 @@
if (options.enableNonNullTracking && nonNullTracker != null) {
// Computation of non-null parameters on normal exits rely on the existence of non-null IRs.
- nonNullTracker.computeNonNullParamOnNormalExits(feedback, method, code);
+ nonNullTracker.computeNonNullParamOnNormalExits(feedback, code);
nonNullTracker.cleanupNonNull(code);
assert code.isConsistentSSA();
}
@@ -1155,7 +1155,7 @@
private void computeNonNullParamHints(
OptimizationFeedback feedback, DexEncodedMethod method, IRCode code) {
- List<Value> arguments = code.collectArguments(true);
+ List<Value> arguments = code.collectArguments();
BitSet paramsCheckedForNull = new BitSet();
for (int index = 0; index < arguments.size(); index++) {
Value argument = arguments.get(index);
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 e08e9c4..41faa04 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
@@ -95,15 +95,11 @@
private static ConstNumber createConstNumberReplacement(
IRCode code, long constant, TypeLatticeElement typeLattice, DebugLocalInfo debugLocalInfo) {
- ConstNumber replacement;
- if (typeLattice.isReference()) {
- assert constant == 0;
- replacement = code.createConstNull();
- } else {
- Value returnedValue = code.createValue(typeLattice, debugLocalInfo);
- replacement = new ConstNumber(returnedValue, constant);
- }
- return replacement;
+ assert !typeLattice.isReference() || constant == 0;
+ Value returnedValue =
+ code.createValue(
+ typeLattice.isReference() ? TypeLatticeElement.NULL : typeLattice, debugLocalInfo);
+ return new ConstNumber(returnedValue, constant);
}
private void setValueRangeFromProguardRule(ProguardMemberRule rule, Value value) {
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/NonNullTracker.java b/src/main/java/com/android/tools/r8/ir/optimize/NonNullTracker.java
index b417c48..cbdeaac 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/NonNullTracker.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/NonNullTracker.java
@@ -124,9 +124,9 @@
current.asInvokeMethod().lookupSingleTarget(appInfo, code.method.method.getHolder());
if (singleTarget != null
&& singleTarget.getOptimizationInfo().getNonNullParamOnNormalExits() != null) {
- BitSet hints = singleTarget.getOptimizationInfo().getNonNullParamOnNormalExits();
+ BitSet facts = singleTarget.getOptimizationInfo().getNonNullParamOnNormalExits();
for (int i = 0; i < current.inValues().size(); i++) {
- if (hints.get(i)) {
+ if (facts.get(i)) {
Value knownToBeNonNullValue = current.inValues().get(i);
if (isNonNullCandidate(knownToBeNonNullValue)) {
knownToBeNonNullValues.add(knownToBeNonNullValue);
@@ -350,8 +350,7 @@
&& typeLattice.isReference();
}
- public void computeNonNullParamOnNormalExits(
- OptimizationFeedback feedback, DexEncodedMethod method, IRCode code) {
+ public void computeNonNullParamOnNormalExits(OptimizationFeedback feedback, IRCode code) {
Set<BasicBlock> normalExits = Sets.newIdentityHashSet();
normalExits.addAll(code.computeNormalExitBlocks());
DominatorTree dominatorTree = new DominatorTree(code, MAY_HAVE_UNREACHABLE_BLOCKS);
@@ -364,9 +363,10 @@
if (!argument.getTypeLattice().isReference()) {
continue;
}
- if (index == 0 && !method.accessFlags.isStatic()) {
- // The receiver is always non-null after an invocation;
+ // The receiver is always non-null on normal exits.
+ if (argument.isThis()) {
facts.set(index);
+ continue;
}
// Collect basic blocks that check nullability of the parameter.
nullCheckedBlocks.clear();
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 f9740c6..7966d3a 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
@@ -24,6 +24,7 @@
import com.android.tools.r8.graph.MethodAccessFlags;
import com.android.tools.r8.graph.ParameterAnnotationsList;
import com.android.tools.r8.graph.UseRegistry;
+import com.android.tools.r8.ir.analysis.type.PrimitiveTypeLatticeElement;
import com.android.tools.r8.ir.analysis.type.TypeLatticeElement;
import com.android.tools.r8.ir.code.Add;
import com.android.tools.r8.ir.code.BasicBlock;
@@ -273,7 +274,7 @@
inValues.add(
builder.readRegister(register, ValueTypeConstraint.fromNumericType(numericType)));
}
- TypeLatticeElement latticeElement = TypeLatticeElement.fromNumericType(numericType);
+ TypeLatticeElement latticeElement = PrimitiveTypeLatticeElement.fromNumericType(numericType);
Value outValue =
builder.writeRegister(outline.argumentCount(), latticeElement, ThrowingInfo.CAN_THROW);
Instruction newInstruction = null;
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/UninstantiatedTypeOptimization.java b/src/main/java/com/android/tools/r8/ir/optimize/UninstantiatedTypeOptimization.java
index 74e9c6a..6a43c03 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/UninstantiatedTypeOptimization.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/UninstantiatedTypeOptimization.java
@@ -498,22 +498,17 @@
return;
}
- BitSet nonNullParamHints = target.getOptimizationInfo().getNonNullParamOrThrow();
- if (nonNullParamHints != null) {
- int argumentIndex = target.isStatic() ? 0 : 1;
- int nonNullParamHintIndex = 0;
- while (argumentIndex < invoke.arguments().size()) {
- Value argument = invoke.arguments().get(argumentIndex);
- if (isAlwaysNull(argument) && nonNullParamHints.get(nonNullParamHintIndex)) {
+ BitSet facts = target.getOptimizationInfo().getNonNullParamOrThrow();
+ if (facts != null) {
+ for (int i = 0; i < invoke.arguments().size(); i++) {
+ Value argument = invoke.arguments().get(i);
+ if (isAlwaysNull(argument) && facts.get(i)) {
replaceCurrentInstructionWithThrowNull(
invoke, blockIterator, instructionIterator, code, blocksToBeRemoved);
++numberOfInvokesWithNullArgument;
return;
}
- argumentIndex++;
- nonNullParamHintIndex++;
}
- assert argumentIndex == nonNullParamHintIndex + (target.isStatic() ? 0 : 1);
}
}
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 243a0ee..c0c6e59 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -529,6 +529,7 @@
public boolean allowFailureOnInnerClassErrors = false;
public boolean noLocalsTableOnInput = false;
public boolean forceNameReflectionOptimization = false;
+ public boolean disallowLoadStoreOptimization = false;
}
private boolean hasMinApi(AndroidApiLevel level) {
diff --git a/src/test/java/com/android/tools/r8/ExternalR8TestBuilder.java b/src/test/java/com/android/tools/r8/ExternalR8TestBuilder.java
index 3b5ddd8..e9d85be 100644
--- a/src/test/java/com/android/tools/r8/ExternalR8TestBuilder.java
+++ b/src/test/java/com/android/tools/r8/ExternalR8TestBuilder.java
@@ -76,6 +76,7 @@
Collections.addAll(
command,
getJavaExecutable(),
+ "-ea",
"-cp",
r8jar.toAbsolutePath().toString(),
R8.class.getTypeName(),
diff --git a/src/test/java/com/android/tools/r8/TestCompilerBuilder.java b/src/test/java/com/android/tools/r8/TestCompilerBuilder.java
index 8276a06..cbd0cb9 100644
--- a/src/test/java/com/android/tools/r8/TestCompilerBuilder.java
+++ b/src/test/java/com/android/tools/r8/TestCompilerBuilder.java
@@ -58,7 +58,9 @@
throws CompilationFailedException;
public T addOptionsModification(Consumer<InternalOptions> optionsConsumer) {
- this.optionsConsumer = this.optionsConsumer.andThen(optionsConsumer);
+ if (optionsConsumer != null) {
+ this.optionsConsumer = this.optionsConsumer.andThen(optionsConsumer);
+ }
return self();
}
diff --git a/src/test/java/com/android/tools/r8/cf/TryRangeTest.java b/src/test/java/com/android/tools/r8/cf/TryRangeTest.java
new file mode 100644
index 0000000..08e01ec
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/cf/TryRangeTest.java
@@ -0,0 +1,41 @@
+// Copyright (c) 2019, 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.cf;
+
+import com.android.tools.r8.NeverInline;
+
+public class TryRangeTest {
+
+ @NeverInline
+ public static float doSomething(int x) throws Exception {
+ if (x == 42) {
+ throw new Exception("is 42");
+ } else {
+ return 1;
+ }
+ }
+
+ @NeverInline
+ public static void test(int count) {
+ int x = count;
+ float y;
+ if (x == 7) {
+ try {
+ y = doSomething(x);
+ } catch (Exception e) {
+ System.out.println(x);
+ return;
+ }
+ } else {
+ System.out.println(x);
+ y = 7;
+ }
+ System.out.println(y);
+ }
+
+ public static void main(String[] args) {
+ test(10);
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/cf/TryRangeTestRunner.java b/src/test/java/com/android/tools/r8/cf/TryRangeTestRunner.java
new file mode 100644
index 0000000..98542ff
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/cf/TryRangeTestRunner.java
@@ -0,0 +1,33 @@
+// Copyright (c) 2019, 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.cf;
+
+import com.android.tools.r8.CompilationMode;
+import com.android.tools.r8.TestBase;
+import org.junit.Test;
+
+/**
+ * This tests that we produce valid code when having normal-flow with exceptional edges in blocks.
+ * We might perform optimizations that add operations (dup, swap, etc.) before and after
+ * instructions that lie on the boundary of the exception table that is generated for a basic block.
+ * If live-ranges are minimized this could produce VerifyErrors. TODO(b/119771771) Will fail if
+ * shorten live ranges without shorten exception table range.
+ */
+public class TryRangeTestRunner extends TestBase {
+
+ @Test
+ public void test() throws Exception {
+ testForR8(Backend.CF)
+ .addProgramClasses(TryRangeTest.class)
+ .addKeepMainRule(TryRangeTest.class)
+ .setMode(CompilationMode.RELEASE)
+ .minification(false)
+ .noTreeShaking()
+ .enableInliningAnnotations()
+ .addOptionsModification(o -> o.testing.disallowLoadStoreOptimization = true)
+ .run(TryRangeTest.class)
+ .assertSuccess();
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/debug/DebugStreamComparator.java b/src/test/java/com/android/tools/r8/debug/DebugStreamComparator.java
index 83dbd9e..04477bd 100644
--- a/src/test/java/com/android/tools/r8/debug/DebugStreamComparator.java
+++ b/src/test/java/com/android/tools/r8/debug/DebugStreamComparator.java
@@ -9,6 +9,7 @@
import com.android.tools.r8.debug.DebugTestBase.JUnit3Wrapper.DebuggeeState;
import com.android.tools.r8.debug.DebugTestBase.JUnit3Wrapper.FrameInspector;
+import com.android.tools.r8.debug.DebugTestConfig.RuntimeKind;
import com.android.tools.r8.utils.ListUtils;
import com.android.tools.r8.utils.Pair;
import com.android.tools.r8.utils.StringUtils;
@@ -288,8 +289,10 @@
String sig = reference.getMethodSignature();
List<Variable> variables = reference.getVisibleVariables();
int frameDepth = reference.getFrameDepth();
+ RuntimeKind referenceRuntime = reference.getConfig().getRuntimeKind();
for (int i = 1; i < states.size(); i++) {
DebuggeeState state = states.get(i);
+ RuntimeKind stateRuntime = state.getConfig().getRuntimeKind();
if (verifyFiles) {
assertEquals("source file mismatch", file, state.getSourceFile());
}
@@ -304,7 +307,11 @@
"method mismatch", method + sig, state.getMethodName() + state.getMethodSignature());
}
if (verifyVariables) {
- verifyVariablesEqual(variables, state.getVisibleVariables());
+ verifyVariablesEqual(
+ referenceRuntime,
+ reference.getVisibleVariables(),
+ stateRuntime,
+ state.getVisibleVariables());
}
if (verifyStack) {
assertEquals(frameDepth, state.getFrameDepth());
@@ -312,20 +319,33 @@
FrameInspector referenceInspector = reference.getFrame(j);
FrameInspector stateInspector = state.getFrame(j);
verifyVariablesEqual(
- referenceInspector.getVisibleVariables(), stateInspector.getVisibleVariables());
+ referenceRuntime,
+ referenceInspector.getVisibleVariables(),
+ stateRuntime,
+ stateInspector.getVisibleVariables());
}
}
}
}
- private static void verifyVariablesEqual(List<Variable> xs, List<Variable> ys) {
+ private static boolean shouldIgnoreVariable(Variable variable, RuntimeKind runtime) {
+ return runtime == RuntimeKind.DEX && variable.getName().isEmpty();
+ }
+
+ private static void verifyVariablesEqual(
+ RuntimeKind xRuntime, List<Variable> xs, RuntimeKind yRuntime, List<Variable> ys) {
Map<String, Variable> map = new HashMap<>(xs.size());
for (Variable x : xs) {
- map.put(x.getName(), x);
+ if (!shouldIgnoreVariable(x, xRuntime)) {
+ map.put(x.getName(), x);
+ }
}
List<Variable> unexpected = new ArrayList<>(ys.size());
List<Pair<Variable, Variable>> different = new ArrayList<>(Math.min(xs.size(), ys.size()));
for (Variable y : ys) {
+ if (shouldIgnoreVariable(y, yRuntime)) {
+ continue;
+ }
Variable x = map.remove(y.getName());
if (x == null) {
unexpected.add(y);
diff --git a/src/test/java/com/android/tools/r8/debug/ExamplesDebugTest.java b/src/test/java/com/android/tools/r8/debug/ExamplesDebugTest.java
index 099f55b..17d5077 100644
--- a/src/test/java/com/android/tools/r8/debug/ExamplesDebugTest.java
+++ b/src/test/java/com/android/tools/r8/debug/ExamplesDebugTest.java
@@ -81,8 +81,7 @@
@Test
public void testBridgeMethod() throws Exception {
- // TODO(b/79671093): D8 has local variables with empty names.
- testDebuggingJvmOnly("bridge", "BridgeMethod");
+ testDebugging("bridge", "BridgeMethod");
}
@Test
@@ -107,8 +106,7 @@
@Test
public void testFloatingPointValuedAnnotation() throws Exception {
- // D8 has no source file.
- testDebuggingJvmOnly("floating_point_annotations", "FloatingPointValuedAnnotationTest");
+ testDebugging("floating_point_annotations", "FloatingPointValuedAnnotationTest");
}
@Test
@@ -190,14 +188,14 @@
@Test
public void testSync() throws Exception {
- // D8 has two local variables with empty names.
+ // TODO(b/79671093): Line number mismatch in D8.
testDebuggingJvmOnly("sync", "Sync");
}
@Test
public void testThrowing() throws Exception {
- // TODO(b/79671093): We don't match JVM's behavior on this example.
- testDebuggingJvmOutputOnly("throwing", "Throwing");
+ // TODO(b/79671093): D8 has unexpected variables (this in throwing.c <init>).
+ testDebuggingJvmOnly("throwing", "Throwing");
}
@Test
@@ -205,11 +203,9 @@
testDebugging("trivial", "Trivial");
}
- @Ignore("TODO(mathiasr): InvalidDebugInfoException in CfSourceCode")
@Test
public void testTryCatch() throws Exception {
- // TODO(b/79671093): We don't match JVM's behavior on this example.
- testDebuggingJvmOutputOnly("trycatch", "TryCatch");
+ testDebugging("trycatch", "TryCatch");
}
@Test
@@ -259,8 +255,7 @@
@Test
public void testRegress62300145() throws Exception {
- // D8 has no source file.
- testDebuggingJvmOnly("regress_62300145", "Regress");
+ testDebugging("regress_62300145", "Regress");
}
@Test
@@ -281,21 +276,17 @@
@Test
public void testRegress70736958() throws Exception {
- // D8 has a local variable with empty name.
- testDebuggingJvmOnly("regress_70736958", "Test");
+ testDebugging("regress_70736958", "Test");
}
- @Ignore("TODO(mathiasr): Different behavior CfSourceCode vs JarSourceCode")
@Test
public void testRegress70737019() throws Exception {
- // TODO(b/79671093): We don't match JVM's behavior on this example.
- testDebuggingJvmOutputOnly("regress_70737019", "Test");
+ testDebugging("regress_70737019", "Test");
}
@Test
public void testRegress72361252() throws Exception {
- // D8 output has variable with empty name.
- testDebuggingJvmOnly("regress_72361252", "Test");
+ testDebugging("regress_72361252", "Test");
}
@Test
@@ -341,6 +332,8 @@
.add("R8/CfSourceCode", r8cf())
.add("R8/JarSourceCode", r8jar())
.add("D8", d8())
+ // When running on CF and DEX runtimes, filter down to states within the test package.
+ .setFilter(state -> state.getClassName().startsWith(pkg))
.compare();
}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/NonNullParamTest.java b/src/test/java/com/android/tools/r8/ir/optimize/NonNullParamTest.java
index 518ea8d..44497b5 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/NonNullParamTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/NonNullParamTest.java
@@ -46,8 +46,6 @@
this.backend = backend;
}
- private void noModification(InternalOptions options) {}
-
private void disableDevirtualization(InternalOptions options) {
options.enableDevirtualization = false;
}
@@ -67,8 +65,9 @@
.enableMergeAnnotations()
.addKeepMainRule(mainClass)
.addKeepRules(
- ImmutableList.of(
- "-keepattributes InnerClasses,Signature,EnclosingMethod", "-dontobfuscate"))
+ ImmutableList.of("-keepattributes InnerClasses,Signature,EnclosingMethod"))
+ // All tests are checking if invocations to certain null-check utils are gone.
+ .noMinification()
.addOptionsModification(
options -> {
// Need to increase a little bit to inline System.out.println
@@ -82,34 +81,31 @@
@Test
public void testIntrinsics() throws Exception {
- Class mainClass = IntrinsicsDeputy.class;
+ Class<?> mainClass = IntrinsicsDeputy.class;
CodeInspector inspector =
- buildAndRun(
- mainClass, ImmutableList.of(NeverInline.class, mainClass), this::noModification);
+ buildAndRun(mainClass, ImmutableList.of(NeverInline.class, mainClass), null);
ClassSubject mainSubject = inspector.clazz(mainClass);
assertThat(mainSubject, isPresent());
- MethodSubject selfCheck = mainSubject.method("void", "selfCheck", ImmutableList.of());
+ MethodSubject selfCheck = mainSubject.uniqueMethodWithName("selfCheck");
assertThat(selfCheck, isPresent());
assertEquals(1, countCallToParamNullCheck(selfCheck));
assertEquals(1, countPrintCall(selfCheck));
assertEquals(0, countThrow(selfCheck));
- MethodSubject checkNull = mainSubject.method("void", "checkNull", ImmutableList.of());
+ MethodSubject checkNull = mainSubject.uniqueMethodWithName("checkNull");
assertThat(checkNull, isPresent());
assertEquals(1, countCallToParamNullCheck(checkNull));
assertEquals(1, countPrintCall(checkNull));
assertEquals(0, countThrow(checkNull));
- MethodSubject paramCheck =
- mainSubject.method("void", "nonNullAfterParamCheck", ImmutableList.of());
+ MethodSubject paramCheck = mainSubject.uniqueMethodWithName("nonNullAfterParamCheck");
assertThat(paramCheck, isPresent());
assertEquals(1, countPrintCall(paramCheck));
assertEquals(0, countThrow(paramCheck));
- paramCheck = mainSubject.method(
- "void", "nonNullAfterParamCheckDifferently", ImmutableList.of());
+ paramCheck = mainSubject.uniqueMethodWithName("nonNullAfterParamCheckDifferently");
assertThat(paramCheck, isPresent());
assertEquals(1, countPrintCall(paramCheck));
assertEquals(0, countThrow(paramCheck));
@@ -123,26 +119,22 @@
mainClass,
ImmutableList.of(
NeverInline.class, IntrinsicsDeputy.class, NotPinnedClass.class, mainClass),
- this::noModification);
+ null);
ClassSubject mainSubject = inspector.clazz(mainClass);
assertThat(mainSubject, isPresent());
- String argTypeName = NotPinnedClass.class.getName();
- MethodSubject checkViaCall =
- mainSubject.method("void", "checkViaCall", ImmutableList.of(argTypeName, argTypeName));
+ MethodSubject checkViaCall = mainSubject.uniqueMethodWithName("checkViaCall");
assertThat(checkViaCall, isPresent());
assertEquals(0, countActCall(checkViaCall));
assertEquals(2, countPrintCall(checkViaCall));
- MethodSubject checkViaIntrinsic =
- mainSubject.method("void", "checkViaIntrinsic", ImmutableList.of(argTypeName));
+ MethodSubject checkViaIntrinsic = mainSubject.uniqueMethodWithName("checkViaIntrinsic");
assertThat(checkViaIntrinsic, isPresent());
assertEquals(0, countCallToParamNullCheck(checkViaIntrinsic));
assertEquals(1, countPrintCall(checkViaIntrinsic));
- MethodSubject checkAtOneLevelHigher =
- mainSubject.method("void", "checkAtOneLevelHigher", ImmutableList.of(argTypeName));
+ MethodSubject checkAtOneLevelHigher = mainSubject.uniqueMethodWithName("checkAtOneLevelHigher");
assertThat(checkAtOneLevelHigher, isPresent());
assertEquals(1, countPrintCall(checkAtOneLevelHigher));
assertEquals(0, countThrow(checkAtOneLevelHigher));
@@ -156,26 +148,22 @@
mainClass,
ImmutableList.of(
NeverInline.class, IntrinsicsDeputy.class, NotPinnedClass.class, mainClass),
- this::noModification);
+ null);
ClassSubject mainSubject = inspector.clazz(mainClass);
assertThat(mainSubject, isPresent());
- String argTypeName = NotPinnedClass.class.getName();
- MethodSubject checkViaCall =
- mainSubject.method("void", "checkViaCall", ImmutableList.of(argTypeName, argTypeName));
+ MethodSubject checkViaCall = mainSubject.uniqueMethodWithName("checkViaCall");
assertThat(checkViaCall, isPresent());
assertEquals(0, countActCall(checkViaCall));
assertEquals(2, countPrintCall(checkViaCall));
- MethodSubject checkViaIntrinsic =
- mainSubject.method("void", "checkViaIntrinsic", ImmutableList.of(argTypeName));
+ MethodSubject checkViaIntrinsic = mainSubject.uniqueMethodWithName("checkViaIntrinsic");
assertThat(checkViaIntrinsic, isPresent());
assertEquals(0, countCallToParamNullCheck(checkViaIntrinsic));
assertEquals(1, countPrintCall(checkViaIntrinsic));
- MethodSubject checkAtOneLevelHigher =
- mainSubject.method("void", "checkAtOneLevelHigher", ImmutableList.of(argTypeName));
+ MethodSubject checkAtOneLevelHigher = mainSubject.uniqueMethodWithName("checkAtOneLevelHigher");
assertThat(checkAtOneLevelHigher, isPresent());
assertEquals(1, countPrintCall(checkAtOneLevelHigher));
assertEquals(0, countThrow(checkAtOneLevelHigher));
@@ -193,26 +181,22 @@
NonNullParamAfterInvokeVirtual.class,
NotPinnedClass.class,
mainClass),
- this::noModification);
+ null);
ClassSubject mainSubject = inspector.clazz(NonNullParamAfterInvokeVirtual.class);
assertThat(mainSubject, isPresent());
- String argTypeName = NotPinnedClass.class.getName();
- MethodSubject checkViaCall =
- mainSubject.method("void", "checkViaCall", ImmutableList.of(argTypeName, argTypeName));
+ MethodSubject checkViaCall = mainSubject.uniqueMethodWithName("checkViaCall");
assertThat(checkViaCall, isPresent());
assertEquals(0, countActCall(checkViaCall));
assertEquals(2, countPrintCall(checkViaCall));
- MethodSubject checkViaIntrinsic =
- mainSubject.method("void", "checkViaIntrinsic", ImmutableList.of(argTypeName));
+ MethodSubject checkViaIntrinsic = mainSubject.uniqueMethodWithName("checkViaIntrinsic");
assertThat(checkViaIntrinsic, isPresent());
assertEquals(0, countCallToParamNullCheck(checkViaIntrinsic));
assertEquals(1, countPrintCall(checkViaIntrinsic));
- MethodSubject checkAtOneLevelHigher =
- mainSubject.method("void", "checkAtOneLevelHigher", ImmutableList.of(argTypeName));
+ MethodSubject checkAtOneLevelHigher = mainSubject.uniqueMethodWithName("checkAtOneLevelHigher");
assertThat(checkAtOneLevelHigher, isPresent());
assertEquals(1, countPrintCall(checkAtOneLevelHigher));
assertEquals(0, countThrow(checkAtOneLevelHigher));
@@ -237,12 +221,7 @@
ClassSubject mainSubject = inspector.clazz(NonNullParamAfterInvokeInterface.class);
assertThat(mainSubject, isPresent());
- String argTypeName = NotPinnedClass.class.getName();
- MethodSubject checkViaCall =
- mainSubject.method(
- "void",
- "checkViaCall",
- ImmutableList.of(NonNullParamInterface.class.getName(), argTypeName, argTypeName));
+ MethodSubject checkViaCall = mainSubject.uniqueMethodWithName("checkViaCall");
assertThat(checkViaCall, isPresent());
assertEquals(0, countActCall(checkViaCall));
// The DEX backend reuses the System.out.println invoke.
diff --git a/src/test/java/com/android/tools/r8/regress/b120164595/B120164595.java b/src/test/java/com/android/tools/r8/regress/b120164595/B120164595.java
new file mode 100644
index 0000000..08e86e6
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/regress/b120164595/B120164595.java
@@ -0,0 +1,79 @@
+// Copyright (c) 2019 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.b120164595;
+
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.matchers.JUnitMatchers.containsString;
+
+import com.android.tools.r8.CompilationFailedException;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestCompileResult;
+import com.android.tools.r8.ToolHelper.DexVm;
+import com.android.tools.r8.ToolHelper.ProcessResult;
+import com.google.common.collect.ImmutableList;
+import java.io.IOException;
+import org.junit.Test;
+
+/**
+ * Regression test for art issue with multi catch-handlers.
+ * The problem is that if d8/r8 creates the same target address for two different exceptions,
+ * art will use that address to check if it was already handled.
+ */
+class TestClass {
+ public static void main(String[] args) {
+ for (int i = 0; i < 20000; i++) {
+ toBeOptimized();
+ }
+ }
+
+ private static void toBeOptimized() {
+ try {
+ willThrow();
+ } catch (IllegalStateException | NullPointerException e) {
+ if (e instanceof NullPointerException) {
+ return;
+ }
+ throw new Error("Expected NullPointerException");
+ }
+ }
+
+ private static void willThrow() {
+ throw new NullPointerException();
+ }
+}
+
+public class B120164595 extends TestBase {
+ @Test
+ public void testD8()
+ throws IOException, CompilationFailedException {
+ TestCompileResult d8Result = testForD8().addProgramClasses(TestClass.class).compile();
+ checkArt(d8Result);
+ }
+
+ @Test
+ public void testR8()
+ throws IOException, CompilationFailedException {
+ TestCompileResult r8Result = testForR8(Backend.DEX)
+ .addProgramClasses(TestClass.class)
+ .addKeepClassAndMembersRules(TestClass.class)
+ .compile();
+ checkArt(r8Result);
+ }
+
+ private void checkArt(TestCompileResult result) throws IOException {
+ ProcessResult artResult = runOnArtRaw(
+ result.app,
+ TestClass.class.getCanonicalName(),
+ builder -> {
+ builder.appendArtOption("-Xusejit:true");
+ },
+ DexVm.ART_9_0_0_HOST
+ );
+ // TODO(120164595): Remove when workaround lands.
+ assertNotEquals(artResult.exitCode, 0);
+ assertTrue(artResult.stderr.contains("Expected NullPointerException"));
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/PrintConfigurationTest.java b/src/test/java/com/android/tools/r8/shaking/PrintConfigurationTest.java
index 94c9896..9f2488e 100644
--- a/src/test/java/com/android/tools/r8/shaking/PrintConfigurationTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/PrintConfigurationTest.java
@@ -9,6 +9,7 @@
import static org.junit.Assert.assertThat;
import com.android.tools.r8.TestBase;
+import com.android.tools.r8.ToolHelper;
import com.android.tools.r8.utils.FileUtils;
import com.android.tools.r8.utils.StringUtils;
import com.google.common.base.Charsets;
@@ -84,7 +85,11 @@
"}")));
assertThat(
proguardConfigOut,
- containsString("-printconfiguration " + proguardConfigOutFile.toAbsolutePath().toString()));
+ containsString(
+ "-printconfiguration "
+ + (ToolHelper.isWindows()
+ ? ("'" + proguardConfigOutFile.toAbsolutePath().toString() + "'")
+ : proguardConfigOutFile.toAbsolutePath().toString())));
}
@Test
diff --git a/tools/historic_memory_usage.py b/tools/historic_memory_usage.py
index fec7b31..f23faaa 100755
--- a/tools/historic_memory_usage.py
+++ b/tools/historic_memory_usage.py
@@ -16,6 +16,7 @@
import sys
import utils
+MASTER_COMMITS = 'gs://r8-releases/raw/master'
APPS = ['gmscore', 'nest', 'youtube', 'gmail', 'chrome']
COMPILERS = ['d8', 'r8']
@@ -41,8 +42,9 @@
class GitCommit(object):
- def __init__(self, git_hash, destination, timestamp):
+ def __init__(self, git_hash, destination_dir, destination, timestamp):
self.git_hash = git_hash
+ self.destination_dir = destination_dir
self.destination = destination
self.timestamp = timestamp
@@ -56,8 +58,9 @@
commit_timestamp = subprocess.check_output(['git', 'show', '--no-patch',
'--no-notes', '--pretty=\'%ct\'',
hash]).strip().strip('\'')
- destination = 'gs://r8-releases/raw/master/%s/r8.jar' % hash
- commit = GitCommit(hash, destination, commit_timestamp)
+ destination_dir = '%s/%s/' % (MASTER_COMMITS, hash)
+ destination = '%s%s' % (destination_dir, 'r8.jar')
+ commit = GitCommit(hash, destination_dir, destination, commit_timestamp)
return commit
def enumerate_git_commits(options):
@@ -79,9 +82,10 @@
return commits
def get_available_commits(commits):
+ cloud_commits = subprocess.check_output(['gsutil.py', 'ls', MASTER_COMMITS]).splitlines()
available_commits = []
for commit in commits:
- if utils.cloud_storage_exists(commit.destination):
+ if commit.destination_dir in cloud_commits:
available_commits.append(commit)
return available_commits
@@ -119,7 +123,7 @@
app = options.app
compiler = options.compiler
cmd = ['tools/run_on_app.py', '--app', app, '--compiler', compiler,
- '--find-min-xmx']
+ '--no-build', '--find-min-xmx']
stdout = subprocess.check_output(cmd)
output_path = options.output or 'build'
time_commit = '%s_%s' % (commit.timestamp, commit.git_hash)
@@ -134,8 +138,14 @@
def benchmark(commits, options):
commit_permutations = permutate(len(commits))
+ count = 0
for index in commit_permutations:
+ count += 1
+ print('Running commit %s out of %s' % (count, len(commits)))
commit = commits[index]
+ if not utils.cloud_storage_exists(commit.destination):
+ # We may have a directory, but no r8.jar
+ continue
pull_r8_from_cloud(commit)
print('Running for commit: %s' % commit.git_hash)
run_on_app(options, commit)
@@ -148,7 +158,6 @@
available_commits = get_available_commits(commits)
print('Running for:')
print_commits(available_commits)
- print('')
benchmark(available_commits, options)
if __name__ == '__main__':
diff --git a/tools/run_on_as_app.py b/tools/run_on_as_app.py
index 8f37f43..56ea215 100755
--- a/tools/run_on_as_app.py
+++ b/tools/run_on_as_app.py
@@ -117,7 +117,7 @@
return subprocess.check_output(['git', 'clone', git_url]).strip()
def GitPull():
- return subprocess.check_output(['git', 'pull']).strip()
+ return subprocess.call(['git', 'pull']) == 0
def GitCheckout(file):
return subprocess.check_output(['git', 'checkout', file]).strip()
@@ -153,19 +153,34 @@
else:
return
-def BuildAppWithSelectedShrinkers(app, config, options):
+def GetResultsForApp(app, config, options):
git_repo = config['git_repo']
# Checkout and build in the build directory.
checkout_dir = os.path.join(WORKING_DIR, app)
+ result = {}
+
if not os.path.exists(checkout_dir):
with utils.ChangedWorkingDirectory(WORKING_DIR):
GitClone(git_repo)
else:
with utils.ChangedWorkingDirectory(checkout_dir):
- GitPull()
+ if not GitPull():
+ result['status'] = 'failed'
+ result['error_message'] = 'Unable to pull from remote'
+ return result
+ result['status'] = 'success'
+
+ result_per_shrinker = BuildAppWithSelectedShrinkers(
+ app, config, options, checkout_dir)
+ for shrinker, shrinker_result in result_per_shrinker.iteritems():
+ result[shrinker] = shrinker_result
+
+ return result
+
+def BuildAppWithSelectedShrinkers(app, config, options, checkout_dir):
result_per_shrinker = {}
with utils.ChangedWorkingDirectory(checkout_dir):
@@ -292,6 +307,12 @@
def LogResults(result_per_shrinker_per_app, options):
for app, result_per_shrinker in result_per_shrinker_per_app.iteritems():
print(app + ':')
+
+ if result_per_shrinker.get('status') != 'success':
+ error_message = result_per_shrinker.get('error_message')
+ print(' skipped ({})'.format(error_message))
+ continue
+
baseline = result_per_shrinker.get('proguard', {}).get('dex_size', -1)
for shrinker, result in result_per_shrinker.iteritems():
build_status = result.get('build_status')
@@ -355,12 +376,12 @@
result_per_shrinker_per_app = {}
if options.app:
- result_per_shrinker_per_app[options.app] = BuildAppWithSelectedShrinkers(
+ result_per_shrinker_per_app[options.app] = GetResultsForApp(
options.app, APPS.get(options.app), options)
else:
for app, config in APPS.iteritems():
if not config.get('skip', False):
- result_per_shrinker_per_app[app] = BuildAppWithSelectedShrinkers(
+ result_per_shrinker_per_app[app] = GetResultsForApp(
app, config, options)
LogResults(result_per_shrinker_per_app, options)
