[dart2js] Fix side-effects bugs
Make the `HInstruction.sideEffects` field final. The instruction owns the mutable SideEffects objects so it would be dangerous to make the field reference a shared SideEffects object. Update the side effects of an interface call when the target becomes known. We were essentially missing any benefit of inferred side-effects for instance members. Change-Id: I46da1d87e166864cd7fa4cee01f209bcc19986af Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/430546 Commit-Queue: Stephen Adams <sra@google.com> Reviewed-by: Nate Biggs <natebiggs@google.com>
This commit is contained in:
committed by
Commit Queue
parent
816b974c15
commit
29d5395c49
@@ -6775,7 +6775,9 @@ class KernelSsaGraphBuilder extends ir.VisitorDefault<void>
|
||||
_currentImplicitInstantiations,
|
||||
);
|
||||
}
|
||||
instruction.sideEffects = _inferredData.getSideEffectsOfElement(target);
|
||||
instruction.sideEffects.setTo(
|
||||
_inferredData.getSideEffectsOfElement(target),
|
||||
);
|
||||
instruction.allowCSE = closedWorld.annotationsData.allowCSE(target);
|
||||
instruction.allowDCE = closedWorld.annotationsData.allowDCE(target);
|
||||
push(instruction);
|
||||
@@ -7391,9 +7393,8 @@ class KernelSsaGraphBuilder extends ir.VisitorDefault<void>
|
||||
isSetter: selector.isSetter || selector.isIndexSet,
|
||||
);
|
||||
// TODO(natebiggs): Pass typeMask below and make non-nullable.
|
||||
instruction.sideEffects = _inferredData.getSideEffectsOfSelector(
|
||||
selector,
|
||||
null,
|
||||
instruction.sideEffects.setTo(
|
||||
_inferredData.getSideEffectsOfSelector(selector, null),
|
||||
);
|
||||
push(instruction);
|
||||
return instruction;
|
||||
|
||||
@@ -1172,7 +1172,7 @@ abstract class HInstruction implements SpannableWithEntity {
|
||||
/// Type of the instruction.
|
||||
late AbstractValue instructionType;
|
||||
|
||||
SideEffects sideEffects = SideEffects.empty();
|
||||
final SideEffects sideEffects = SideEffects.empty();
|
||||
bool _useGvn = false;
|
||||
|
||||
// TODO(sra): Consider whether to reduce instruction size by collecting all
|
||||
@@ -3516,7 +3516,8 @@ class HThrowExpression extends HInstruction {
|
||||
|
||||
class HAwait extends HInstruction {
|
||||
HAwait(super.value, super.type) : super._oneInput() {
|
||||
sideEffects = SideEffects();
|
||||
sideEffects.setAllSideEffects();
|
||||
sideEffects.setDependsOnSomething();
|
||||
}
|
||||
@override
|
||||
String toString() => 'await';
|
||||
@@ -3535,7 +3536,8 @@ class HYield extends HInstruction {
|
||||
SourceInformation? sourceInformation,
|
||||
) : super._oneInput() {
|
||||
this.sourceInformation = sourceInformation;
|
||||
sideEffects = SideEffects();
|
||||
sideEffects.setAllSideEffects();
|
||||
sideEffects.setDependsOnSomething();
|
||||
}
|
||||
bool hasStar;
|
||||
@override
|
||||
|
||||
@@ -1071,6 +1071,9 @@ class SsaInstructionSimplifier extends HBaseVisitor<HInstruction>
|
||||
node.element = method;
|
||||
}
|
||||
|
||||
node.sideEffects.restrictTo(
|
||||
_globalInferenceResults.inferredData.getSideEffectsOfElement(element),
|
||||
);
|
||||
if (_closedWorld.annotationsData.allowCSE(element)) {
|
||||
node.allowCSE = true;
|
||||
}
|
||||
@@ -2035,6 +2038,9 @@ class SsaInstructionSimplifier extends HBaseVisitor<HInstruction>
|
||||
if (_nativeData.isNativeMember(member)) {
|
||||
return tryInlineNativeGetter(node, member) ?? node;
|
||||
}
|
||||
node.sideEffects.restrictTo(
|
||||
_globalInferenceResults.inferredData.getSideEffectsOfElement(member),
|
||||
);
|
||||
if (_closedWorld.annotationsData.allowCSE(member)) {
|
||||
node.allowCSE = true;
|
||||
}
|
||||
|
||||
@@ -184,6 +184,10 @@ class SideEffects {
|
||||
_flags = other._flags;
|
||||
}
|
||||
|
||||
void restrictTo(SideEffects other) {
|
||||
_flags = _flags.intersection(other._flags);
|
||||
}
|
||||
|
||||
@override
|
||||
String toString() {
|
||||
StringBuffer buffer = StringBuffer();
|
||||
|
||||
Reference in New Issue
Block a user