Version 2.1.32 Cherry pick: Add synchronization to call site optimization assert CL: https://r8-review.googlesource.com/c/r8/+/51860 Cherry pick: Fix missing type propagation in constant propagation CL: https://r8-review.googlesource.com/c/r8/+/51706 Cherry pick: Fix inadequate type propagation in EnumValueOptimizer CL: https://r8-review.googlesource.com/c/r8/+/51783 Cherry pick: Add missing type parameter in TestDiagnosticMessages. CL: https://r8-review.googlesource.com/c/r8/+/51697 This version also fixes a use of assertSuccessWithEmptyOutput() in DeadConstructorWithCycleTest which does not exist on 2.1. Bug: 157546167 Change-Id: I4e7b0255d645dba75d2f3e5d99ee31cbf00f1a4b
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java index 0a02824..0c85791 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 = "2.1.31"; + public static final String LABEL = "2.1.32"; private Version() { }
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/constant/SparseConditionalConstantPropagation.java b/src/main/java/com/android/tools/r8/ir/analysis/constant/SparseConditionalConstantPropagation.java index be72b24..54d5160 100644 --- a/src/main/java/com/android/tools/r8/ir/analysis/constant/SparseConditionalConstantPropagation.java +++ b/src/main/java/com/android/tools/r8/ir/analysis/constant/SparseConditionalConstantPropagation.java
@@ -3,6 +3,8 @@ // BSD-style license that can be found in the LICENSE file. package com.android.tools.r8.ir.analysis.constant; +import com.android.tools.r8.graph.AppView; +import com.android.tools.r8.ir.analysis.type.TypeAnalysis; import com.android.tools.r8.ir.code.BasicBlock; import com.android.tools.r8.ir.code.ConstNumber; import com.android.tools.r8.ir.code.IRCode; @@ -14,6 +16,7 @@ import com.android.tools.r8.ir.code.Phi; import com.android.tools.r8.ir.code.StringSwitch; import com.android.tools.r8.ir.code.Value; +import com.google.common.collect.Sets; import java.util.ArrayList; import java.util.BitSet; import java.util.Deque; @@ -21,6 +24,7 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Set; /** * Implementation of Sparse Conditional Constant Propagation from the paper of Wegman and Zadeck @@ -29,6 +33,7 @@ */ public class SparseConditionalConstantPropagation { + private final AppView<?> appView; private final IRCode code; private final Map<Value, LatticeElement> mapping = new HashMap<>(); private final Deque<Value> ssaEdges = new LinkedList<>(); @@ -37,7 +42,8 @@ private final BitSet[] executableFlowEdges; private final BitSet visitedBlocks; - public SparseConditionalConstantPropagation(IRCode code) { + public SparseConditionalConstantPropagation(AppView<?> appView, IRCode code) { + this.appView = appView; this.code = code; nextBlockNumber = code.getHighestBlockNumber() + 1; executableFlowEdges = new BitSet[nextBlockNumber]; @@ -45,7 +51,6 @@ } public void run() { - BasicBlock firstBlock = code.entryBlock(); visitInstructions(firstBlock); @@ -77,8 +82,8 @@ } private void rewriteCode() { + Set<Value> affectedValues = Sets.newIdentityHashSet(); List<BasicBlock> blockToAnalyze = new ArrayList<>(); - mapping.entrySet().stream() .filter(entry -> entry.getValue().isConst()) .forEach( @@ -86,6 +91,7 @@ Value value = entry.getKey(); ConstNumber evaluatedConst = entry.getValue().asConst().getConstNumber(); if (value.definition != evaluatedConst) { + value.addAffectedValuesTo(affectedValues); if (value.isPhi()) { // D8 relies on dead code removal to get rid of the dead phi itself. if (value.hasAnyUsers()) { @@ -112,11 +118,12 @@ } } }); - for (BasicBlock block : blockToAnalyze) { block.deduplicatePhis(); } - + if (!affectedValues.isEmpty()) { + new TypeAnalysis(appView).narrowing(affectedValues); + } code.removeAllDeadAndTrivialPhis(); }
diff --git a/src/main/java/com/android/tools/r8/ir/code/ConstString.java b/src/main/java/com/android/tools/r8/ir/code/ConstString.java index 453c161..0a76de9 100644 --- a/src/main/java/com/android/tools/r8/ir/code/ConstString.java +++ b/src/main/java/com/android/tools/r8/ir/code/ConstString.java
@@ -3,6 +3,8 @@ // BSD-style license that can be found in the LICENSE file. package com.android.tools.r8.ir.code; +import static com.android.tools.r8.ir.analysis.type.Nullability.definitelyNotNull; + import com.android.tools.r8.cf.LoadStoreHelper; import com.android.tools.r8.cf.TypeVerificationHelper; import com.android.tools.r8.cf.code.CfConstString; @@ -11,7 +13,6 @@ import com.android.tools.r8.graph.DexString; import com.android.tools.r8.graph.DexType; import com.android.tools.r8.graph.ProgramMethod; -import com.android.tools.r8.ir.analysis.type.Nullability; import com.android.tools.r8.ir.analysis.type.TypeElement; import com.android.tools.r8.ir.analysis.value.AbstractValue; import com.android.tools.r8.ir.analysis.value.UnknownValue; @@ -158,7 +159,7 @@ @Override public TypeElement evaluate(AppView<?> appView) { - return TypeElement.stringClassType(appView, Nullability.definitelyNotNull()); + return TypeElement.stringClassType(appView, definitelyNotNull()); } @Override @@ -174,4 +175,12 @@ } return UnknownValue.getInstance(); } + + @Override + public boolean verifyTypes(AppView<?> appView) { + assert super.verifyTypes(appView); + TypeElement expectedType = TypeElement.stringClassType(appView, definitelyNotNull()); + assert getOutType().equals(expectedType); + return true; + } }
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 c0f75e9..75627a0 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
@@ -1373,7 +1373,7 @@ codeRewriter.splitRangeInvokeConstants(code); timing.end(); timing.begin("Propogate sparse conditionals"); - new SparseConditionalConstantPropagation(code).run(); + new SparseConditionalConstantPropagation(appView, code).run(); timing.end(); timing.begin("Rewrite always throwing invokes"); codeRewriter.processMethodsNeverReturningNormally(code);
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/CallSiteOptimizationInfoPropagator.java b/src/main/java/com/android/tools/r8/ir/optimize/CallSiteOptimizationInfoPropagator.java index 9708120..46508d3 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/CallSiteOptimizationInfoPropagator.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/CallSiteOptimizationInfoPropagator.java
@@ -35,6 +35,7 @@ import com.android.tools.r8.shaking.AppInfoWithLiveness; import com.android.tools.r8.utils.BooleanUtils; import com.android.tools.r8.utils.ForEachable; +import com.android.tools.r8.utils.InternalOptions; import com.android.tools.r8.utils.InternalOptions.CallSiteOptimizationOptions; import com.android.tools.r8.utils.LazyBox; import com.android.tools.r8.utils.Timing; @@ -276,7 +277,13 @@ } private void abandonCallSitePropagation(ForEachable<ProgramMethod> methods) { - methods.forEach(method -> method.getDefinition().abandonCallSiteOptimizationInfo()); + if (InternalOptions.assertionsEnabled()) { + synchronized (this) { + methods.forEach(method -> method.getDefinition().abandonCallSiteOptimizationInfo()); + } + } else { + methods.forEach(method -> method.getDefinition().abandonCallSiteOptimizationInfo()); + } } private CallSiteOptimizationInfo computeCallSiteOptimizationInfoFromArguments( @@ -435,7 +442,7 @@ return null; } - private boolean verifyAllProgramDispatchTargetsHaveBeenAbandoned( + private synchronized boolean verifyAllProgramDispatchTargetsHaveBeenAbandoned( InvokeMethod invoke, ProgramMethod context) { ProgramMethodSet targets = invoke.lookupProgramDispatchTargets(appView, context); if (targets != null) {
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumValueOptimizer.java b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumValueOptimizer.java index 1d6668c..bac8a81 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumValueOptimizer.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumValueOptimizer.java
@@ -4,6 +4,8 @@ package com.android.tools.r8.ir.optimize.enums; +import static com.android.tools.r8.ir.analysis.type.Nullability.definitelyNotNull; + import com.android.tools.r8.graph.AppView; import com.android.tools.r8.graph.DexClass; import com.android.tools.r8.graph.DexEncodedMethod; @@ -15,6 +17,7 @@ import com.android.tools.r8.graph.EnumValueInfoMapCollection.EnumValueInfo; import com.android.tools.r8.graph.EnumValueInfoMapCollection.EnumValueInfoMap; import com.android.tools.r8.ir.analysis.type.TypeAnalysis; +import com.android.tools.r8.ir.analysis.type.TypeElement; import com.android.tools.r8.ir.code.ArrayGet; import com.android.tools.r8.ir.code.BasicBlock; import com.android.tools.r8.ir.code.BasicBlock.ThrowingInfo; @@ -61,6 +64,7 @@ return; } + Set<Value> affectedValues = Sets.newIdentityHashSet(); InstructionListIterator iterator = code.instructionListIterator(); while (iterator.hasNext()) { Instruction current = iterator.next(); @@ -103,8 +107,12 @@ if (isOrdinalInvoke) { iterator.replaceCurrentInstruction(new ConstNumber(outValue, valueInfo.ordinal)); } else if (isNameInvoke) { + Value newValue = + code.createValue(TypeElement.stringClassType(appView, definitelyNotNull())); iterator.replaceCurrentInstruction( - new ConstString(outValue, enumField.name, ThrowingInfo.NO_THROW)); + new ConstString( + newValue, enumField.name, ThrowingInfo.defaultForConstString(appView.options()))); + newValue.addAffectedValuesTo(affectedValues); } else { assert isToStringInvoke; DexClass enumClazz = appView.appInfo().definitionFor(enumField.type); @@ -119,8 +127,12 @@ if (singleTarget != null && singleTarget.method != factory.enumMethods.toString) { continue; } + Value newValue = + code.createValue(TypeElement.stringClassType(appView, definitelyNotNull())); iterator.replaceCurrentInstruction( - new ConstString(outValue, enumField.name, ThrowingInfo.NO_THROW)); + new ConstString( + newValue, enumField.name, ThrowingInfo.defaultForConstString(appView.options()))); + newValue.addAffectedValuesTo(affectedValues); } } else if (current.isArrayLength()) { // Rewrites MyEnum.values().length to a constant int. @@ -140,6 +152,9 @@ } } } + if (!affectedValues.isEmpty()) { + new TypeAnalysis(appView).narrowing(affectedValues); + } assert code.isConsistentSSA(); }
diff --git a/src/test/java/com/android/tools/r8/TestDiagnosticMessages.java b/src/test/java/com/android/tools/r8/TestDiagnosticMessages.java index ac3ef82..ed0e8f1 100644 --- a/src/test/java/com/android/tools/r8/TestDiagnosticMessages.java +++ b/src/test/java/com/android/tools/r8/TestDiagnosticMessages.java
@@ -36,7 +36,7 @@ // Match exact. - default TestDiagnosticMessages assertDiagnosticsMatch(Matcher matcher) { + default TestDiagnosticMessages assertDiagnosticsMatch(Matcher<Diagnostic> matcher) { return assertDiagnosticsMatch(Collections.singletonList(matcher)); }
diff --git a/src/test/java/com/android/tools/r8/ir/analysis/sideeffect/DeadConstructorWithCycleTest.java b/src/test/java/com/android/tools/r8/ir/analysis/sideeffect/DeadConstructorWithCycleTest.java index e64fedd..ef95b71 100644 --- a/src/test/java/com/android/tools/r8/ir/analysis/sideeffect/DeadConstructorWithCycleTest.java +++ b/src/test/java/com/android/tools/r8/ir/analysis/sideeffect/DeadConstructorWithCycleTest.java
@@ -40,7 +40,7 @@ .compile() .inspect(inspector -> assertThat(inspector.clazz(A.class), not(isPresent()))) .run(parameters.getRuntime(), TestClass.class) - .assertSuccessWithEmptyOutput(); + .assertSuccessWithOutput(""); } static class TestClass {