Reapply "Enable constructor inlining for final fields in tests"
This enables constructor inlining for final fields in all tests. The optimization is still disabled by default.
This reverts commit 4ea7aa9f9f6fa9684ce6f4687e188023a1a7e0aa.
Bug: b/278964529
Bug: b/383488282
Change-Id: I9aa308d17f99b522932ef282b2ad890b192fd5f0
diff --git a/src/main/java/com/android/tools/r8/graph/AccessFlags.java b/src/main/java/com/android/tools/r8/graph/AccessFlags.java
index 0c9cb62..38c2ffe 100644
--- a/src/main/java/com/android/tools/r8/graph/AccessFlags.java
+++ b/src/main/java/com/android/tools/r8/graph/AccessFlags.java
@@ -226,6 +226,10 @@
return self();
}
+ public boolean wasFinal() {
+ return wasSet(Constants.ACC_FINAL);
+ }
+
public boolean isSynthetic() {
return isSet(Constants.ACC_SYNTHETIC);
}
diff --git a/src/main/java/com/android/tools/r8/graph/AppInfoWithClassHierarchy.java b/src/main/java/com/android/tools/r8/graph/AppInfoWithClassHierarchy.java
index 2ae1e75..6143f20 100644
--- a/src/main/java/com/android/tools/r8/graph/AppInfoWithClassHierarchy.java
+++ b/src/main/java/com/android/tools/r8/graph/AppInfoWithClassHierarchy.java
@@ -164,6 +164,17 @@
return this;
}
+ public boolean hasNonTrivialFinalizeMethod(DexClass clazz) {
+ if (clazz.isInterface()) {
+ return false;
+ }
+ DexClassAndMethod resolvedMethod =
+ resolveMethodOnClass(clazz, dexItemFactory().objectMembers.finalize).getResolutionPair();
+ return resolvedMethod != null
+ && resolvedMethod.getHolderType().isNotIdenticalTo(dexItemFactory().enumType)
+ && resolvedMethod.getHolderType().isNotIdenticalTo(dexItemFactory().objectType);
+ }
+
/** Primitive traversal over all (non-interface) superclasses of a given type. */
public <B> TraversalContinuation<B, ?> traverseSuperClasses(
DexClass clazz, TriFunction<DexType, DexClass, DexClass, TraversalContinuation<B, ?>> fn) {
diff --git a/src/main/java/com/android/tools/r8/ir/code/DefaultInstructionVisitor.java b/src/main/java/com/android/tools/r8/ir/code/DefaultInstructionVisitor.java
index a3bbdee..2e48a35 100644
--- a/src/main/java/com/android/tools/r8/ir/code/DefaultInstructionVisitor.java
+++ b/src/main/java/com/android/tools/r8/ir/code/DefaultInstructionVisitor.java
@@ -335,6 +335,11 @@
}
@Override
+ public T visit(StoreStoreFence instruction) {
+ return null;
+ }
+
+ @Override
public T visit(Sub instruction) {
return null;
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/IRMetadata.java b/src/main/java/com/android/tools/r8/ir/code/IRMetadata.java
index c0888c0..9941e6d 100644
--- a/src/main/java/com/android/tools/r8/ir/code/IRMetadata.java
+++ b/src/main/java/com/android/tools/r8/ir/code/IRMetadata.java
@@ -296,6 +296,10 @@
return get(Opcodes.STATIC_PUT);
}
+ public boolean mayHaveStoreStoreFence() {
+ return get(Opcodes.STORE_STORE_FENCE);
+ }
+
public boolean mayHaveStringSwitch() {
return get(Opcodes.STRING_SWITCH);
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/InstancePut.java b/src/main/java/com/android/tools/r8/ir/code/InstancePut.java
index 4e3f1ce..a007ce5 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InstancePut.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InstancePut.java
@@ -30,6 +30,7 @@
import com.android.tools.r8.ir.conversion.CfBuilder;
import com.android.tools.r8.ir.conversion.DexBuilder;
import com.android.tools.r8.ir.conversion.MethodConversionOptions;
+import com.android.tools.r8.ir.optimize.DeadCodeRemover.DeadInstructionResult;
import com.android.tools.r8.ir.optimize.Inliner.ConstraintWithTarget;
import com.android.tools.r8.ir.optimize.InliningConstraints;
import com.android.tools.r8.ir.regalloc.RegisterAllocator;
@@ -126,6 +127,15 @@
}
@Override
+ public DeadInstructionResult canBeDeadCode(AppView<?> appView, IRCode code) {
+ if (object().isDefinedByInstructionSatisfying(Instruction::isNewInstance)
+ && !instructionInstanceCanThrow(appView, code.context())) {
+ return DeadInstructionResult.deadIfInValueIsDead(object());
+ }
+ return DeadInstructionResult.notDead();
+ }
+
+ @Override
public boolean instructionTypeCanThrow() {
return true;
}
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 9da772e..126f23a 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
@@ -1507,6 +1507,14 @@
return null;
}
+ public boolean isStoreStoreFence() {
+ return false;
+ }
+
+ public StoreStoreFence asStoreStoreFence() {
+ return null;
+ }
+
public boolean isSwap() {
return false;
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/InstructionVisitor.java b/src/main/java/com/android/tools/r8/ir/code/InstructionVisitor.java
index 62bd3de..d54b02f 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InstructionVisitor.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InstructionVisitor.java
@@ -136,6 +136,8 @@
T visit(Store instruction);
+ T visit(StoreStoreFence instruction);
+
T visit(Sub instruction);
T visit(Swap instruction);
diff --git a/src/main/java/com/android/tools/r8/ir/code/InvokeDirect.java b/src/main/java/com/android/tools/r8/ir/code/InvokeDirect.java
index 5de30f1..b60c42e 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InvokeDirect.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InvokeDirect.java
@@ -9,6 +9,7 @@
import com.android.tools.r8.dex.code.DexInvokeDirect;
import com.android.tools.r8.dex.code.DexInvokeDirectRange;
import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.graph.DexClass;
import com.android.tools.r8.graph.DexClassAndMethod;
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexMethod;
@@ -170,7 +171,16 @@
if (getReceiver().getAliasedValue() == code.getThis()) {
return DeadInstructionResult.notDead();
}
- // Constructor calls can only be removed if the receiver is dead.
+ // Constructor calls can only be removed if the receiver is dead and there is no finalize
+ // method.
+ if (appView.hasClassHierarchy() && getReceiver().getType().isClassType()) {
+ DexType receiverType = getReceiver().getType().asClassType().getClassType();
+ DexClass receiverClass = appView.definitionFor(receiverType);
+ if (receiverClass == null
+ || appView.appInfoWithClassHierarchy().hasNonTrivialFinalizeMethod(receiverClass)) {
+ return DeadInstructionResult.notDead();
+ }
+ }
return DeadInstructionResult.deadIfInValueIsDead(getReceiver());
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/Opcodes.java b/src/main/java/com/android/tools/r8/ir/code/Opcodes.java
index 5ee18f3..64d8f37 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Opcodes.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Opcodes.java
@@ -79,4 +79,5 @@
int RECORD_FIELD_VALUES = 70;
int RESOURCE_CONST_NUMBER = 71;
int ORIGINAL_FIELD_WITNESS = 72;
+ int STORE_STORE_FENCE = 73;
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/StoreStoreFence.java b/src/main/java/com/android/tools/r8/ir/code/StoreStoreFence.java
new file mode 100644
index 0000000..71d1792
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/ir/code/StoreStoreFence.java
@@ -0,0 +1,120 @@
+// Copyright (c) 2024, 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.ir.code;
+
+import com.android.tools.r8.cf.LoadStoreHelper;
+import com.android.tools.r8.errors.Unreachable;
+import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.graph.DexClassAndMethod;
+import com.android.tools.r8.graph.ProgramMethod;
+import com.android.tools.r8.graph.UseRegistry;
+import com.android.tools.r8.ir.conversion.CfBuilder;
+import com.android.tools.r8.ir.conversion.DexBuilder;
+import com.android.tools.r8.ir.optimize.DeadCodeRemover.DeadInstructionResult;
+import com.android.tools.r8.ir.optimize.Inliner.ConstraintWithTarget;
+import com.android.tools.r8.ir.optimize.InliningConstraints;
+import com.android.tools.r8.lightir.LirBuilder;
+
+public class StoreStoreFence extends Instruction {
+
+ public StoreStoreFence(Value value) {
+ super(null, value);
+ }
+
+ @Override
+ public boolean isStoreStoreFence() {
+ return true;
+ }
+
+ @Override
+ public StoreStoreFence asStoreStoreFence() {
+ return this;
+ }
+
+ @Override
+ public int opcode() {
+ return Opcodes.STORE_STORE_FENCE;
+ }
+
+ @Override
+ public <T> T accept(InstructionVisitor<T> visitor) {
+ return visitor.visit(this);
+ }
+
+ @Override
+ public void buildCf(CfBuilder builder) {
+ throw new Unreachable();
+ }
+
+ @Override
+ public void buildDex(DexBuilder builder) {
+ throw new Unreachable();
+ }
+
+ @Override
+ public void buildLir(LirBuilder<Value, ?> builder) {
+ builder.addStoreStoreFence(getFirstOperand());
+ }
+
+ @Override
+ public DeadInstructionResult canBeDeadCode(AppView<?> appView, IRCode code) {
+ return DeadInstructionResult.deadIfInValueIsDead(getFirstOperand());
+ }
+
+ @Override
+ public boolean identicalNonValueNonPositionParts(Instruction other) {
+ return other.isStoreStoreFence();
+ }
+
+ @Override
+ public boolean instructionInstanceCanThrow(
+ AppView<?> appView,
+ ProgramMethod context,
+ AbstractValueSupplier abstractValueSupplier,
+ SideEffectAssumption assumption) {
+ return false;
+ }
+
+ @Override
+ public boolean instructionMayTriggerMethodInvocation(AppView<?> appView, ProgramMethod context) {
+ return false;
+ }
+
+ @Override
+ public boolean instructionTypeCanThrow() {
+ return true;
+ }
+
+ @Override
+ public int maxInValueRegister() {
+ throw new Unreachable();
+ }
+
+ @Override
+ public int maxOutValueRegister() {
+ throw new Unreachable();
+ }
+
+ @Override
+ public ConstraintWithTarget inliningConstraint(
+ InliningConstraints inliningConstraints, ProgramMethod context) {
+ return inliningConstraints.forStoreStoreFence();
+ }
+
+ @Override
+ public void insertLoadAndStores(LoadStoreHelper helper) {
+ throw new Unreachable();
+ }
+
+ @Override
+ public boolean hasInvariantOutType() {
+ return true;
+ }
+
+ @Override
+ void internalRegisterUse(UseRegistry<?> registry, DexClassAndMethod context) {
+ registry.registerInvokeStatic(
+ registry.dexItemFactory().javaLangInvokeVarHandleMembers.storeStoreFence);
+ }
+}
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 a17b3f1..f626f7b 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
@@ -640,7 +640,6 @@
previous = printMethod(code, "IR after inserting assume instructions (SSA)", previous);
}
-
if (inliner != null && !isDebugMode) {
timing.begin("Inlining");
inliner.performInlining(code.context(), code, feedback, methodProcessor, timing);
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/LirConverter.java b/src/main/java/com/android/tools/r8/ir/conversion/LirConverter.java
index 4a9e696..60d1cd4 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/LirConverter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/LirConverter.java
@@ -20,6 +20,7 @@
import com.android.tools.r8.ir.conversion.passes.FilledNewArrayRewriter;
import com.android.tools.r8.ir.conversion.passes.InitClassRemover;
import com.android.tools.r8.ir.conversion.passes.OriginalFieldWitnessRemover;
+import com.android.tools.r8.ir.conversion.passes.StoreStoreFenceToInvokeRewriter;
import com.android.tools.r8.ir.conversion.passes.StringSwitchConverter;
import com.android.tools.r8.ir.conversion.passes.StringSwitchRemover;
import com.android.tools.r8.ir.optimize.ConstantCanonicalizer;
@@ -154,6 +155,7 @@
new CodeRewriterPassCollection(
new AdaptClassStringsRewriter(appView),
new ConstResourceNumberRemover(appView),
+ new StoreStoreFenceToInvokeRewriter(appView),
new OriginalFieldWitnessRemover(appView),
// Must run before DexItemBasedConstStringRemover.
new StringSwitchRemover(appView),
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/MethodProcessor.java b/src/main/java/com/android/tools/r8/ir/conversion/MethodProcessor.java
index 9c8b286..e26ea21 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/MethodProcessor.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/MethodProcessor.java
@@ -8,6 +8,10 @@
public abstract class MethodProcessor {
+ public boolean hasWaves() {
+ return false;
+ }
+
public boolean isD8MethodProcessor() {
return false;
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/PostMethodProcessor.java b/src/main/java/com/android/tools/r8/ir/conversion/PostMethodProcessor.java
index d31a884..f625c8e 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/PostMethodProcessor.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/PostMethodProcessor.java
@@ -85,6 +85,11 @@
}
@Override
+ public boolean hasWaves() {
+ return true;
+ }
+
+ @Override
public boolean isPostMethodProcessor() {
return true;
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/PrimaryMethodProcessor.java b/src/main/java/com/android/tools/r8/ir/conversion/PrimaryMethodProcessor.java
index 4e3f618..788bfcb 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/PrimaryMethodProcessor.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/PrimaryMethodProcessor.java
@@ -71,6 +71,11 @@
}
@Override
+ public boolean hasWaves() {
+ return true;
+ }
+
+ @Override
public boolean isPrimaryMethodProcessor() {
return true;
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/passes/StoreStoreFenceToInvokeRewriter.java b/src/main/java/com/android/tools/r8/ir/conversion/passes/StoreStoreFenceToInvokeRewriter.java
new file mode 100644
index 0000000..51e5e6a
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/ir/conversion/passes/StoreStoreFenceToInvokeRewriter.java
@@ -0,0 +1,51 @@
+// Copyright (c) 2024, 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.ir.conversion.passes;
+
+import com.android.tools.r8.graph.AppInfo;
+import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.ir.code.IRCode;
+import com.android.tools.r8.ir.code.Instruction;
+import com.android.tools.r8.ir.code.InstructionListIterator;
+import com.android.tools.r8.ir.code.InvokeStatic;
+import com.android.tools.r8.ir.code.StoreStoreFence;
+import com.android.tools.r8.ir.conversion.MethodProcessor;
+import com.android.tools.r8.ir.conversion.passes.result.CodeRewriterResult;
+
+public class StoreStoreFenceToInvokeRewriter extends CodeRewriterPass<AppInfo> {
+
+ public StoreStoreFenceToInvokeRewriter(AppView<?> appView) {
+ super(appView);
+ }
+
+ @Override
+ protected String getRewriterId() {
+ return "StoreStoreFenceToInvokeRewriter";
+ }
+
+ @Override
+ protected boolean shouldRewriteCode(IRCode code, MethodProcessor methodProcessor) {
+ return code.metadata().mayHaveStoreStoreFence();
+ }
+
+ @Override
+ protected CodeRewriterResult rewriteCode(IRCode code) {
+ boolean hasChanged = false;
+ InstructionListIterator iterator = code.instructionListIterator();
+ while (iterator.hasNext()) {
+ Instruction current = iterator.next();
+ if (current.isStoreStoreFence()) {
+ StoreStoreFence storeStoreFence = current.asStoreStoreFence();
+ storeStoreFence.getFirstOperand().removeUser(storeStoreFence);
+ iterator.replaceCurrentInstruction(
+ InvokeStatic.builder()
+ .setMethod(appView.dexItemFactory().javaLangInvokeVarHandleMembers.storeStoreFence)
+ .setPosition(storeStoreFence)
+ .build());
+ hasChanged = true;
+ }
+ }
+ return CodeRewriterResult.hasChanged(hasChanged);
+ }
+}
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 f5c745d..e811b72 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
@@ -18,8 +18,7 @@
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.Code;
import com.android.tools.r8.graph.DefaultUseRegistryWithResult;
-import com.android.tools.r8.graph.DexEncodedField;
-import com.android.tools.r8.graph.DexField;
+import com.android.tools.r8.graph.DexClassAndField;
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexItemFactory.BoxUnboxPrimitiveMethodRoundtrip;
import com.android.tools.r8.graph.DexMethod;
@@ -726,20 +725,19 @@
} else if (instruction.isInstancePut()) {
// Final fields may not be initialized outside of a constructor in the enclosing class.
InstancePut instancePut = instruction.asInstancePut();
- DexField field = instancePut.getField();
- DexEncodedField target = appView.appInfo().lookupInstanceTarget(field);
+ DexClassAndField target =
+ instancePut.resolveField(appView, singleTarget).getResolutionPair();
if (target == null) {
whyAreYouNotInliningReporter.reportUnsafeConstructorInliningDueToMissingFieldAssignment(
instancePut);
return false;
}
- if (target.isFinal()) {
- // TODO(b/278964529): We should unset the final flag for good measure. Find a way to do
- // this in a thread safe manner.
+ if (target.getAccessFlags().isFinal()) {
if (inlinerOptions.enableConstructorInliningWithFinalFields
- && options.canAssignFinalInstanceFieldOutsideConstructor()
- && options.canUseJavaLangVarHandleStoreStoreFence(appView)) {
- actionBuilder.setShouldEnsureStoreStoreBarrier();
+ && options.canUseJavaLangVarHandleStoreStoreFence(appView)
+ && methodProcessor.hasWaves()
+ && target.isProgramField()) {
+ actionBuilder.setShouldEnsureStoreStoreFence(target.asProgramField());
} else {
whyAreYouNotInliningReporter.reportUnsafeConstructorInliningDueToFinalFieldAssignment(
instancePut);
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
index b58048e..5215acd 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
@@ -21,6 +21,7 @@
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.MethodResolutionResult.SingleResolutionResult;
import com.android.tools.r8.graph.NestMemberClassAttribute;
+import com.android.tools.r8.graph.ProgramField;
import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.ir.analysis.ClassInitializationAnalysis;
import com.android.tools.r8.ir.analysis.proto.ProtoInliningReasonStrategy;
@@ -36,14 +37,15 @@
import com.android.tools.r8.ir.code.InstructionIterator;
import com.android.tools.r8.ir.code.InstructionListIterator;
import com.android.tools.r8.ir.code.Invoke;
+import com.android.tools.r8.ir.code.InvokeDirect;
import com.android.tools.r8.ir.code.InvokeMethod;
-import com.android.tools.r8.ir.code.InvokeStatic;
import com.android.tools.r8.ir.code.InvokeVirtual;
import com.android.tools.r8.ir.code.Monitor;
import com.android.tools.r8.ir.code.MonitorType;
import com.android.tools.r8.ir.code.MoveException;
import com.android.tools.r8.ir.code.Phi;
import com.android.tools.r8.ir.code.Position;
+import com.android.tools.r8.ir.code.StoreStoreFence;
import com.android.tools.r8.ir.code.Throw;
import com.android.tools.r8.ir.code.Value;
import com.android.tools.r8.ir.conversion.IRConverter;
@@ -69,6 +71,7 @@
import com.android.tools.r8.utils.ListUtils;
import com.android.tools.r8.utils.Timing;
import com.android.tools.r8.utils.collections.LongLivedProgramMethodSetBuilder;
+import com.android.tools.r8.utils.collections.ProgramFieldSet;
import com.android.tools.r8.utils.collections.ProgramMethodMap;
import com.android.tools.r8.utils.collections.ProgramMethodSet;
import com.google.common.collect.ImmutableList;
@@ -84,6 +87,7 @@
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
+import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Consumer;
import java.util.stream.Collectors;
@@ -100,6 +104,10 @@
private final MultiCallerInliner multiCallerInliner;
+ // The set of final fields that have been assigned by a constructor that has been inlined in the
+ // current wave.
+ private final ProgramFieldSet inlinedFinalFieldsInWave = ProgramFieldSet.createConcurrent();
+
// The set of methods that have been single caller inlined in the current wave. These need to be
// pruned when the wave ends.
private final Map<DexProgramClass, ProgramMethodMap<ProgramMethod>>
@@ -109,6 +117,8 @@
private final AvailableApiExceptions availableApiExceptions;
+ private final AtomicBoolean waveDoneScheduled = new AtomicBoolean();
+
public Inliner(AppView<AppInfoWithLiveness> appView) {
this(appView, null);
}
@@ -565,7 +575,7 @@
final Reason reason;
private boolean shouldEnsureStaticInitialization;
- private boolean shouldEnsureStoreStoreBarrier;
+ private ProgramFieldSet shouldEnsureStoreStoreFenceCauses;
private DexProgramClass downcastClass;
@@ -596,8 +606,8 @@
shouldEnsureStaticInitialization = true;
}
- void setShouldEnsureStoreStoreBarrier() {
- shouldEnsureStoreStoreBarrier = true;
+ void setShouldEnsureStoreStoreFenceCauses(ProgramFieldSet shouldEnsureStoreStoreFenceCauses) {
+ this.shouldEnsureStoreStoreFenceCauses = shouldEnsureStoreStoreFenceCauses;
}
boolean mustBeInlined() {
@@ -626,9 +636,14 @@
failingBlock -> synthesizeInitClass(code, failingBlock));
}
- // Insert a store-store barrier at the end of the method.
- if (shouldEnsureStoreStoreBarrier) {
- synthesizeStoreStoreBarrier(appView, code);
+ // Insert a store-store fence at the end of the method.
+ if (invoke.isInvokeConstructor(dexItemFactory)) {
+ InvokeDirect invokeDirect = invoke.asInvokeDirect();
+ Value receiver = invokeDirect.getReceiver();
+ if (shouldEnsureStoreStoreFenceCauses != null
+ && receiver.isDefinedByInstructionSatisfying(Instruction::isNewInstance)) {
+ synthesizeStoreStoreFence(appView, code, invokeDirect);
+ }
}
// Insert a null check if this is needed to preserve the implicit null check for the receiver.
@@ -825,7 +840,8 @@
}
}
- private void synthesizeStoreStoreBarrier(AppView<AppInfoWithLiveness> appView, IRCode code) {
+ private void synthesizeStoreStoreFence(
+ AppView<AppInfoWithLiveness> appView, IRCode code, InvokeDirect invoke) {
assert target.getDefinition().isInstanceInitializer();
BasicBlockIterator blockIterator = code.listIterator();
while (blockIterator.hasNext()) {
@@ -833,18 +849,26 @@
if (!block.isReturnBlock()) {
continue;
}
- // It is an invariant that return blocks do not have catch handlers.
- assert !block.hasCatchHandlers();
- InstructionListIterator instructionIterator =
- block.listIterator(block.getInstructions().size() - 1);
- DexMethod storeStoreFenceMethod =
- appView.dexItemFactory().javaLangInvokeVarHandleMembers.storeStoreFence;
- assert appView.definitionFor(storeStoreFenceMethod) != null;
- instructionIterator.add(
- InvokeStatic.builder()
- .setMethod(storeStoreFenceMethod)
- .setPosition(Position.syntheticNone())
- .build());
+ InstructionListIterator instructionIterator;
+ if (block.hasCatchHandlers()) {
+ instructionIterator = block.listIterator();
+ Instruction throwingInstruction =
+ instructionIterator.nextUntil(Instruction::instructionTypeCanThrow);
+ if (throwingInstruction != null) {
+ instructionIterator =
+ instructionIterator
+ .splitCopyCatchHandlers(code, blockIterator, appView.options())
+ .listIterator();
+ } else {
+ Instruction previousInstruction = instructionIterator.previous();
+ assert previousInstruction.isReturn();
+ }
+ } else {
+ instructionIterator = block.listIterator(block.getInstructions().size() - 1);
+ }
+ StoreStoreFence storeStoreFence = new StoreStoreFence(invoke.getReceiver());
+ storeStoreFence.setPosition(invoke.getPosition());
+ instructionIterator.add(storeStoreFence);
}
}
@@ -867,7 +891,7 @@
private InvokeMethod invoke;
private Reason reason;
private boolean shouldEnsureStaticInitialization;
- private boolean shouldEnsureStoreStoreBarrier;
+ private ProgramFieldSet shouldEnsureStoreStoreFenceCauses;
private ProgramMethod target;
Builder setDowncastClass(DexProgramClass downcastClass) {
@@ -890,8 +914,11 @@
return this;
}
- Builder setShouldEnsureStoreStoreBarrier() {
- this.shouldEnsureStoreStoreBarrier = true;
+ Builder setShouldEnsureStoreStoreFence(ProgramField finalFieldCause) {
+ if (shouldEnsureStoreStoreFenceCauses == null) {
+ shouldEnsureStoreStoreFenceCauses = ProgramFieldSet.create();
+ }
+ shouldEnsureStoreStoreFenceCauses.add(finalFieldCause);
return this;
}
@@ -908,8 +935,8 @@
if (shouldEnsureStaticInitialization) {
action.setShouldEnsureStaticInitialization();
}
- if (shouldEnsureStoreStoreBarrier) {
- action.setShouldEnsureStoreStoreBarrier();
+ if (shouldEnsureStoreStoreFenceCauses != null) {
+ action.setShouldEnsureStoreStoreFenceCauses(shouldEnsureStoreStoreFenceCauses);
}
return action;
}
@@ -1149,6 +1176,13 @@
iterator.inlineInvoke(
appView, code, inlinee, blockIterator, blocksToRemove, action.getDowncastClass());
+ if (action.shouldEnsureStoreStoreFenceCauses != null) {
+ assert !action.shouldEnsureStoreStoreFenceCauses.isEmpty();
+ assert converter.isInWave();
+ inlinedFinalFieldsInWave.addAll(action.shouldEnsureStoreStoreFenceCauses);
+ scheduleWaveDone();
+ }
+
if (methodProcessor.getCallSiteInformation().hasSingleCallSite(singleTarget, context)) {
feedback.markInlinedIntoSingleCallSite(singleTargetMethod);
appView.withArgumentPropagator(
@@ -1157,9 +1191,7 @@
singleTarget, context, methodProcessor));
if (!(methodProcessor instanceof OneTimeMethodProcessor)) {
assert converter.isInWave();
- if (singleCallerInlinedMethodsInWave.isEmpty()) {
- converter.addWaveDoneAction(this::onWaveDone);
- }
+ scheduleWaveDone();
singleCallerInlinedMethodsInWave
.computeIfAbsent(
singleTarget.getHolder(), ignoreKey(ProgramMethodMap::createConcurrent))
@@ -1339,6 +1371,7 @@
}
private void onWaveDone() {
+ inlinedFinalFieldsInWave.forEach(field -> field.getAccessFlags().demoteFromFinal());
singleCallerInlinedMethodsInWave.forEach(
(clazz, singleCallerInlinedMethodsForClass) -> {
// Convert and remove virtual single caller inlined methods to abstract or throw null.
@@ -1384,7 +1417,15 @@
});
}
});
+ inlinedFinalFieldsInWave.clear();
singleCallerInlinedMethodsInWave.clear();
+ waveDoneScheduled.set(false);
+ }
+
+ private void scheduleWaveDone() {
+ if (!waveDoneScheduled.getAndSet(true)) {
+ converter.addWaveDoneAction(this::onWaveDone);
+ }
}
public void onLastWaveDone(
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java b/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java
index a9a29a2..1f10305 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java
@@ -269,6 +269,10 @@
return ConstraintWithTarget.ALWAYS;
}
+ public ConstraintWithTarget forStoreStoreFence() {
+ return ConstraintWithTarget.ALWAYS;
+ }
+
public ConstraintWithTarget forSwap() {
return ConstraintWithTarget.ALWAYS;
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/RedundantFieldLoadAndStoreElimination.java b/src/main/java/com/android/tools/r8/ir/optimize/RedundantFieldLoadAndStoreElimination.java
index 3570e90..b436961 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/RedundantFieldLoadAndStoreElimination.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/RedundantFieldLoadAndStoreElimination.java
@@ -335,16 +335,6 @@
}
}
- public boolean isFinal(DexClassAndField field) {
- if (field.isProgramField() || field.isClasspathField()) {
- // Treat this field as being final if it is declared final or we have determined a constant
- // value for it.
- return field.getDefinition().isFinal()
- || field.getDefinition().getOptimizationInfo().getAbstractValue().isSingleValue();
- }
- return appView.libraryMethodOptimizer().isFinalLibraryField(field.asLibraryField());
- }
-
private DexClassAndField resolveField(DexField field) {
if (appView.enableWholeProgramOptimizations()) {
SingleFieldResolutionResult resolutionResult =
@@ -456,6 +446,7 @@
|| instruction.isThrow()
|| instruction.isUnop()
|| instruction.isRecordFieldValues()
+ || instruction.isStoreStoreFence()
: "Unexpected instruction of type " + instruction.getClass().getTypeName();
}
}
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 e7fc8c5..ea79a41 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
@@ -349,6 +349,10 @@
}
}
+ if (user.isStoreStoreFence()) {
+ continue;
+ }
+
return user; // Not eligible.
}
currentUsers = indirectUsers;
@@ -706,6 +710,11 @@
continue;
}
+ if (user.isStoreStoreFence()) {
+ removeInstruction(user);
+ continue;
+ }
+
throw new Unreachable(
"Unexpected usage left in method `"
+ method.toSourceString()
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxerImpl.java b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxerImpl.java
index 1a6404b..ef6f72b 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxerImpl.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxerImpl.java
@@ -26,6 +26,7 @@
import static com.android.tools.r8.ir.code.Opcodes.RETURN;
import static com.android.tools.r8.ir.code.Opcodes.STATIC_GET;
import static com.android.tools.r8.ir.code.Opcodes.STATIC_PUT;
+import static com.android.tools.r8.ir.code.Opcodes.STORE_STORE_FENCE;
import static com.android.tools.r8.utils.MapUtils.ignoreKey;
import com.android.tools.r8.graph.AppView;
@@ -1191,6 +1192,8 @@
return analyzeReturnUser(context, enumClass);
case STATIC_PUT:
return analyzeFieldPutUser(instruction.asStaticPut(), code, context, enumClass, enumValue);
+ case STORE_STORE_FENCE:
+ return analyzeStoreStoreFenceUser();
default:
return Reason.OTHER_UNSUPPORTED_INSTRUCTION;
}
@@ -1622,6 +1625,10 @@
return Reason.ELIGIBLE;
}
+ private Reason analyzeStoreStoreFenceUser() {
+ return Reason.ELIGIBLE;
+ }
+
private void reportEnumsAnalysis() {
assert debugLogEnabled;
Reporter reporter = appView.reporter();
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfoCollector.java b/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfoCollector.java
index ab2fc60..2f09a9f 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfoCollector.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfoCollector.java
@@ -51,9 +51,7 @@
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.DexProgramClass;
import com.android.tools.r8.graph.DexType;
-import com.android.tools.r8.graph.MethodResolutionResult;
import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.ir.analysis.DeterminismAnalysis;
import com.android.tools.r8.ir.analysis.InitializedClassesOnNormalExitAnalysis;
@@ -956,11 +954,6 @@
if (method.isSynchronized()) {
// If the method is synchronized then it acquires a lock.
mayHaveSideEffects = true;
- } else if (method.isInstanceInitializer() && hasNonTrivialFinalizeMethod(context.getHolder())) {
- // If a class T overrides java.lang.Object.finalize(), then treat the constructor as having
- // side effects. This ensures that we won't remove instructions on the form `new-instance
- // {v0}, T`.
- mayHaveSideEffects = true;
} else {
mayHaveSideEffects = false;
// Otherwise, check if there is an instruction that has side effects.
@@ -987,23 +980,6 @@
}
}
- @SuppressWarnings("ReferenceEquality")
- // Returns true if the given class overrides the method `void java.lang.Object.finalize()`.
- private boolean hasNonTrivialFinalizeMethod(DexProgramClass clazz) {
- if (clazz.isInterface()) {
- return false;
- }
- DexItemFactory dexItemFactory = appView.dexItemFactory();
- MethodResolutionResult resolutionResult =
- appView
- .appInfo()
- .resolveMethodOnClassLegacy(clazz, appView.dexItemFactory().objectMembers.finalize);
- DexEncodedMethod target = resolutionResult.getSingleTarget();
- return target != null
- && target.getReference() != dexItemFactory.enumMembers.finalize
- && target.getReference() != dexItemFactory.objectMembers.finalize;
- }
-
private void computeReturnValueOnlyDependsOnArguments(
OptimizationFeedback feedback, DexEncodedMethod method, IRCode code, Timing timing) {
timing.begin("Return value only depends on argument");
diff --git a/src/main/java/com/android/tools/r8/lightir/Lir2IRConverter.java b/src/main/java/com/android/tools/r8/lightir/Lir2IRConverter.java
index fd08861..e2e645d 100644
--- a/src/main/java/com/android/tools/r8/lightir/Lir2IRConverter.java
+++ b/src/main/java/com/android/tools/r8/lightir/Lir2IRConverter.java
@@ -93,6 +93,7 @@
import com.android.tools.r8.ir.code.Shr;
import com.android.tools.r8.ir.code.StaticGet;
import com.android.tools.r8.ir.code.StaticPut;
+import com.android.tools.r8.ir.code.StoreStoreFence;
import com.android.tools.r8.ir.code.StringSwitch;
import com.android.tools.r8.ir.code.Sub;
import com.android.tools.r8.ir.code.Throw;
@@ -824,6 +825,11 @@
}
@Override
+ public void onStoreStoreFence(EV value) {
+ addInstruction(new StoreStoreFence(getValue(value)));
+ }
+
+ @Override
public void onInstanceGet(DexField field, EV object) {
Value dest = getOutValueForNextInstruction(field.getTypeElement(appView));
addInstruction(new InstanceGet(dest, getValue(object), field));
diff --git a/src/main/java/com/android/tools/r8/lightir/LirBuilder.java b/src/main/java/com/android/tools/r8/lightir/LirBuilder.java
index ba37ad1..5aedb4f 100644
--- a/src/main/java/com/android/tools/r8/lightir/LirBuilder.java
+++ b/src/main/java/com/android/tools/r8/lightir/LirBuilder.java
@@ -1110,6 +1110,11 @@
return addOneItemInstruction(LirOpcodes.INITCLASS, clazz);
}
+ public LirBuilder<V, EV> addStoreStoreFence(V value) {
+ return addInstructionTemplate(
+ LirOpcodes.STORESTOREFENCE, Collections.emptyList(), Collections.singletonList(value));
+ }
+
public LirBuilder<V, EV> addRecordFieldValues(DexField[] fields, List<V> values) {
RecordFieldValuesPayload payload = new RecordFieldValuesPayload(fields);
return addInstructionTemplate(
diff --git a/src/main/java/com/android/tools/r8/lightir/LirOpcodes.java b/src/main/java/com/android/tools/r8/lightir/LirOpcodes.java
index d1881c3..39522bf 100644
--- a/src/main/java/com/android/tools/r8/lightir/LirOpcodes.java
+++ b/src/main/java/com/android/tools/r8/lightir/LirOpcodes.java
@@ -214,6 +214,7 @@
int STRINGSWITCH = 227;
int RESOURCENUMBER = 228;
int ORIGINALFIELDWITNESS = 229;
+ int STORESTOREFENCE = 230;
static String toString(int opcode) {
switch (opcode) {
@@ -551,6 +552,8 @@
return "CHECKCAST_IGNORE_COMPAT";
case CONSTCLASS_IGNORE_COMPAT:
return "CONSTCLASS_IGNORE_COMPAT";
+ case STORESTOREFENCE:
+ return "STORESTOREFENCE";
case STRINGSWITCH:
return "STRINGSWITCH";
case RESOURCENUMBER:
diff --git a/src/main/java/com/android/tools/r8/lightir/LirParsedInstructionCallback.java b/src/main/java/com/android/tools/r8/lightir/LirParsedInstructionCallback.java
index 953890c..77a8ed6 100644
--- a/src/main/java/com/android/tools/r8/lightir/LirParsedInstructionCallback.java
+++ b/src/main/java/com/android/tools/r8/lightir/LirParsedInstructionCallback.java
@@ -169,6 +169,10 @@
onAdd(NumericType.DOUBLE, leftValueIndex, rightValueIndex);
}
+ public void onStoreStoreFence(EV value) {
+ onInstruction();
+ }
+
public void onSub(NumericType type, EV leftValueIndex, EV rightValueIndex) {
onArithmeticBinop(type, leftValueIndex, rightValueIndex);
}
@@ -1301,6 +1305,12 @@
onOriginalFieldWitness(witness, value);
return;
}
+ case LirOpcodes.STORESTOREFENCE:
+ {
+ EV value = getNextValueOperand(view);
+ onStoreStoreFence(value);
+ return;
+ }
default:
throw new Unimplemented("No dispatch for opcode " + LirOpcodes.toString(opcode));
}
diff --git a/src/main/java/com/android/tools/r8/lightir/LirPrinter.java b/src/main/java/com/android/tools/r8/lightir/LirPrinter.java
index e01877a..7269dd9 100644
--- a/src/main/java/com/android/tools/r8/lightir/LirPrinter.java
+++ b/src/main/java/com/android/tools/r8/lightir/LirPrinter.java
@@ -332,6 +332,11 @@
}
@Override
+ public void onStoreStoreFence(EV value) {
+ appendValueArguments(value);
+ }
+
+ @Override
public void onInstanceGet(DexField field, EV object) {
appendOutValue();
builder.append(field).append(' ');
diff --git a/src/main/java/com/android/tools/r8/lightir/LirSizeEstimation.java b/src/main/java/com/android/tools/r8/lightir/LirSizeEstimation.java
index 3c0961e..3ff4a45 100644
--- a/src/main/java/com/android/tools/r8/lightir/LirSizeEstimation.java
+++ b/src/main/java/com/android/tools/r8/lightir/LirSizeEstimation.java
@@ -257,6 +257,7 @@
case INVOKESPECIAL:
case INVOKESTATIC:
case INVOKEINTERFACE:
+ case STORESTOREFENCE:
return DexBase3Format.SIZE;
case INVOKEDYNAMIC:
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 915973c..b39b630 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -1081,7 +1081,7 @@
worklist.enqueueMarkMethodLiveAction(clazz.getProgramDefaultInitializer(), clazz, reason);
applyMinimumKeepInfoWhenLiveOrTargeted(
clazz.getProgramDefaultInitializer(),
- KeepMethodInfo.newEmptyJoiner().disallowOptimization());
+ KeepMethodInfo.newEmptyJoiner().disallowClosedWorldReasoning());
}
}
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 20f725b..e730c30 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -2681,11 +2681,6 @@
return !appView.enableWholeProgramOptimizations();
}
- public boolean canAssignFinalInstanceFieldOutsideConstructor() {
- // ART does not check this property.
- return isGeneratingDex();
- }
-
/**
* Dex2Oat issues a warning for abstract methods on non-abstract classes, so we never allow this.
*
@@ -2887,7 +2882,7 @@
}
public boolean canUseJavaLangVarHandleStoreStoreFence(DexDefinitionSupplier definitions) {
- if (isGeneratingDex() && hasMinApi(AndroidApiLevel.P)) {
+ if (isGeneratingDex() && hasMinApi(AndroidApiLevel.T)) {
DexItemFactory factory = definitions.dexItemFactory();
return definitions.hasDefinitionFor(factory.javaLangInvokeVarHandleMembers.storeStoreFence);
}
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/ClInitMergeSuperTypeApiLevelTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/ClInitMergeSuperTypeApiLevelTest.java
index 6584d53..df0f702 100644
--- a/src/test/java/com/android/tools/r8/classmerging/horizontal/ClInitMergeSuperTypeApiLevelTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/ClInitMergeSuperTypeApiLevelTest.java
@@ -4,6 +4,7 @@
package com.android.tools.r8.classmerging.horizontal;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isAbsentIf;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
@@ -85,7 +86,9 @@
clazz.allMethods(MethodSubject::isInstanceInitializer).size());
if (canUseExecutable()) {
MethodSubject constructorInit = clazz.init(Executable.class.getTypeName(), "int");
- assertThat(constructorInit, isPresent());
+ assertThat(
+ constructorInit,
+ isAbsentIf(parameters.canUseJavaLangInvokeVarHandleStoreStoreFence()));
} else {
MethodSubject constructorInit = clazz.init(Constructor.class.getTypeName());
assertThat(constructorInit, isPresent());
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithClassIdAndDifferentArgumentAndAssignmentOrderMergingTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithClassIdAndDifferentArgumentAndAssignmentOrderMergingTest.java
index 2ad2931..dcf7ad6 100644
--- a/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithClassIdAndDifferentArgumentAndAssignmentOrderMergingTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithClassIdAndDifferentArgumentAndAssignmentOrderMergingTest.java
@@ -54,7 +54,8 @@
assertThat(aClassSubject, isPresent());
// TODO(b/189296638): Enable constructor merging by changing the constructor
assertEquals(
- 2, aClassSubject.allMethods(FoundMethodSubject::isInstanceInitializer).size());
+ parameters.canUseJavaLangInvokeVarHandleStoreStoreFence() ? 1 : 2,
+ aClassSubject.allMethods(FoundMethodSubject::isInstanceInitializer).size());
})
.run(parameters.getRuntime(), Main.class)
.assertSuccessWithOutputLines("CD", "CD");
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithoutClassIdAfterUnusedArgumentRemovalMergingTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithoutClassIdAfterUnusedArgumentRemovalMergingTest.java
index 51c434b..6efb03f 100644
--- a/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithoutClassIdAfterUnusedArgumentRemovalMergingTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithoutClassIdAfterUnusedArgumentRemovalMergingTest.java
@@ -57,7 +57,8 @@
ClassSubject aClassSubject = inspector.clazz(A.class);
assertThat(aClassSubject, isPresent());
assertEquals(
- 1, aClassSubject.allMethods(FoundMethodSubject::isInstanceInitializer).size());
+ parameters.canUseJavaLangInvokeVarHandleStoreStoreFence() ? 0 : 1,
+ aClassSubject.allMethods(FoundMethodSubject::isInstanceInitializer).size());
})
.run(parameters.getRuntime(), Main.class)
.assertSuccessWithOutputLines("C", "D");
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithoutClassIdMergingTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithoutClassIdMergingTest.java
index 0a36734..ab46d2b 100644
--- a/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithoutClassIdMergingTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithoutClassIdMergingTest.java
@@ -55,7 +55,8 @@
ClassSubject aClassSubject = inspector.clazz(A.class);
assertThat(aClassSubject, isPresent());
assertEquals(
- 1, aClassSubject.allMethods(FoundMethodSubject::isInstanceInitializer).size());
+ parameters.canUseJavaLangInvokeVarHandleStoreStoreFence() ? 0 : 1,
+ aClassSubject.allMethods(FoundMethodSubject::isInstanceInitializer).size());
})
.run(parameters.getRuntime(), Main.class)
.assertSuccessWithOutputLines("C", "D");
diff --git a/src/test/java/com/android/tools/r8/deadcode/DeadObjectWithFinalizeTest.java b/src/test/java/com/android/tools/r8/deadcode/DeadObjectWithFinalizeTest.java
new file mode 100644
index 0000000..81959a6
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/deadcode/DeadObjectWithFinalizeTest.java
@@ -0,0 +1,51 @@
+// Copyright (c) 2024, 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.deadcode;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class DeadObjectWithFinalizeTest extends TestBase {
+
+ @Parameter(0)
+ public TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(getClass())
+ .addKeepMainRule(Main.class)
+ .setMinApi(parameters)
+ .compile()
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines("Finalize!");
+ }
+
+ static class Main {
+
+ public static void main(String[] args) {
+ new Object() {
+
+ @Override
+ protected void finalize() {
+ System.out.println("Finalize!");
+ }
+ };
+ Runtime.getRuntime().gc();
+ Runtime.getRuntime().runFinalization();
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/enumunboxing/enummerging/AbstractEnumMergingTest.java b/src/test/java/com/android/tools/r8/enumunboxing/enummerging/AbstractEnumMergingTest.java
index c96b6ab..ee83030 100644
--- a/src/test/java/com/android/tools/r8/enumunboxing/enummerging/AbstractEnumMergingTest.java
+++ b/src/test/java/com/android/tools/r8/enumunboxing/enummerging/AbstractEnumMergingTest.java
@@ -6,7 +6,6 @@
import com.android.tools.r8.NeverInline;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.enumunboxing.EnumUnboxingTestBase;
-import com.android.tools.r8.utils.codeinspector.VerticallyMergedClassesInspector;
import java.util.List;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -40,22 +39,20 @@
.addInnerClasses(getClass())
.addKeepMainRule(Main.class)
.addKeepRules(enumKeepRules.getKeepRules())
- .applyIf(
- enumKeepRules.isNone(),
- testBuilder ->
- testBuilder
- .addEnumUnboxingInspector(
- inspector ->
- inspector.assertUnboxed(MyEnum2Cases.class, myEnum1CaseSubClass))
- .addVerticallyMergedClassesInspector(
- inspector -> inspector.assertMergedIntoSubtype(MyEnum1Case.class)),
- testBuilder ->
- testBuilder
- .addKeepRules(enumKeepRules.getKeepRules())
- .addEnumUnboxingInspector(
- inspector -> inspector.assertUnboxed(MyEnum2Cases.class, MyEnum1Case.class))
- .addVerticallyMergedClassesInspector(
- VerticallyMergedClassesInspector::assertNoClassesMerged))
+ .addEnumUnboxingInspector(
+ inspector ->
+ inspector
+ .assertUnboxed(MyEnum2Cases.class)
+ // TODO(b/383488282): Should always be unboxed.
+ .assertUnboxedIf(
+ !parameters.canUseJavaLangInvokeVarHandleStoreStoreFence(),
+ myEnum1CaseSubClass))
+ .addVerticallyMergedClassesInspector(
+ inspector ->
+ inspector
+ .applyIf(
+ enumKeepRules.isNone(), i -> i.assertMergedIntoSubtype(MyEnum1Case.class))
+ .assertNoOtherClassesMerged())
.enableInliningAnnotations()
.addOptionsModification(opt -> enableEnumOptions(opt, enumValueOptimization))
.setMinApi(parameters)
diff --git a/src/test/java/com/android/tools/r8/examples/sync/SyncTestRunner.java b/src/test/java/com/android/tools/r8/examples/sync/SyncTestRunner.java
index 8502356..9cb6ee1 100644
--- a/src/test/java/com/android/tools/r8/examples/sync/SyncTestRunner.java
+++ b/src/test/java/com/android/tools/r8/examples/sync/SyncTestRunner.java
@@ -3,6 +3,8 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.examples.sync;
+import static org.junit.Assume.assumeTrue;
+
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.TestParametersCollection;
import com.android.tools.r8.examples.ExamplesTestBase;
@@ -10,7 +12,6 @@
import com.google.common.collect.ImmutableList;
import java.util.ArrayList;
import java.util.List;
-import org.junit.Assume;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -80,7 +81,7 @@
@Test
public void testDebug() throws Exception {
// TODO(b/79671093): DEX has different line number info during stepping.
- Assume.assumeTrue(parameters.isCfRuntime());
+ assumeTrue(parameters.isCfRuntime());
runTestDebugComparator();
}
}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/inliner/InlineConstructorWithFinalFieldsTest.java b/src/test/java/com/android/tools/r8/ir/optimize/inliner/InlineConstructorWithFinalFieldsTest.java
index e5cca20..6e8e2f9 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/inliner/InlineConstructorWithFinalFieldsTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/inliner/InlineConstructorWithFinalFieldsTest.java
@@ -5,9 +5,10 @@
import static com.android.tools.r8.utils.codeinspector.CodeMatchers.invokesMethod;
import static com.android.tools.r8.utils.codeinspector.Matchers.isAbsentIf;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isFinal;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.Assert.assertFalse;
import com.android.tools.r8.AlwaysInline;
import com.android.tools.r8.TestBase;
@@ -15,9 +16,9 @@
import com.android.tools.r8.TestParametersCollection;
import com.android.tools.r8.ToolHelper;
import com.android.tools.r8.references.Reference;
-import com.android.tools.r8.utils.AndroidApiLevel;
import com.android.tools.r8.utils.MethodReferenceUtils;
import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.FieldSubject;
import com.android.tools.r8.utils.codeinspector.MethodSubject;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -45,11 +46,6 @@
parameters.isDexRuntime(),
testBuilder -> testBuilder.addLibraryFiles(ToolHelper.getMostRecentAndroidJar()))
.addKeepMainRule(Main.class)
- .addOptionsModification(
- options -> {
- assertFalse(options.inlinerOptions().enableConstructorInliningWithFinalFields);
- options.inlinerOptions().enableConstructorInliningWithFinalFields = true;
- })
.enableAlwaysInliningAnnotations()
.setMinApi(parameters)
.compile()
@@ -58,17 +54,24 @@
ClassSubject mainClassSubject = inspector.clazz(Main.class);
assertThat(mainClassSubject, isPresent());
+ FieldSubject xFieldSubject = mainClassSubject.uniqueFieldWithOriginalName("x");
+ assertThat(xFieldSubject, isPresent());
+
+ FieldSubject yFieldSubject = mainClassSubject.uniqueFieldWithOriginalName("y");
+ assertThat(yFieldSubject, isPresent());
+
MethodSubject initMethodSubject = mainClassSubject.init("int", "int");
assertThat(
initMethodSubject,
- isAbsentIf(
- parameters.isDexRuntime()
- && parameters.getApiLevel().isGreaterThanOrEqualTo(AndroidApiLevel.P)));
+ isAbsentIf(parameters.canUseJavaLangInvokeVarHandleStoreStoreFence()));
MethodSubject mainMethodSubject = mainClassSubject.mainMethod();
assertThat(mainMethodSubject, isPresent());
+
if (initMethodSubject.isPresent()) {
assertThat(mainMethodSubject, invokesMethod(initMethodSubject));
+ assertThat(xFieldSubject, isFinal());
+ assertThat(yFieldSubject, isFinal());
} else {
assertThat(
mainMethodSubject,
@@ -78,6 +81,8 @@
invokesMethod(
Reference.methodFromDescriptor(
"Ljava/lang/invoke/VarHandle;", "storeStoreFence", "()V")));
+ assertThat(xFieldSubject, not(isFinal()));
+ assertThat(yFieldSubject, not(isFinal()));
}
})
.run(parameters.getRuntime(), Main.class, "20", "22")
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/unusedarguments/UnusedAnnotatedArgumentsWithMissingAnnotationsTest.java b/src/test/java/com/android/tools/r8/ir/optimize/unusedarguments/UnusedAnnotatedArgumentsWithMissingAnnotationsTest.java
index 9b6f0c7..abe8855 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/unusedarguments/UnusedAnnotatedArgumentsWithMissingAnnotationsTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/unusedarguments/UnusedAnnotatedArgumentsWithMissingAnnotationsTest.java
@@ -4,6 +4,7 @@
package com.android.tools.r8.ir.optimize.unusedarguments;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isAbsent;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
@@ -19,6 +20,8 @@
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
import org.objectweb.asm.AnnotationVisitor;
import org.objectweb.asm.ClassWriter;
import org.objectweb.asm.FieldVisitor;
@@ -29,22 +32,23 @@
public class UnusedAnnotatedArgumentsWithMissingAnnotationsTest extends TestBase
implements Opcodes {
- private final boolean enableProguardCompatibilityMode;
- private final TestParameters parameters;
+ @Parameter(0)
+ public boolean enableProguardCompatibilityMode;
- @Parameterized.Parameters(name = "{1}, compat: {0}")
+ @Parameter(1)
+ public TestParameters parameters;
+
+ @Parameters(name = "{1}, compat: {0}")
public static List<Object[]> data() {
return buildParameters(
BooleanUtils.values(), getTestParameters().withAllRuntimesAndApiLevels().build());
}
- public UnusedAnnotatedArgumentsWithMissingAnnotationsTest(
- boolean enableProguardCompatibilityMode, TestParameters parameters) {
- this.enableProguardCompatibilityMode = enableProguardCompatibilityMode;
- this.parameters = parameters;
- }
-
private void checkClass(ClassSubject clazz, String expectedAnnotationClass) {
+ if (parameters.canUseJavaLangInvokeVarHandleStoreStoreFence()) {
+ assertThat(clazz, isAbsent());
+ return;
+ }
assertThat(clazz, isPresent());
MethodSubject init = clazz.init("Test", "java.lang.String");
assertThat(init, isPresent());
diff --git a/src/test/java/com/android/tools/r8/kotlin/lambda/b148525512/B148525512.java b/src/test/java/com/android/tools/r8/kotlin/lambda/b148525512/B148525512.java
index d068e38..9c4f28b 100644
--- a/src/test/java/com/android/tools/r8/kotlin/lambda/b148525512/B148525512.java
+++ b/src/test/java/com/android/tools/r8/kotlin/lambda/b148525512/B148525512.java
@@ -4,8 +4,6 @@
package com.android.tools.r8.kotlin.lambda.b148525512;
-import static org.hamcrest.CoreMatchers.equalTo;
-
import com.android.tools.r8.DexIndexedConsumer.ArchiveConsumer;
import com.android.tools.r8.KotlinTestBase;
import com.android.tools.r8.KotlinTestParameters;
@@ -31,7 +29,7 @@
private static final Package pkg = B148525512.class.getPackage();
private static final String kotlinTestClassesPackage = pkg.getName();
private static final String baseKtClassName = kotlinTestClassesPackage + ".BaseKt";
- private static final String featureKtClassNamet = kotlinTestClassesPackage + ".FeatureKt";
+ private static final String featureKtClassName = kotlinTestClassesPackage + ".FeatureKt";
private static final String baseClassName = kotlinTestClassesPackage + ".Base";
private static Path featureApiPath;
@@ -87,10 +85,16 @@
.addProgramClasses(FeatureAPI.class)
.addKeepMainRule(baseKtClassName)
.addKeepClassAndMembersRules(baseClassName)
- .addKeepClassAndMembersRules(featureKtClassNamet)
+ .addKeepClassAndMembersRules(featureKtClassName)
.addKeepClassAndMembersRules(FeatureAPI.class)
+ .addKeepRules(
+ "-reprocessmethod class * {",
+ " void main(java.lang.String[]);",
+ " void feature(int);",
+ "}")
.addHorizontallyMergedClassesInspector(
HorizontallyMergedClassesInspector::assertNoClassesMerged)
+ .enableProguardTestOptions()
.setMinApi(parameters)
.addFeatureSplit(
builder ->
diff --git a/src/test/java/com/android/tools/r8/kotlin/reflection/ReflectiveConstructionWithInlineClassTest.java b/src/test/java/com/android/tools/r8/kotlin/reflection/ReflectiveConstructionWithInlineClassTest.java
index 4d28c48..ab5b651 100644
--- a/src/test/java/com/android/tools/r8/kotlin/reflection/ReflectiveConstructionWithInlineClassTest.java
+++ b/src/test/java/com/android/tools/r8/kotlin/reflection/ReflectiveConstructionWithInlineClassTest.java
@@ -4,7 +4,6 @@
package com.android.tools.r8.kotlin.reflection;
-
import com.android.tools.r8.KotlinCompilerTool.KotlinCompilerVersion;
import com.android.tools.r8.KotlinTestBase;
import com.android.tools.r8.KotlinTestParameters;
diff --git a/src/test/java/com/android/tools/r8/shaking/KeepClassMembersFieldTest.java b/src/test/java/com/android/tools/r8/shaking/KeepClassMembersFieldTest.java
index 31d557e..c4e838e 100644
--- a/src/test/java/com/android/tools/r8/shaking/KeepClassMembersFieldTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/KeepClassMembersFieldTest.java
@@ -49,7 +49,7 @@
assertThat(
inspector.clazz(Foo.class).uniqueFieldWithOriginalName("value"), isPresent()))
.run(parameters.getRuntime(), Foo.class)
- .assertSuccessWithEmptyOutput();
+ .assertSuccessWithOutputLines("Foo");
}
static class Bar {}
@@ -59,7 +59,12 @@
Bar value = new Bar();
public static void main(String[] args) {
- new Foo();
+ System.out.println(new Foo());
+ }
+
+ @Override
+ public String toString() {
+ return "Foo";
}
}
}
diff --git a/src/test/testbase/java/com/android/tools/r8/TestCompilerBuilder.java b/src/test/testbase/java/com/android/tools/r8/TestCompilerBuilder.java
index 2a9697a..f4f0034 100644
--- a/src/test/testbase/java/com/android/tools/r8/TestCompilerBuilder.java
+++ b/src/test/testbase/java/com/android/tools/r8/TestCompilerBuilder.java
@@ -67,6 +67,7 @@
options.testing.listIterationRewritingEnabled = true;
options.horizontalClassMergerOptions().enable();
options.horizontalClassMergerOptions().setEnableInterfaceMerging();
+ options.inlinerOptions().enableConstructorInliningWithFinalFields = true;
options
.getCfCodeAnalysisOptions()
.setAllowUnreachableCfBlocks(false)
diff --git a/src/test/testbase/java/com/android/tools/r8/TestParameters.java b/src/test/testbase/java/com/android/tools/r8/TestParameters.java
index fc90c26..ab3077c 100644
--- a/src/test/testbase/java/com/android/tools/r8/TestParameters.java
+++ b/src/test/testbase/java/com/android/tools/r8/TestParameters.java
@@ -85,6 +85,10 @@
.isGreaterThanOrEqualTo(TestBase.apiLevelWithDefaultInterfaceMethodsSupport());
}
+ public boolean canUseJavaLangInvokeVarHandleStoreStoreFence() {
+ return isDexRuntime() && getApiLevel().isGreaterThanOrEqualTo(AndroidApiLevel.T);
+ }
+
public boolean canUseNativeDexPC() {
assert isCfRuntime() || isDexRuntime();
return isDexRuntime() && getDexRuntimeVersion().isNewerThanOrEqual(DexVm.Version.V8_1_0);
