Skip to content

Commit 6474c70

Browse files
tlivelykripken
tlively
and
kripken
authored
Handle ArrayCmpxchg in Heap2Local (#8551)
We already took care of converting array cmpxchg operations to struct cmpxchg operations in Array2Struct, but we did so even when the optimized allocation flowed into the `expected` field. In that case, since we aren't optimizing the accessed object, we should keep the array cmpxchg and later optimize it in Heap2Local. Fixes #8548. --------- Co-authored-by: Alon Zakai <alonzakai@gmail.com>
1 parent b6eba84 commit 6474c70

File tree

2 files changed

+175
-14
lines changed

2 files changed

+175
-14
lines changed

‎src/passes/Heap2Local.cpp‎

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1215,6 +1215,37 @@ struct Struct2Local : PostWalker<Struct2Local> {
12151215
block->type = type;
12161216
replaceCurrent(block);
12171217
}
1218+
1219+
void visitArrayCmpxchg(ArrayCmpxchg* curr) {
1220+
if (curr->type == Type::unreachable) {
1221+
// Leave this for DCE.
1222+
return;
1223+
}
1224+
// Array2Struct would have replaced this operation if the optimized
1225+
// allocation were flowing into the `ref` field, but not if it is flowing
1226+
// into the `expected` field.
1227+
if (analyzer.getInteraction(curr->expected) ==
1228+
ParentChildInteraction::Flows) {
1229+
// See the equivalent handling of allocations flowing through the
1230+
// `expected` field of StructCmpxchg.
1231+
auto refType = curr->ref->type.with(Nullable);
1232+
auto refScratch = builder.addVar(func, refType);
1233+
auto* setRefScratch = builder.makeLocalSet(refScratch, curr->ref);
1234+
auto* getRefScratch = builder.makeLocalGet(refScratch, refType);
1235+
auto* arrayGet = builder.makeArrayGet(
1236+
getRefScratch, curr->index, curr->order, curr->type);
1237+
auto* block = builder.makeBlock({setRefScratch,
1238+
builder.makeDrop(curr->expected),
1239+
builder.makeDrop(curr->replacement),
1240+
arrayGet});
1241+
replaceCurrent(block);
1242+
// Since `ref` must be an array and arrays are processed before structs,
1243+
// it is not possible that `ref` will be processed later. We therefore do
1244+
// not need to do the extra bookkeeping that we needed to do for
1245+
// StructCmpxchg.
1246+
return;
1247+
}
1248+
}
12181249
};
12191250

12201251
// An optimizer that handles the rewriting to turn a nonescaping array
@@ -1445,9 +1476,20 @@ struct Array2Struct : PostWalker<Array2Struct> {
14451476
return;
14461477
}
14471478

1448-
// Convert the ArrayCmpxchg into a StructCmpxchg.
1449-
replaceCurrent(builder.makeStructCmpxchg(
1450-
index, curr->ref, curr->expected, curr->replacement, curr->order));
1479+
// The allocation might flow into `ref` or `expected`, but not
1480+
// `replacement`, because then it would be considered to have escaped.
1481+
if (analyzer.getInteraction(curr->ref) == ParentChildInteraction::Flows) {
1482+
// The accessed array is being optimzied. Convert the ArrayCmpxchg into a
1483+
// StructCmpxchg.
1484+
replaceCurrent(builder.makeStructCmpxchg(
1485+
index, curr->ref, curr->expected, curr->replacement, curr->order));
1486+
return;
1487+
}
1488+
1489+
// The `expected` value must be getting optimized. Since the accessed object
1490+
// is remaining an array for now, do not change anything. The ArrayCmpxchg
1491+
// will be optimized later by Struct2Local.
1492+
return;
14511493
}
14521494

14531495
// Some additional operations need special handling

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

Lines changed: 130 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1085,22 +1085,24 @@
10851085
;; CHECK: (type $0 (func))
10861086

10871087
;; CHECK: (rec
1088-
;; CHECK-NEXT: (type $array (array i8))
1089-
(type $array (array i8))
1090-
;; CHECK: (type $struct (struct (field (mut anyref))))
1091-
(type $struct (struct (field (mut anyref))))
1088+
;; CHECK-NEXT: (type $inner (array i8))
1089+
(type $inner (array i8))
1090+
;; CHECK: (type $struct (struct (field (mut eqref))))
1091+
(type $struct (struct (field (mut eqref))))
1092+
;; CHECK: (type $array (array (mut eqref)))
1093+
(type $array (array (field (mut eqref))))
10921094
)
10931095
;; CHECK: (func $test-cmpxchg-scratch-oob (type $0)
10941096
;; CHECK-NEXT: (local $a arrayref)
10951097
;; CHECK-NEXT: (local $1 (ref null (exact $struct)))
1096-
;; CHECK-NEXT: (local $2 anyref)
1098+
;; CHECK-NEXT: (local $2 eqref)
10971099
;; CHECK-NEXT: (drop
10981100
;; CHECK-NEXT: (block (result nullref)
10991101
;; CHECK-NEXT: (ref.null none)
11001102
;; CHECK-NEXT: )
11011103
;; CHECK-NEXT: )
11021104
;; CHECK-NEXT: (drop
1103-
;; CHECK-NEXT: (block (result anyref)
1105+
;; CHECK-NEXT: (block (result eqref)
11041106
;; CHECK-NEXT: (drop
11051107
;; CHECK-NEXT: (block (result nullref)
11061108
;; CHECK-NEXT: (local.set $2
@@ -1115,9 +1117,9 @@
11151117
;; CHECK-NEXT: )
11161118
;; CHECK-NEXT: )
11171119
;; CHECK-NEXT: (drop
1118-
;; CHECK-NEXT: (array.new_fixed $array 0)
1120+
;; CHECK-NEXT: (array.new_fixed $inner 0)
11191121
;; CHECK-NEXT: )
1120-
;; CHECK-NEXT: (block (result anyref)
1122+
;; CHECK-NEXT: (block (result eqref)
11211123
;; CHECK-NEXT: (drop
11221124
;; CHECK-NEXT: (ref.null none)
11231125
;; CHECK-NEXT: )
@@ -1131,16 +1133,84 @@
11311133
(local $a arrayref)
11321134
;; This allocation and set get optimized, creating the LocalGraph flower.
11331135
(local.set $a
1134-
(array.new_fixed $array 0)
1136+
(array.new_fixed $inner 0)
11351137
)
11361138
(drop
11371139
;; Then `expected` gets optimized, creating a new scratch local. Since the
11381140
;; flower is already created, the index of the scratch local would be out
11391141
;; of bounds if we tried to look it up in the LocalGraph.
11401142
(struct.atomic.rmw.cmpxchg $struct 0
11411143
(struct.new_default $struct)
1142-
(array.new_fixed $array 0)
1143-
(array.new_fixed $array 0)
1144+
(array.new_fixed $inner 0)
1145+
(array.new_fixed $inner 0)
1146+
)
1147+
)
1148+
(unreachable)
1149+
)
1150+
1151+
;; CHECK: (func $test-cmpxchg-scratch-oob-array (type $0)
1152+
;; CHECK-NEXT: (local $a arrayref)
1153+
;; CHECK-NEXT: (local $1 eqref)
1154+
;; CHECK-NEXT: (local $2 eqref)
1155+
;; CHECK-NEXT: (local $3 eqref)
1156+
;; CHECK-NEXT: (local $4 eqref)
1157+
;; CHECK-NEXT: (drop
1158+
;; CHECK-NEXT: (block (result nullref)
1159+
;; CHECK-NEXT: (ref.null none)
1160+
;; CHECK-NEXT: )
1161+
;; CHECK-NEXT: )
1162+
;; CHECK-NEXT: (drop
1163+
;; CHECK-NEXT: (block (result eqref)
1164+
;; CHECK-NEXT: (drop
1165+
;; CHECK-NEXT: (block (result nullref)
1166+
;; CHECK-NEXT: (local.set $1
1167+
;; CHECK-NEXT: (ref.null none)
1168+
;; CHECK-NEXT: )
1169+
;; CHECK-NEXT: (ref.null none)
1170+
;; CHECK-NEXT: )
1171+
;; CHECK-NEXT: )
1172+
;; CHECK-NEXT: (local.set $3
1173+
;; CHECK-NEXT: (block (result nullref)
1174+
;; CHECK-NEXT: (ref.null none)
1175+
;; CHECK-NEXT: )
1176+
;; CHECK-NEXT: )
1177+
;; CHECK-NEXT: (local.set $4
1178+
;; CHECK-NEXT: (array.new_fixed $inner 0)
1179+
;; CHECK-NEXT: )
1180+
;; CHECK-NEXT: (local.set $2
1181+
;; CHECK-NEXT: (local.get $1)
1182+
;; CHECK-NEXT: )
1183+
;; CHECK-NEXT: (if
1184+
;; CHECK-NEXT: (ref.eq
1185+
;; CHECK-NEXT: (local.get $1)
1186+
;; CHECK-NEXT: (local.get $3)
1187+
;; CHECK-NEXT: )
1188+
;; CHECK-NEXT: (then
1189+
;; CHECK-NEXT: (local.set $1
1190+
;; CHECK-NEXT: (local.get $4)
1191+
;; CHECK-NEXT: )
1192+
;; CHECK-NEXT: )
1193+
;; CHECK-NEXT: )
1194+
;; CHECK-NEXT: (local.get $2)
1195+
;; CHECK-NEXT: )
1196+
;; CHECK-NEXT: )
1197+
;; CHECK-NEXT: (unreachable)
1198+
;; CHECK-NEXT: )
1199+
(func $test-cmpxchg-scratch-oob-array
1200+
;; Same as above, but accessing an array type. `ref` is always processed
1201+
;; first, so we do not have the same problem to avoid.
1202+
(local $a arrayref)
1203+
(local.set $a
1204+
(array.new_fixed $inner 0)
1205+
)
1206+
(drop
1207+
(array.atomic.rmw.cmpxchg $array
1208+
(array.new_default $array
1209+
(i32.const 1)
1210+
)
1211+
(i32.const 0)
1212+
(array.new_fixed $inner 0)
1213+
(array.new_fixed $inner 0)
11441214
)
11451215
)
11461216
(unreachable)
@@ -1318,3 +1388,52 @@
13181388
)
13191389
)
13201390
)
1391+
1392+
(module
1393+
;; CHECK: (type $array (array (mut eqref)))
1394+
(type $array (array (mut eqref)))
1395+
1396+
;; CHECK: (type $1 (func (param (ref $array))))
1397+
1398+
;; CHECK: (func $array-cmpxchg-expected (type $1) (param $array (ref $array))
1399+
;; CHECK-NEXT: (local $1 eqref)
1400+
;; CHECK-NEXT: (local $2 (ref null $array))
1401+
;; CHECK-NEXT: (drop
1402+
;; CHECK-NEXT: (block (result eqref)
1403+
;; CHECK-NEXT: (local.set $2
1404+
;; CHECK-NEXT: (local.get $array)
1405+
;; CHECK-NEXT: )
1406+
;; CHECK-NEXT: (drop
1407+
;; CHECK-NEXT: (block (result nullref)
1408+
;; CHECK-NEXT: (local.set $1
1409+
;; CHECK-NEXT: (ref.null none)
1410+
;; CHECK-NEXT: )
1411+
;; CHECK-NEXT: (ref.null none)
1412+
;; CHECK-NEXT: )
1413+
;; CHECK-NEXT: )
1414+
;; CHECK-NEXT: (drop
1415+
;; CHECK-NEXT: (ref.null none)
1416+
;; CHECK-NEXT: )
1417+
;; CHECK-NEXT: (array.atomic.get $array
1418+
;; CHECK-NEXT: (local.get $2)
1419+
;; CHECK-NEXT: (i32.const 0)
1420+
;; CHECK-NEXT: )
1421+
;; CHECK-NEXT: )
1422+
;; CHECK-NEXT: )
1423+
;; CHECK-NEXT: )
1424+
(func $array-cmpxchg-expected (param $array (ref $array))
1425+
(drop
1426+
;; We should not convert this to a struct cmpxchg because we do not
1427+
;; optimize the `ref`. We should still be able to optimize the `expected`
1428+
;; field, though.
1429+
(array.atomic.rmw.cmpxchg $array
1430+
(local.get $array)
1431+
(i32.const 0)
1432+
(array.new_default $array
1433+
(i32.const 1)
1434+
)
1435+
(ref.null none)
1436+
)
1437+
)
1438+
)
1439+
)

0 commit comments

Comments
 (0)