Skip to content

Commit aa8f15f

Browse files
natebiggsCommit Queue
authored andcommitted
[dart2wasm] Reland br_table change.
Dedupes all the logic that was being copied between the int and enum case by adding helpers into the BrTableInfo class. Reverts: https://dart-review.googlesource.com/c/sdk/+/443082?tab=comments Fixes: #61223 Change-Id: I861d4b878059d9c633789ce1a09e0e69a2ad925f Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/443220 Commit-Queue: Nate Biggs <[email protected]> Reviewed-by: Ömer Ağacan <[email protected]> Reviewed-by: Martin Kustermann <[email protected]>
1 parent e21caf8 commit aa8f15f

File tree

2 files changed

+183
-20
lines changed

2 files changed

+183
-20
lines changed

pkg/dart2wasm/lib/code_generator.dart

Lines changed: 181 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1424,29 +1424,45 @@ abstract class AstCodeGenerator
14241424
b.end();
14251425
}
14261426

1427-
// Compare against all case values
1428-
for (SwitchCase c in node.cases) {
1429-
for (Expression exp in c.expressions) {
1430-
if (exp is NullLiteral ||
1431-
exp is ConstantExpression && exp.constant is NullConstant) {
1432-
// Null already checked, skip
1433-
} else {
1434-
switchInfo.compare(
1435-
switchValueNonNullableLocal,
1436-
() => translateExpression(exp, switchInfo.nonNullableType),
1437-
);
1438-
b.br_if(switchLabels[c]!);
1439-
}
1427+
final brTable = switchInfo.brTable;
1428+
if (brTable != null) {
1429+
// Map each entry in the range to the appropriate jump table entry.
1430+
final indexBlocks = <w.Label>[];
1431+
final defaultLabel =
1432+
defaultCase != null ? switchLabels[defaultCase]! : doneLabel;
1433+
for (int i = brTable.minValue; i <= brTable.maxValue; ++i) {
1434+
final c = brTable.caseMap[i];
1435+
indexBlocks.add(c == null ? defaultLabel : switchLabels[c]!);
14401436
}
1441-
}
14421437

1443-
// No explicit cases matched
1444-
if (node.isExplicitlyExhaustive) {
1445-
b.unreachable();
1438+
brTable.emitBrTableExpr(b, switchValueNonNullableLocal);
1439+
1440+
b.br_table(indexBlocks, defaultLabel);
14461441
} else {
1447-
w.Label defaultLabel =
1448-
defaultCase != null ? switchLabels[defaultCase]! : doneLabel;
1449-
b.br(defaultLabel);
1442+
// Compare against all case values
1443+
for (SwitchCase c in node.cases) {
1444+
for (Expression exp in c.expressions) {
1445+
if (exp is NullLiteral ||
1446+
exp is ConstantExpression && exp.constant is NullConstant) {
1447+
// Null already checked, skip
1448+
} else {
1449+
switchInfo.compare(
1450+
switchValueNonNullableLocal,
1451+
() => translateExpression(exp, switchInfo.nonNullableType),
1452+
);
1453+
b.br_if(switchLabels[c]!);
1454+
}
1455+
}
1456+
}
1457+
1458+
// No explicit cases matched
1459+
if (node.isExplicitlyExhaustive) {
1460+
b.unreachable();
1461+
} else {
1462+
w.Label defaultLabel =
1463+
defaultCase != null ? switchLabels[defaultCase]! : doneLabel;
1464+
b.br(defaultLabel);
1465+
}
14501466
}
14511467

14521468
// Emit case bodies
@@ -4297,6 +4313,63 @@ class SwitchBackwardJumpInfo {
42974313
: defaultLoopLabel = null;
42984314
}
42994315

4316+
/// Info needed to represent a switch statement using a br_table instruction.
4317+
///
4318+
/// This is used for switches on integers and enums (using their indicies). We
4319+
/// map each option to an index in the jump table and then jump directly to the
4320+
/// target case. This is much faster than iteratively comparing each cases's
4321+
/// expression to the switch expression.
4322+
///
4323+
/// Sometimes switches over ranges of ints are sparse enough that a table would
4324+
/// bloat the code compared to the iterative comparison.
4325+
class BrTableInfo {
4326+
// At least 50% of the [min, max] must be occupied for us to use `br_table`.
4327+
static const double _minimumTableOccupancy = 0.5;
4328+
// This is the maximum size where it's always worth it to use a br_table.
4329+
// If the br_table is bigger than this then we start to check sparseness.
4330+
// Below this point we always accept the potential code size hit.
4331+
static const int _maxSparseSize = 50;
4332+
4333+
int get rangeSize => _rangeSize(minValue, maxValue);
4334+
final int minValue;
4335+
final int maxValue;
4336+
final void Function(w.Local switchExprLocal) _brTableExpr;
4337+
final Map<int, SwitchCase> caseMap;
4338+
4339+
BrTableInfo._(this.minValue, this.maxValue, this.caseMap, this._brTableExpr)
4340+
: assert(!_isTooSparse(minValue, maxValue, caseMap));
4341+
4342+
/// Heuristically validate whether the provided table would be too sparse and
4343+
/// if so return null. Otherwise return the expected table.
4344+
static BrTableInfo? build(Map<int, SwitchCase> caseMap,
4345+
void Function(w.Local switchExprLocal) brTableExpr,
4346+
{required int minValue, required int maxValue}) {
4347+
// Validate the table density and size is worth putting into a br_table.
4348+
if (_isTooSparse(minValue, maxValue, caseMap)) return null;
4349+
4350+
return BrTableInfo._(minValue, maxValue, caseMap, brTableExpr);
4351+
}
4352+
4353+
static bool _isTooSparse(int min, int max, Map<int, SwitchCase> caseMap) {
4354+
int rangeSize = _rangeSize(min, max);
4355+
return (caseMap.length / rangeSize) < _minimumTableOccupancy &&
4356+
rangeSize > _maxSparseSize;
4357+
}
4358+
4359+
static int _rangeSize(int min, int max) => max - min + 1;
4360+
4361+
void emitBrTableExpr(w.InstructionsBuilder b, w.Local switchExprLocal) {
4362+
_brTableExpr(switchExprLocal);
4363+
// Normalize on 0.
4364+
if (minValue != 0) {
4365+
b.i64_const(minValue);
4366+
b.i64_sub();
4367+
}
4368+
// Now that we've normalized on 0 it should be safe to switch to i32.
4369+
b.i32_wrap_i64();
4370+
}
4371+
}
4372+
43004373
class SwitchInfo {
43014374
/// Non-nullable Wasm type of the `switch` expression. Used when the
43024375
/// expression is not nullable, and after the null check.
@@ -4323,6 +4396,11 @@ class SwitchInfo {
43234396
/// The `null: ...` case, if exists.
43244397
late final SwitchCase? nullCase;
43254398

4399+
/// Info needed to compile this switch statement into a wasm br_table. If null
4400+
/// this switch statement should not use a br_table and should use comparison
4401+
/// based case matching instead.
4402+
BrTableInfo? brTable;
4403+
43264404
SwitchInfo(AstCodeGenerator codeGen, SwitchStatement node) {
43274405
final translator = codeGen.translator;
43284406

@@ -4453,6 +4531,31 @@ class SwitchInfo {
44534531
nonNullableType = w.NumType.i64;
44544532
nullableType =
44554533
translator.classInfo[translator.boxedIntClass]!.nullableType;
4534+
4535+
// Calculate the range covered by the cases and create the jump table.
4536+
int? minValue;
4537+
int? maxValue;
4538+
Map<int, SwitchCase> caseMap = {};
4539+
for (final c in node.cases) {
4540+
for (final e in c.expressions) {
4541+
final value = e is IntLiteral
4542+
? e.value
4543+
: ((e as ConstantExpression).constant as IntConstant).value;
4544+
caseMap[value] = c;
4545+
if (minValue == null || value < minValue) minValue = value;
4546+
if (maxValue == null || value > maxValue) maxValue = value;
4547+
}
4548+
}
4549+
if (maxValue != null) {
4550+
brTable = BrTableInfo.build(
4551+
minValue: minValue!,
4552+
maxValue: maxValue,
4553+
caseMap, (switchExprLocal) {
4554+
codeGen.b.local_get(switchExprLocal);
4555+
});
4556+
}
4557+
4558+
// Provide a compare as a fallback in case the range is too sparse.
44564559
compare = (switchExprLocal, pushCaseExpr) {
44574560
codeGen.b.local_get(switchExprLocal);
44584561
pushCaseExpr();
@@ -4467,6 +4570,64 @@ class SwitchInfo {
44674570
pushCaseExpr();
44684571
codeGen.call(translator.jsStringEquals.reference);
44694572
};
4573+
} else if (switchExprClass.isEnum) {
4574+
// If this is an applicable switch over enums, create a jump table.
4575+
bool isValid = true;
4576+
var caseMap = <int, SwitchCase>{};
4577+
int? minIndex;
4578+
int? maxIndex;
4579+
outer:
4580+
for (final c in node.cases) {
4581+
for (final e in c.expressions) {
4582+
if (e is! ConstantExpression) {
4583+
isValid = false;
4584+
break outer;
4585+
}
4586+
final constant = e.constant;
4587+
if (constant is! InstanceConstant) {
4588+
isValid = false;
4589+
break outer;
4590+
}
4591+
if (constant.classNode != switchExprClass) {
4592+
isValid = false;
4593+
break outer;
4594+
}
4595+
final enumIndex =
4596+
(constant.fieldValues[translator.enumIndexField.fieldReference]
4597+
as IntConstant)
4598+
.value;
4599+
caseMap[enumIndex] = c;
4600+
if (maxIndex == null || enumIndex > maxIndex) maxIndex = enumIndex;
4601+
if (minIndex == null || enumIndex < minIndex) minIndex = enumIndex;
4602+
}
4603+
}
4604+
4605+
if (isValid && maxIndex != null) {
4606+
brTable = BrTableInfo.build(
4607+
minValue: minIndex!,
4608+
maxValue: maxIndex,
4609+
caseMap, (switchExprLocal) {
4610+
codeGen.b.local_get(switchExprLocal);
4611+
codeGen.call(translator.enumIndexField.getterReference);
4612+
});
4613+
}
4614+
4615+
if (brTable == null) {
4616+
// Object identity switch
4617+
nonNullableType = translator.topTypeNonNullable;
4618+
nullableType = translator.topType;
4619+
} else {
4620+
nonNullableType =
4621+
translator.classInfo[switchExprClass]!.nonNullableType;
4622+
nullableType = translator.classInfo[switchExprClass]!.nullableType;
4623+
}
4624+
4625+
// Set compare anyway for state machine handling
4626+
compare = (switchExprLocal, pushCaseExpr) {
4627+
codeGen.b.local_get(switchExprLocal);
4628+
pushCaseExpr();
4629+
codeGen.call(translator.coreTypes.identicalProcedure.reference);
4630+
};
44704631
} else {
44714632
// Object identity switch
44724633
nonNullableType = translator.topTypeNonNullable;

pkg/dart2wasm/lib/kernel_nodes.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@ mixin KernelNodes {
5757
late final Class typeErrorClass = index.getClass("dart:core", "_TypeError");
5858
late final Class javaScriptErrorClass =
5959
index.getClass("dart:core", "_JavaScriptError");
60+
late final Field enumIndexField =
61+
index.getField('dart:core', '_Enum', 'index');
6062

6163
// dart:core runtime type classes
6264
late final Class typeClass = index.getClass("dart:core", "_Type");

0 commit comments

Comments
 (0)