Skip to content

Commit 0bb686f

Browse files
tlivelytlively
authored
More ArrayCmpxchg expected opts in Heap2Local (#8567)
When non-escaping allocations flow into the `expected` field of an ArrayCmpxchg, optimize even if the ArrayCmpxchg has a non-constant index. We typically only optimize array instructions with constant fields, but that's because we need to know what field of the array is accessed. Arrays flowing into the `expected` field are not accessed, though, so there is no need for the accessed index to be constant. Make sure that effects in the potentially non-constant index field are preserved in the correct order by using a new scratch local to propagate the index value past the `expected` and `replacement` expressions to the newly generated `struct.atomic.get`.
1 parent 13cb918 commit 0bb686f

File tree

2 files changed

+136
-34
lines changed

2 files changed

+136
-34
lines changed

‎src/passes/Heap2Local.cpp‎

Lines changed: 30 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -461,40 +461,31 @@ struct EscapeAnalyzer {
461461
}
462462
}
463463
void visitArraySet(ArraySet* curr) {
464-
if (!curr->index->is<Const>()) {
465-
// Array operations on nonconstant indexes do not escape in the normal
466-
// sense, but they do escape from our being able to analyze them, so
467-
// stop as soon as we see one.
468-
return;
469-
}
470-
471-
// As StructGet.
472-
if (curr->ref == child) {
464+
// Arrays flowing into array operations on nonconstant indexes do not
465+
// escape in the normal sense, but they do escape from our being able to
466+
// analyze them, so stop as soon as we see one.
467+
if (child == curr->ref && curr->index->is<Const>()) {
473468
escapes = false;
474469
fullyConsumes = true;
475470
}
476471
}
477472
void visitArrayGet(ArrayGet* curr) {
478-
if (!curr->index->is<Const>()) {
479-
return;
473+
if (child == curr->ref && curr->index->is<Const>()) {
474+
escapes = false;
475+
fullyConsumes = true;
480476
}
481-
escapes = false;
482-
fullyConsumes = true;
483477
}
484478
void visitArrayRMW(ArrayRMW* curr) {
485-
if (!curr->index->is<Const>()) {
486-
return;
487-
}
488-
if (curr->ref == child) {
479+
if (child == curr->ref && curr->index->is<Const>()) {
489480
escapes = false;
490481
fullyConsumes = true;
491482
}
492483
}
493484
void visitArrayCmpxchg(ArrayCmpxchg* curr) {
494-
if (!curr->index->is<Const>()) {
495-
return;
496-
}
497-
if (curr->ref == child || curr->expected == child) {
485+
// Allocations flowing into `expected` are fully consumed and
486+
// optimizable even if the index is not constant.
487+
if (child == curr->expected ||
488+
(child == curr->ref && curr->index->is<Const>())) {
498489
escapes = false;
499490
fullyConsumes = true;
500491
}
@@ -1233,9 +1224,15 @@ struct Struct2Local : PostWalker<Struct2Local> {
12331224
auto refScratch = builder.addVar(func, refType);
12341225
auto* setRefScratch = builder.makeLocalSet(refScratch, curr->ref);
12351226
auto* getRefScratch = builder.makeLocalGet(refScratch, refType);
1227+
1228+
auto indexScratch = builder.addVar(func, Type::i32);
1229+
auto* setIndexScratch = builder.makeLocalSet(indexScratch, curr->index);
1230+
auto* getIndexScratch = builder.makeLocalGet(indexScratch, Type::i32);
1231+
12361232
auto* arrayGet = builder.makeArrayGet(
1237-
getRefScratch, curr->index, curr->order, curr->type);
1233+
getRefScratch, getIndexScratch, curr->order, curr->type);
12381234
auto* block = builder.makeBlock({setRefScratch,
1235+
setIndexScratch,
12391236
builder.makeDrop(curr->expected),
12401237
builder.makeDrop(curr->replacement),
12411238
arrayGet});
@@ -1467,20 +1464,20 @@ struct Array2Struct : PostWalker<Array2Struct> {
14671464
return;
14681465
}
14691466

1470-
auto index = getIndex(curr->index);
1471-
if (index >= numFields) {
1472-
replaceCurrent(builder.makeBlock({builder.makeDrop(curr->ref),
1473-
builder.makeDrop(curr->expected),
1474-
builder.makeDrop(curr->replacement),
1475-
builder.makeUnreachable()}));
1476-
refinalize = true;
1477-
return;
1478-
}
1479-
14801467
// The allocation might flow into `ref` or `expected`, but not
14811468
// `replacement`, because then it would be considered to have escaped.
14821469
if (analyzer.getInteraction(curr->ref) == ParentChildInteraction::Flows) {
1483-
// The accessed array is being optimzied. Convert the ArrayCmpxchg into a
1470+
auto index = getIndex(curr->index);
1471+
if (index >= numFields) {
1472+
replaceCurrent(builder.makeBlock({builder.makeDrop(curr->ref),
1473+
builder.makeDrop(curr->expected),
1474+
builder.makeDrop(curr->replacement),
1475+
builder.makeUnreachable()}));
1476+
refinalize = true;
1477+
return;
1478+
}
1479+
1480+
// The accessed array is being optimized. Convert the ArrayCmpxchg into a
14841481
// StructCmpxchg.
14851482
replaceCurrent(builder.makeStructCmpxchg(
14861483
index, curr->ref, curr->expected, curr->replacement, curr->order));

‎test/lit/passes/heap2local-rmw.wast‎

Lines changed: 106 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1465,11 +1465,15 @@
14651465
;; CHECK: (func $array-cmpxchg-expected (type $1) (param $array (ref $array))
14661466
;; CHECK-NEXT: (local $1 eqref)
14671467
;; CHECK-NEXT: (local $2 (ref null $array))
1468+
;; CHECK-NEXT: (local $3 i32)
14681469
;; CHECK-NEXT: (drop
14691470
;; CHECK-NEXT: (block (result eqref)
14701471
;; CHECK-NEXT: (local.set $2
14711472
;; CHECK-NEXT: (local.get $array)
14721473
;; CHECK-NEXT: )
1474+
;; CHECK-NEXT: (local.set $3
1475+
;; CHECK-NEXT: (i32.const 0)
1476+
;; CHECK-NEXT: )
14731477
;; CHECK-NEXT: (drop
14741478
;; CHECK-NEXT: (block (result nullref)
14751479
;; CHECK-NEXT: (local.set $1
@@ -1483,7 +1487,7 @@
14831487
;; CHECK-NEXT: )
14841488
;; CHECK-NEXT: (array.atomic.get $array
14851489
;; CHECK-NEXT: (local.get $2)
1486-
;; CHECK-NEXT: (i32.const 0)
1490+
;; CHECK-NEXT: (local.get $3)
14871491
;; CHECK-NEXT: )
14881492
;; CHECK-NEXT: )
14891493
;; CHECK-NEXT: )
@@ -1504,3 +1508,104 @@
15041508
)
15051509
)
15061510
)
1511+
1512+
(module
1513+
;; CHECK: (type $array (array (mut eqref)))
1514+
(type $array (array (mut eqref)))
1515+
;; CHECK: (type $1 (func (param (ref $array))))
1516+
1517+
;; CHECK: (type $2 (func (result i32)))
1518+
1519+
;; CHECK: (type $3 (func (result eqref)))
1520+
1521+
;; CHECK: (import "" "" (func $effect-i32 (type $2) (result i32)))
1522+
(import "" "" (func $effect-i32 (result i32)))
1523+
;; CHECK: (import "" "" (func $effect-eq (type $3) (result eqref)))
1524+
(import "" "" (func $effect-eq (result eqref)))
1525+
1526+
;; CHECK: (func $array-cmpxchg-expected-index-effect (type $1) (param $array (ref $array))
1527+
;; CHECK-NEXT: (local $1 eqref)
1528+
;; CHECK-NEXT: (local $2 (ref null $array))
1529+
;; CHECK-NEXT: (local $3 i32)
1530+
;; CHECK-NEXT: (drop
1531+
;; CHECK-NEXT: (block (result eqref)
1532+
;; CHECK-NEXT: (local.set $2
1533+
;; CHECK-NEXT: (local.get $array)
1534+
;; CHECK-NEXT: )
1535+
;; CHECK-NEXT: (local.set $3
1536+
;; CHECK-NEXT: (call $effect-i32)
1537+
;; CHECK-NEXT: )
1538+
;; CHECK-NEXT: (drop
1539+
;; CHECK-NEXT: (block (result nullref)
1540+
;; CHECK-NEXT: (local.set $1
1541+
;; CHECK-NEXT: (ref.null none)
1542+
;; CHECK-NEXT: )
1543+
;; CHECK-NEXT: (ref.null none)
1544+
;; CHECK-NEXT: )
1545+
;; CHECK-NEXT: )
1546+
;; CHECK-NEXT: (drop
1547+
;; CHECK-NEXT: (call $effect-eq)
1548+
;; CHECK-NEXT: )
1549+
;; CHECK-NEXT: (array.atomic.get $array
1550+
;; CHECK-NEXT: (local.get $2)
1551+
;; CHECK-NEXT: (local.get $3)
1552+
;; CHECK-NEXT: )
1553+
;; CHECK-NEXT: )
1554+
;; CHECK-NEXT: )
1555+
;; CHECK-NEXT: )
1556+
(func $array-cmpxchg-expected-index-effect (param $array (ref $array))
1557+
(drop
1558+
;; The index is non-constant, but we can still optimize the expected
1559+
;; field. We must preserve the index and the order of its effects.
1560+
(array.atomic.rmw.cmpxchg $array
1561+
(local.get $array)
1562+
(call $effect-i32)
1563+
(array.new_default $array (i32.const 1))
1564+
(call $effect-eq)
1565+
)
1566+
)
1567+
)
1568+
1569+
;; CHECK: (func $array-cmpxchg-expected-index-oob (type $1) (param $array (ref $array))
1570+
;; CHECK-NEXT: (local $1 eqref)
1571+
;; CHECK-NEXT: (local $2 (ref null $array))
1572+
;; CHECK-NEXT: (local $3 i32)
1573+
;; CHECK-NEXT: (drop
1574+
;; CHECK-NEXT: (block (result eqref)
1575+
;; CHECK-NEXT: (local.set $2
1576+
;; CHECK-NEXT: (local.get $array)
1577+
;; CHECK-NEXT: )
1578+
;; CHECK-NEXT: (local.set $3
1579+
;; CHECK-NEXT: (i32.const -1)
1580+
;; CHECK-NEXT: )
1581+
;; CHECK-NEXT: (drop
1582+
;; CHECK-NEXT: (block (result nullref)
1583+
;; CHECK-NEXT: (local.set $1
1584+
;; CHECK-NEXT: (ref.null none)
1585+
;; CHECK-NEXT: )
1586+
;; CHECK-NEXT: (ref.null none)
1587+
;; CHECK-NEXT: )
1588+
;; CHECK-NEXT: )
1589+
;; CHECK-NEXT: (drop
1590+
;; CHECK-NEXT: (call $effect-eq)
1591+
;; CHECK-NEXT: )
1592+
;; CHECK-NEXT: (array.atomic.get $array
1593+
;; CHECK-NEXT: (local.get $2)
1594+
;; CHECK-NEXT: (local.get $3)
1595+
;; CHECK-NEXT: )
1596+
;; CHECK-NEXT: )
1597+
;; CHECK-NEXT: )
1598+
;; CHECK-NEXT: )
1599+
(func $array-cmpxchg-expected-index-oob (param $array (ref $array))
1600+
(drop
1601+
;; Now the index is constant but surely out-of-bounds. We still optimize
1602+
;; the same, way leaving the array.atomic.get to trap.
1603+
(array.atomic.rmw.cmpxchg $array
1604+
(local.get $array)
1605+
(i32.const -1)
1606+
(array.new_default $array (i32.const 1))
1607+
(call $effect-eq)
1608+
)
1609+
)
1610+
)
1611+
)

0 commit comments

Comments
 (0)