[dart2js] Insert HNullCheck ahead of HGetLength
HNullCheck is inserted before HGetLength when the receiver might be null.
This makes HGetLength and HFieldGet use the same pattern.
HNullCheck supports strengthening of dominated uses, which allows
specialization of string and array interceptors, leading to better code
in some cases. An example improvement is:
if (aLocale.length < 2)
return aLocale;
return J.substring$2$s(aLocale, 0, 2).toLowerCase();
-->
if (aLocale.length < 2)
return aLocale;
return B.JSString_methods.substring$2(aLocale, 0, 2).toLowerCase();
Here `aLocale` has the type String*, and the explicit null check before
`aLocale.length` strengthens the receiver at the call to `substring`.
There is very little code size change.
The inconsistency between the handling of HFieldGet and HGetlength came
to light when investigating an unexpected difference in Uri.parse between two
apps.
Change-Id: I6e76de2070ab2c0058a109896c738ea2a09b322a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/221028
Reviewed-by: Joshua Litt <joshualitt@google.com>
Commit-Queue: Stephen Adams <sra@google.com>
diff --git a/pkg/compiler/lib/src/ssa/invoke_dynamic_specializers.dart b/pkg/compiler/lib/src/ssa/invoke_dynamic_specializers.dart
index 2ceac61..ab0b1a6 100644
--- a/pkg/compiler/lib/src/ssa/invoke_dynamic_specializers.dart
+++ b/pkg/compiler/lib/src/ssa/invoke_dynamic_specializers.dart
@@ -117,9 +117,24 @@
.isEmitted;
}
- HBoundsCheck insertBoundsCheck(HInstruction indexerNode, HInstruction array,
- HInstruction indexArgument, JClosedWorld closedWorld) {
+ HBoundsCheck insertBoundsCheck(
+ HInvokeDynamic indexerNode,
+ HInstruction array,
+ HInstruction indexArgument,
+ JClosedWorld closedWorld,
+ OptimizationTestLog log) {
final abstractValueDomain = closedWorld.abstractValueDomain;
+
+ if (abstractValueDomain.isNull(array.instructionType).isPotentiallyTrue) {
+ HNullCheck check = HNullCheck(
+ array, abstractValueDomain.excludeNull(array.instructionType))
+ ..selector = indexerNode.selector
+ ..sourceInformation = indexerNode.sourceInformation;
+ log?.registerNullCheck(indexerNode, check);
+ indexerNode.block.addBefore(indexerNode, check);
+ array = check;
+ }
+
HGetLength length = HGetLength(array, abstractValueDomain.positiveIntType,
isAssignable: abstractValueDomain
.isFixedLengthJsIndexable(array.instructionType)
@@ -212,7 +227,7 @@
HInstruction checkedIndex = index;
if (requiresBoundsCheck(instruction, closedWorld)) {
checkedIndex =
- insertBoundsCheck(instruction, receiver, index, closedWorld);
+ insertBoundsCheck(instruction, receiver, index, closedWorld, log);
}
HIndexAssign converted = HIndexAssign(
closedWorld.abstractValueDomain, receiver, checkedIndex, value);
@@ -291,7 +306,7 @@
HInstruction checkedIndex = index;
if (requiresBoundsCheck(instruction, closedWorld)) {
checkedIndex =
- insertBoundsCheck(instruction, receiver, index, closedWorld);
+ insertBoundsCheck(instruction, receiver, index, closedWorld, log);
}
HIndex converted = HIndex(receiver, checkedIndex, elementType);
log?.registerIndex(instruction, converted);
@@ -322,7 +337,7 @@
if (requiresBoundsCheck(instruction, closedWorld)) {
HConstant zeroIndex = graph.addConstantInt(0, closedWorld);
HBoundsCheck check =
- insertBoundsCheck(instruction, receiver, zeroIndex, closedWorld);
+ insertBoundsCheck(instruction, receiver, zeroIndex, closedWorld, log);
HInstruction minusOne = graph.addConstantInt(-1, closedWorld);
check.inputs.add(minusOne);
minusOne.usedBy.add(check);
diff --git a/pkg/compiler/lib/src/ssa/optimize.dart b/pkg/compiler/lib/src/ssa/optimize.dart
index d600d7e..0475798 100644
--- a/pkg/compiler/lib/src/ssa/optimize.dart
+++ b/pkg/compiler/lib/src/ssa/optimize.dart
@@ -606,8 +606,20 @@
.isDefinitelyTrue) {
resultType = _abstractValueDomain.uint32Type;
}
+ HInstruction checkedReceiver = actualReceiver;
+ if (actualReceiver.isNull(_abstractValueDomain).isPotentiallyTrue) {
+ // The receiver is potentially `null` so we insert a null receiver
+ // check. This will be folded into the length access later.
+ HNullCheck check = HNullCheck(actualReceiver,
+ _abstractValueDomain.excludeNull(actualReceiver.instructionType))
+ ..selector = node.selector
+ ..sourceInformation = node.sourceInformation;
+ _log?.registerNullCheck(node, check);
+ node.block.addBefore(node, check);
+ checkedReceiver = check;
+ }
HGetLength result =
- HGetLength(actualReceiver, resultType, isAssignable: !isFixed);
+ HGetLength(checkedReceiver, resultType, isAssignable: !isFixed);
return result;
} else if (actualReceiver.isConstantMap()) {
HConstant constantInput = actualReceiver;
@@ -2607,12 +2619,14 @@
if (branch is HIf) {
if (branch.thenBlock.isLive == branch.elseBlock.isLive) return;
assert(branch.condition.isConstant());
- HBasicBlock target =
+ HBasicBlock liveSuccessor =
branch.thenBlock.isLive ? branch.thenBlock : branch.elseBlock;
- HInstruction instruction = target.first;
- while (!instruction.isControlFlow()) {
- HInstruction next = instruction.next;
+ HInstruction instruction = liveSuccessor.first;
+ // Move instructions up until the final control flow instruction or pinned
+ // HTypeKnown.
+ while (instruction.next != null) {
if (instruction is HTypeKnown && instruction.isPinned) break;
+ HInstruction next = instruction.next;
// It might be worth re-running GVN optimizations if we hoisted a
// GVN-able instructions from [target] into [block].
newGvnCandidates = newGvnCandidates || instruction.useGvn();
