Skip to content

Commit 82c0c99

Browse files
tlivelytlively
authored
Fix scratch local when optimizing cmpxchg in Heap2Local (#8496)
When optimizing the `ref` operand to a StructCmpxchg in Heap2Local, we previously used a scratch local with the type of the accessed struct field to hold `expected`. But `expected` is allowed to have any subtype of `eqref` (or shared `eqref`). Fix the type of the scratch local to avoid validation failures.
1 parent 4ee183b commit 82c0c99

2 files changed

Lines changed: 147 additions & 4 deletions

File tree

‎src/passes/Heap2Local.cpp‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1175,9 +1175,15 @@ struct Struct2Local : PostWalker<Struct2Local> {
11751175
assert(!field.isPacked());
11761176

11771177
// Hold everything in scratch locals, just like for other RMW ops and
1178-
// struct.new.
1178+
// struct.new. Use a nullable (shared) eqref local for `expected` to
1179+
// accommodate any allowed optimized or unoptimized value there.
1180+
auto expectedType = type;
1181+
if (type.isRef()) {
1182+
expectedType =
1183+
Type(HeapTypes::eq.getBasic(type.getHeapType().getShared()), Nullable);
1184+
}
11791185
auto oldScratch = builder.addVar(func, type);
1180-
auto expectedScratch = builder.addVar(func, type);
1186+
auto expectedScratch = builder.addVar(func, expectedType);
11811187
auto replacementScratch = builder.addVar(func, type);
11821188
auto local = localIndexes[curr->index];
11831189

@@ -1189,7 +1195,7 @@ struct Struct2Local : PostWalker<Struct2Local> {
11891195

11901196
// Create the check for whether we should do the exchange.
11911197
auto* lhs = builder.makeLocalGet(local, type);
1192-
auto* rhs = builder.makeLocalGet(expectedScratch, type);
1198+
auto* rhs = builder.makeLocalGet(expectedScratch, expectedType);
11931199
Expression* pred;
11941200
if (type.isRef()) {
11951201
pred = builder.makeRefEq(lhs, rhs);

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

Lines changed: 138 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -590,7 +590,7 @@
590590
;; CHECK: (func $rmw-cmpxchg-ref (type $4) (param $0 (ref null $struct)) (param $1 (ref null $struct)) (result (ref null $struct))
591591
;; CHECK-NEXT: (local $2 (ref null $struct))
592592
;; CHECK-NEXT: (local $3 (ref null $struct))
593-
;; CHECK-NEXT: (local $4 (ref null $struct))
593+
;; CHECK-NEXT: (local $4 eqref)
594594
;; CHECK-NEXT: (local $5 (ref null $struct))
595595
;; CHECK-NEXT: (drop
596596
;; CHECK-NEXT: (block (result nullref)
@@ -1146,3 +1146,140 @@
11461146
(unreachable)
11471147
)
11481148
)
1149+
1150+
(module
1151+
(rec
1152+
;; CHECK: (rec
1153+
;; CHECK-NEXT: (type $outer (struct (field (mut (ref $inner)))))
1154+
(type $outer (struct (field (mut (ref $inner)))))
1155+
;; CHECK: (type $inner (struct))
1156+
(type $inner (struct))
1157+
1158+
;; CHECK: (type $shared-outer (shared (struct (field (mut (ref $shared-inner))))))
1159+
(type $shared-outer (shared (struct (field (mut (ref $shared-inner))))))
1160+
;; CHECK: (type $shared-inner (shared (struct)))
1161+
(type $shared-inner (shared (struct)))
1162+
)
1163+
1164+
;; CHECK: (type $4 (func))
1165+
1166+
;; CHECK: (func $cmpxchg-non-nullable-field (type $4)
1167+
;; CHECK-NEXT: (local $0 (ref $inner))
1168+
;; CHECK-NEXT: (local $1 (ref $inner))
1169+
;; CHECK-NEXT: (local $2 (ref $inner))
1170+
;; CHECK-NEXT: (local $3 eqref)
1171+
;; CHECK-NEXT: (local $4 (ref $inner))
1172+
;; CHECK-NEXT: (drop
1173+
;; CHECK-NEXT: (block (result (ref $inner))
1174+
;; CHECK-NEXT: (drop
1175+
;; CHECK-NEXT: (block (result nullref)
1176+
;; CHECK-NEXT: (local.set $1
1177+
;; CHECK-NEXT: (struct.new_default $inner)
1178+
;; CHECK-NEXT: )
1179+
;; CHECK-NEXT: (local.set $0
1180+
;; CHECK-NEXT: (local.get $1)
1181+
;; CHECK-NEXT: )
1182+
;; CHECK-NEXT: (ref.null none)
1183+
;; CHECK-NEXT: )
1184+
;; CHECK-NEXT: )
1185+
;; CHECK-NEXT: (local.set $3
1186+
;; CHECK-NEXT: (block (result nullref)
1187+
;; CHECK-NEXT: (ref.null none)
1188+
;; CHECK-NEXT: )
1189+
;; CHECK-NEXT: )
1190+
;; CHECK-NEXT: (local.set $4
1191+
;; CHECK-NEXT: (struct.new_default $inner)
1192+
;; CHECK-NEXT: )
1193+
;; CHECK-NEXT: (local.set $2
1194+
;; CHECK-NEXT: (local.get $0)
1195+
;; CHECK-NEXT: )
1196+
;; CHECK-NEXT: (if
1197+
;; CHECK-NEXT: (ref.eq
1198+
;; CHECK-NEXT: (local.get $0)
1199+
;; CHECK-NEXT: (local.get $3)
1200+
;; CHECK-NEXT: )
1201+
;; CHECK-NEXT: (then
1202+
;; CHECK-NEXT: (local.set $0
1203+
;; CHECK-NEXT: (local.get $4)
1204+
;; CHECK-NEXT: )
1205+
;; CHECK-NEXT: )
1206+
;; CHECK-NEXT: )
1207+
;; CHECK-NEXT: (local.get $2)
1208+
;; CHECK-NEXT: )
1209+
;; CHECK-NEXT: )
1210+
;; CHECK-NEXT: )
1211+
(func $cmpxchg-non-nullable-field
1212+
(drop
1213+
(struct.atomic.rmw.cmpxchg $outer 0
1214+
;; When `ref` gets optimized, we need to make sure the scratch local for
1215+
;; `expected` is nullable, even though the operand and field are both
1216+
;; non-nullable. This avoids type errors when we later optimize the
1217+
;; `expected` field and make it a nullref.
1218+
(struct.new $outer
1219+
(struct.new_default $inner)
1220+
)
1221+
(struct.new_default $inner)
1222+
(struct.new_default $inner)
1223+
)
1224+
)
1225+
)
1226+
1227+
;; CHECK: (func $cmpxchg-non-nullable-field-shared (type $4)
1228+
;; CHECK-NEXT: (local $0 (ref $shared-inner))
1229+
;; CHECK-NEXT: (local $1 (ref $shared-inner))
1230+
;; CHECK-NEXT: (local $2 (ref $shared-inner))
1231+
;; CHECK-NEXT: (local $3 (ref null (shared eq)))
1232+
;; CHECK-NEXT: (local $4 (ref $shared-inner))
1233+
;; CHECK-NEXT: (drop
1234+
;; CHECK-NEXT: (block (result (ref $shared-inner))
1235+
;; CHECK-NEXT: (drop
1236+
;; CHECK-NEXT: (block (result (ref null (shared none)))
1237+
;; CHECK-NEXT: (local.set $1
1238+
;; CHECK-NEXT: (struct.new_default $shared-inner)
1239+
;; CHECK-NEXT: )
1240+
;; CHECK-NEXT: (local.set $0
1241+
;; CHECK-NEXT: (local.get $1)
1242+
;; CHECK-NEXT: )
1243+
;; CHECK-NEXT: (ref.null (shared none))
1244+
;; CHECK-NEXT: )
1245+
;; CHECK-NEXT: )
1246+
;; CHECK-NEXT: (local.set $3
1247+
;; CHECK-NEXT: (block (result (ref null (shared none)))
1248+
;; CHECK-NEXT: (ref.null (shared none))
1249+
;; CHECK-NEXT: )
1250+
;; CHECK-NEXT: )
1251+
;; CHECK-NEXT: (local.set $4
1252+
;; CHECK-NEXT: (struct.new_default $shared-inner)
1253+
;; CHECK-NEXT: )
1254+
;; CHECK-NEXT: (local.set $2
1255+
;; CHECK-NEXT: (local.get $0)
1256+
;; CHECK-NEXT: )
1257+
;; CHECK-NEXT: (if
1258+
;; CHECK-NEXT: (ref.eq
1259+
;; CHECK-NEXT: (local.get $0)
1260+
;; CHECK-NEXT: (local.get $3)
1261+
;; CHECK-NEXT: )
1262+
;; CHECK-NEXT: (then
1263+
;; CHECK-NEXT: (local.set $0
1264+
;; CHECK-NEXT: (local.get $4)
1265+
;; CHECK-NEXT: )
1266+
;; CHECK-NEXT: )
1267+
;; CHECK-NEXT: )
1268+
;; CHECK-NEXT: (local.get $2)
1269+
;; CHECK-NEXT: )
1270+
;; CHECK-NEXT: )
1271+
;; CHECK-NEXT: )
1272+
(func $cmpxchg-non-nullable-field-shared
1273+
(drop
1274+
(struct.atomic.rmw.cmpxchg $shared-outer 0
1275+
;; Same, but now with shared types. The scratch local must be a shared
1276+
;; eqref.
1277+
(struct.new $shared-outer
1278+
(struct.new_default $shared-inner)
1279+
)
1280+
(struct.new_default $shared-inner)
1281+
(struct.new_default $shared-inner)
1282+
)
1283+
)
1284+
)
1285+
)

0 commit comments

Comments
 (0)