Skip to content

Commit e66a0cb

Browse files
rakudramaCommit Queue
authored and
Commit Queue
committed
[dart2js] Unmodifiable Array and typed data using flags
- Add SSA HArrayFlags{Check,Get,Set} instructions to check for fixed-length and unmodifiable JSArray and JavaScript typed data instances. - Add optimizations to remove redundant checks. - Added HOutputConstrained interface for instructions that must have the input and output in the same JavaScript variable. This generalizes some code generation logic already applied to HCheck. Bug: #53785 Change-Id: I61750dd03aa3a964eed3bc76e1656c5f60f77109 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/372002 Commit-Queue: Stephen Adams <[email protected]> Reviewed-by: Mayank Patke <[email protected]>
1 parent 2076529 commit e66a0cb

34 files changed

+1824
-476
lines changed

pkg/compiler/lib/src/common/elements.dart

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -820,6 +820,9 @@ abstract class CommonElements {
820820
FunctionEntity get cyclicThrowHelper =>
821821
_findHelperFunction("throwCyclicInit");
822822

823+
FunctionEntity get throwUnsupportedOperation =>
824+
_findHelperFunction('throwUnsupportedOperation');
825+
823826
FunctionEntity get defineProperty => _findHelperFunction('defineProperty');
824827

825828
FunctionEntity get throwLateFieldNI =>

pkg/compiler/lib/src/js_backend/namer.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2192,6 +2192,8 @@ class FixedNames {
21922192

21932193
String get recordShapeRecipe => r'$recipe';
21942194
String get recordShapeTag => r'$shape';
2195+
2196+
String get arrayFlagsPropertyName => r'$flags';
21952197
}
21962198

21972199
/// Minified version of the fixed names usage by the namer.

pkg/compiler/lib/src/js_emitter/interceptor_stub_generator.dart

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
library dart2js.js_emitter.interceptor_stub_generator;
66

7+
import 'package:js_runtime/synced/array_flags.dart';
78
import 'package:js_runtime/synced/embedded_names.dart' as embeddedNames;
89

910
import '../common/elements.dart';
@@ -409,10 +410,14 @@ class InterceptorStubGenerator {
409410

410411
return js.statement(r'''
411412
if (typeof a0 === "number")
412-
if (# && !receiver.immutable$list &&
413+
if (# && !(receiver.# & #) &&
413414
(a0 >>> 0) === a0 && a0 < receiver.length)
414415
return receiver[a0] = a1;
415-
''', typeCheck);
416+
''', [
417+
typeCheck,
418+
_namer.fixedNames.arrayFlagsPropertyName,
419+
js.number(ArrayFlags.unmodifiableCheck)
420+
]);
416421
}
417422
} else if (selector.isCall) {
418423
if (selector.name == 'abs' && selector.argumentCount == 0) {

pkg/compiler/lib/src/js_emitter/startup_emitter/fragment_emitter.dart

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -171,11 +171,7 @@ function lazyFinal(holder, name, getterName, initializer) {
171171
//
172172
// The runtime ensures that const-lists cannot be modified.
173173
function makeConstList(list) {
174-
// By assigning a function to the properties they become part of the
175-
// hidden class. The actual values of the fields don't matter, since we
176-
// only check if they exist.
177-
list.immutable\$list = Array;
178-
list.fixed\$length = Array;
174+
list.#arrayFlagsProperty = ${ArrayFlags.constant};
179175
return list;
180176
}
181177
@@ -675,6 +671,7 @@ class FragmentEmitter {
675671
'directAccessTestExpression': js.js(_directAccessTestExpression),
676672
'throwLateFieldADI': _emitter
677673
.staticFunctionAccess(_closedWorld.commonElements.throwLateFieldADI),
674+
'arrayFlagsProperty': js.string(_namer.fixedNames.arrayFlagsPropertyName),
678675
'operatorIsPrefix': js.string(_namer.fixedNames.operatorIsPrefix),
679676
'tearOffCode': js.Block(
680677
buildTearOffCode(_options, _emitter, _closedWorld.commonElements)),

pkg/compiler/lib/src/js_emitter/startup_emitter/model_emitter.dart

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ library dart2js.js_emitter.startup_emitter.model_emitter;
66

77
import 'dart:convert' show JsonEncoder;
88

9+
import 'package:js_runtime/synced/array_flags.dart' show ArrayFlags;
10+
911
import 'package:js_runtime/synced/embedded_names.dart'
1012
show
1113
DEFERRED_INITIALIZED,
@@ -35,6 +37,7 @@ import 'package:js_shared/synced/embedded_names.dart'
3537
RTI_UNIVERSE,
3638
RtiUniverseFieldNames,
3739
TYPES;
40+
3841
import 'package:js_shared/variance.dart';
3942

4043
import 'package:js_ast/src/precedence.dart' as js_precedence;

pkg/compiler/lib/src/kernel/dart2js_target.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,7 @@ const implicitlyUsedLibraries = <String>[
255255
// compile-platform should just specify which libraries to compile instead.
256256
const requiredLibraries = <String, List<String>>{
257257
'dart2js': [
258+
'dart:_array_flags',
258259
'dart:_async_status_codes',
259260
'dart:_dart2js_only',
260261
'dart:_dart2js_runtime_metrics',
@@ -297,6 +298,7 @@ const requiredLibraries = <String, List<String>>{
297298
'dart:web_gl',
298299
],
299300
'dart2js_server': [
301+
'dart:_array_flags',
300302
'dart:_async_status_codes',
301303
'dart:_dart2js_only',
302304
'dart:_dart2js_runtime_metrics',

pkg/compiler/lib/src/ssa/builder.dart

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4631,6 +4631,12 @@ class KernelSsaGraphBuilder extends ir.VisitorDefault<void>
46314631
_handleLateInitializeOnceCheck(invocation, sourceInformation);
46324632
} else if (name == 'HCharCodeAt') {
46334633
_handleCharCodeAt(invocation, sourceInformation);
4634+
} else if (name == 'HArrayFlagsGet') {
4635+
_handleArrayFlagsGet(invocation, sourceInformation);
4636+
} else if (name == 'HArrayFlagsSet') {
4637+
_handleArrayFlagsSet(invocation, sourceInformation);
4638+
} else if (name == 'HArrayFlagsCheck') {
4639+
_handleArrayFlagsCheck(invocation, sourceInformation);
46344640
} else {
46354641
reporter.internalError(
46364642
_elementMap.getSpannable(targetElement, invocation),
@@ -5372,6 +5378,73 @@ class KernelSsaGraphBuilder extends ir.VisitorDefault<void>
53725378
..sourceInformation = sourceInformation);
53735379
}
53745380

5381+
void _handleArrayFlagsGet(
5382+
ir.StaticInvocation invocation, SourceInformation? sourceInformation) {
5383+
if (_unexpectedForeignArguments(invocation,
5384+
minPositional: 1, maxPositional: 1)) {
5385+
// Result expected on stack.
5386+
stack.add(graph.addConstantNull(closedWorld));
5387+
return;
5388+
}
5389+
List<HInstruction> inputs = _visitPositionalArguments(invocation.arguments);
5390+
final array = inputs.single;
5391+
5392+
push(HArrayFlagsGet(array, _abstractValueDomain.uint31Type)
5393+
..sourceInformation = sourceInformation);
5394+
}
5395+
5396+
void _handleArrayFlagsSet(
5397+
ir.StaticInvocation invocation, SourceInformation? sourceInformation) {
5398+
if (_unexpectedForeignArguments(invocation,
5399+
minPositional: 2, maxPositional: 2, typeArgumentCount: 1)) {
5400+
// Result expected on stack.
5401+
stack.add(graph.addConstantNull(closedWorld));
5402+
return;
5403+
}
5404+
5405+
List<HInstruction> inputs = _visitPositionalArguments(invocation.arguments);
5406+
final array = inputs[0];
5407+
final flags = inputs[1];
5408+
5409+
// TODO(sra): Use the flags to improve in the AbstractValue, which may
5410+
// contain powerset domain bits outside of the conventional type
5411+
// system. Perhaps do this in types_propagation.
5412+
DartType type = _getDartTypeIfValid(invocation.arguments.types.single);
5413+
AbstractValue? instructionType = _typeBuilder.trustTypeMask(type);
5414+
// TODO(sra): Better type
5415+
instructionType ??= _abstractValueDomain.dynamicType;
5416+
5417+
push(HArrayFlagsSet(array, flags, instructionType)
5418+
..sourceInformation = sourceInformation);
5419+
}
5420+
5421+
void _handleArrayFlagsCheck(
5422+
ir.StaticInvocation invocation, SourceInformation? sourceInformation) {
5423+
if (_unexpectedForeignArguments(invocation,
5424+
minPositional: 4, maxPositional: 4, typeArgumentCount: 1)) {
5425+
// Result expected on stack.
5426+
stack.add(graph.addConstantNull(closedWorld));
5427+
return;
5428+
}
5429+
List<HInstruction> inputs = _visitPositionalArguments(invocation.arguments);
5430+
final array = inputs[0];
5431+
final arrayFlags = inputs[1];
5432+
final checkFlags = inputs[2];
5433+
final operation = inputs[3];
5434+
5435+
// TODO(sra): Use the flags to improve in the AbstractValue, which may
5436+
// contain powerset domain bits outside of the conventional type
5437+
// system. Perhaps do this in types_propagation.
5438+
DartType type = _getDartTypeIfValid(invocation.arguments.types.single);
5439+
AbstractValue? instructionType = _typeBuilder.trustTypeMask(type);
5440+
// TODO(sra): Better type
5441+
instructionType ??= _abstractValueDomain.dynamicType;
5442+
5443+
push(HArrayFlagsCheck(
5444+
array, arrayFlags, checkFlags, operation, instructionType)
5445+
..sourceInformation = sourceInformation);
5446+
}
5447+
53755448
void _handleForeignTypeRef(
53765449
ir.StaticInvocation invocation, SourceInformation? sourceInformation) {
53775450
if (_unexpectedForeignArguments(invocation,

pkg/compiler/lib/src/ssa/codegen.dart

Lines changed: 76 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -682,14 +682,15 @@ class SsaCodeGenerator implements HVisitor<void>, HBlockInformationVisitor {
682682
// emit an assignment, because the intTypeCheck just returns its
683683
// argument.
684684
bool needsAssignment = true;
685-
if (instruction is HCheck) {
685+
if (instruction is HOutputConstrainedToAnInput) {
686686
if (instruction is HPrimitiveCheck ||
687687
instruction is HAsCheck ||
688688
instruction is HAsCheckSimple ||
689689
instruction is HBoolConversion ||
690690
instruction is HNullCheck ||
691-
instruction is HLateReadCheck) {
692-
String? inputName = variableNames.getName(instruction.checkedInput);
691+
instruction is HLateReadCheck ||
692+
instruction is HArrayFlagsSet) {
693+
String? inputName = variableNames.getName(instruction.constrainedInput);
693694
if (variableNames.getName(instruction) == inputName) {
694695
needsAssignment = false;
695696
}
@@ -715,9 +716,9 @@ class SsaCodeGenerator implements HVisitor<void>, HBlockInformationVisitor {
715716
}
716717
}
717718

718-
HInstruction skipGenerateAtUseCheckInputs(HCheck check) {
719-
HInstruction input = check.checkedInput;
720-
if (input is HCheck && isGenerateAtUseSite(input)) {
719+
HInstruction skipGenerateAtUseCheckInputs(HOutputConstrainedToAnInput check) {
720+
HInstruction input = check.constrainedInput;
721+
if (input is HOutputConstrainedToAnInput && isGenerateAtUseSite(input)) {
721722
return skipGenerateAtUseCheckInputs(input);
722723
}
723724
return input;
@@ -726,7 +727,8 @@ class SsaCodeGenerator implements HVisitor<void>, HBlockInformationVisitor {
726727
void use(HInstruction argument) {
727728
if (isGenerateAtUseSite(argument)) {
728729
visitExpression(argument);
729-
} else if (argument is HCheck && !variableNames.hasName(argument)) {
730+
} else if (argument is HOutputConstrainedToAnInput &&
731+
!variableNames.hasName(argument)) {
730732
// We have a check that is not generate-at-use and has no name, yet is a
731733
// subexpression (we are in 'use'). This happens when we have a chain of
732734
// checks on an available unnamed value (e.g. a constant). The checks are
@@ -737,11 +739,10 @@ class SsaCodeGenerator implements HVisitor<void>, HBlockInformationVisitor {
737739
// instruction has a name or is generate-at-use". This would require
738740
// naming the input or output of the chain-of-checks.
739741

740-
HCheck check = argument;
741742
// This can only happen if the checked node also does not have a name.
742-
assert(!variableNames.hasName(check.checkedInput));
743+
assert(!variableNames.hasName(argument.constrainedInput));
743744

744-
use(skipGenerateAtUseCheckInputs(check));
745+
use(skipGenerateAtUseCheckInputs(argument));
745746
} else {
746747
assert(variableNames.hasName(argument));
747748
push(js.VariableUse(variableNames.getName(argument)!));
@@ -3441,4 +3442,69 @@ class SsaCodeGenerator implements HVisitor<void>, HBlockInformationVisitor {
34413442
_metrics.countHIsLateSentinel++;
34423443
_emitIsLateSentinel(node.inputs.single, node.sourceInformation);
34433444
}
3445+
3446+
@override
3447+
void visitArrayFlagsGet(HArrayFlagsGet node) {
3448+
use(node.inputs.single);
3449+
js.Expression array = pop();
3450+
js.Expression flags =
3451+
js.js(r'#.#', [array, _namer.fixedNames.arrayFlagsPropertyName]);
3452+
if (isGenerateAtUseSite(node) && node.usedBy.single is HArrayFlagsCheck) {
3453+
// The enclosing expression will be an immediate `& mask`.
3454+
push(flags);
3455+
} else {
3456+
// The flags are reused, possibly hoisted, so force an `undefined` to be a
3457+
// small integer once rather than at each check.
3458+
push(js.js(r'# | 0', flags));
3459+
}
3460+
}
3461+
3462+
@override
3463+
void visitArrayFlagsSet(HArrayFlagsSet node) {
3464+
use(node.inputs[0]);
3465+
js.Expression array = pop();
3466+
use(node.inputs[1]);
3467+
js.Expression arrayFlags = pop();
3468+
pushStatement(js.js.statement(r'#.# = #;', [
3469+
array,
3470+
_namer.fixedNames.arrayFlagsPropertyName,
3471+
arrayFlags
3472+
]).withSourceInformation(node.sourceInformation));
3473+
}
3474+
3475+
@override
3476+
void visitArrayFlagsCheck(HArrayFlagsCheck node) {
3477+
use(node.array);
3478+
js.Expression array = pop();
3479+
3480+
js.Expression? test;
3481+
if (!node.alwaysThrows()) {
3482+
use(node.arrayFlags);
3483+
js.Expression arrayFlags = pop();
3484+
use(node.checkFlags);
3485+
js.Expression checkFlags = pop();
3486+
test = js.js('# & #', [arrayFlags, checkFlags]);
3487+
}
3488+
3489+
final operation = node.operation;
3490+
// Most common operation is "[]=", so 'pass' that by leaving it out.
3491+
if (operation
3492+
case HConstant(constant: StringConstantValue(stringValue: '[]='))) {
3493+
_pushCallStatic(_commonElements.throwUnsupportedOperation, [array],
3494+
node.sourceInformation);
3495+
} else {
3496+
use(operation);
3497+
_pushCallStatic(_commonElements.throwUnsupportedOperation, [array, pop()],
3498+
node.sourceInformation);
3499+
}
3500+
3501+
js.Statement check;
3502+
if (test == null) {
3503+
check = js.js.statement('#;', pop());
3504+
} else {
3505+
check = js.js.statement('# && #;', [test, pop()]);
3506+
}
3507+
3508+
pushStatement(check.withSourceInformation(node.sourceInformation));
3509+
}
34443510
}

pkg/compiler/lib/src/ssa/codegen_helpers.dart

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1041,6 +1041,20 @@ class SsaInstructionMerger extends HBaseVisitor<void> implements CodegenPhase {
10411041
}
10421042
}
10431043

1044+
@override
1045+
void visitArrayFlagsSet(HArrayFlagsSet instruction) {
1046+
// Cannot generate-at-use the array input, it is an alias for the value of
1047+
// this instruction and need to be allocated to a variable.
1048+
analyzeInputs(instruction, 1);
1049+
}
1050+
1051+
@override
1052+
void visitArrayFlagsCheck(HArrayFlagsCheck instruction) {
1053+
// Cannot generate-at-use the array input, it is an alias for the value of
1054+
// this instruction and need to be allocated to a variable.
1055+
analyzeInputs(instruction, 1);
1056+
}
1057+
10441058
void tryGenerateAtUseSite(HInstruction instruction) {
10451059
if (instruction.isControlFlow()) return;
10461060
markAsGenerateAtUseSite(instruction);

pkg/compiler/lib/src/ssa/invoke_dynamic_specializers.dart

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// for details. All rights reserved. Use of this source code is governed by a
33
// BSD-style license that can be found in the LICENSE file.
44

5+
import 'package:js_runtime/synced/array_flags.dart' show ArrayFlags;
56
import '../common/elements.dart' show JCommonElements;
67
import '../constants/constant_system.dart' as constant_system;
78
import '../constants/values.dart';
@@ -203,13 +204,23 @@ class IndexAssignSpecializer extends InvokeDynamicSpecializer {
203204
OptimizationTestLog? log) {
204205
HInstruction receiver = instruction.inputs[1];
205206
HInstruction index = instruction.inputs[2];
206-
if (receiver
207-
.isMutableIndexable(closedWorld.abstractValueDomain)
208-
.isPotentiallyFalse) {
209-
return null;
207+
final abstractValueDomain = closedWorld.abstractValueDomain;
208+
209+
bool needsMutableCheck = false;
210+
if (abstractValueDomain
211+
.isTypedArray(receiver.instructionType)
212+
.isDefinitelyTrue) {
213+
needsMutableCheck = true;
214+
} else if (receiver.isArray(abstractValueDomain).isDefinitelyTrue) {
215+
needsMutableCheck =
216+
receiver.isMutableArray(abstractValueDomain).isPotentiallyFalse;
217+
} else {
218+
if (receiver.isMutableIndexable(abstractValueDomain).isPotentiallyFalse) {
219+
return null;
220+
}
210221
}
211222
// TODO(johnniwinther): Merge this and the following if statement.
212-
if (index.isInteger(closedWorld.abstractValueDomain).isPotentiallyFalse &&
223+
if (index.isInteger(abstractValueDomain).isPotentiallyFalse &&
213224
// TODO(johnniwinther): Support annotations on the possible targets
214225
// and used their parameter check policy here.
215226
closedWorld.annotationsData.getParameterCheckPolicy(null).isEmitted) {
@@ -227,6 +238,24 @@ class IndexAssignSpecializer extends InvokeDynamicSpecializer {
227238
}
228239
}
229240

241+
if (needsMutableCheck) {
242+
HInstruction getFlags =
243+
HArrayFlagsGet(receiver, abstractValueDomain.uint31Type)
244+
..sourceInformation = instruction.sourceInformation;
245+
instruction.block!.addBefore(instruction, getFlags);
246+
HInstruction mask =
247+
graph.addConstantInt(ArrayFlags.unmodifiableCheck, closedWorld);
248+
HInstruction name = graph.addConstantString('[]=', closedWorld);
249+
final instructionType = receiver.instructionType;
250+
final checkFlags =
251+
HArrayFlagsCheck(receiver, getFlags, mask, name, instructionType)
252+
..sourceInformation = instruction.sourceInformation;
253+
instruction.block!.addBefore(instruction, checkFlags);
254+
checkFlags.instructionType = checkFlags.computeInstructionType(
255+
instructionType, abstractValueDomain);
256+
receiver = checkFlags;
257+
}
258+
230259
HInstruction checkedIndex = index;
231260
if (requiresBoundsCheck(instruction, closedWorld)) {
232261
checkedIndex =

0 commit comments

Comments
 (0)