Skip to content

Commit 4017ddd

Browse files
sumleosumleo
authored
Heap2Local: Fix Array2Struct missing handlers for ArrayRMW, ArrayCmpxchg (#8305)
The escape analysis in Heap2Local accepted `ArrayRMW` and `ArrayCmpxchg` as `FullyConsumes`, allowing non-escaping arrays with these operations through to the `Array2Struct` transformation. However, `Array2Struct` had no `visitArrayRMW` or `visitArrayCmpxchg` handlers. This caused these operations to be left unrewritten after the array-to-struct conversion, and the type rewriting infrastructure replaced them with unreachable blocks — silently turning working code into unconditional traps.
1 parent 47a72fb commit 4017ddd

File tree

2 files changed

+234
-1
lines changed

2 files changed

+234
-1
lines changed

‎src/passes/Heap2Local.cpp‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -462,12 +462,18 @@ struct EscapeAnalyzer {
462462
fullyConsumes = true;
463463
}
464464
void visitArrayRMW(ArrayRMW* curr) {
465+
if (!curr->index->is<Const>()) {
466+
return;
467+
}
465468
if (curr->ref == child) {
466469
escapes = false;
467470
fullyConsumes = true;
468471
}
469472
}
470473
void visitArrayCmpxchg(ArrayCmpxchg* curr) {
474+
if (!curr->index->is<Const>()) {
475+
return;
476+
}
471477
if (curr->ref == child || curr->expected == child) {
472478
escapes = false;
473479
fullyConsumes = true;
@@ -1353,6 +1359,45 @@ struct Array2Struct : PostWalker<Array2Struct> {
13531359
index, curr->ref, MemoryOrder::Unordered, curr->type, curr->signed_));
13541360
}
13551361

1362+
void visitArrayRMW(ArrayRMW* curr) {
1363+
if (analyzer.getInteraction(curr) == ParentChildInteraction::None) {
1364+
return;
1365+
}
1366+
1367+
auto index = getIndex(curr->index);
1368+
if (index >= numFields) {
1369+
replaceCurrent(builder.makeBlock({builder.makeDrop(curr->ref),
1370+
builder.makeDrop(curr->value),
1371+
builder.makeUnreachable()}));
1372+
refinalize = true;
1373+
return;
1374+
}
1375+
1376+
// Convert the ArrayRMW into a StructRMW.
1377+
replaceCurrent(builder.makeStructRMW(
1378+
curr->op, index, curr->ref, curr->value, curr->order));
1379+
}
1380+
1381+
void visitArrayCmpxchg(ArrayCmpxchg* curr) {
1382+
if (analyzer.getInteraction(curr) == ParentChildInteraction::None) {
1383+
return;
1384+
}
1385+
1386+
auto index = getIndex(curr->index);
1387+
if (index >= numFields) {
1388+
replaceCurrent(builder.makeBlock({builder.makeDrop(curr->ref),
1389+
builder.makeDrop(curr->expected),
1390+
builder.makeDrop(curr->replacement),
1391+
builder.makeUnreachable()}));
1392+
refinalize = true;
1393+
return;
1394+
}
1395+
1396+
// Convert the ArrayCmpxchg into a StructCmpxchg.
1397+
replaceCurrent(builder.makeStructCmpxchg(
1398+
index, curr->ref, curr->expected, curr->replacement, curr->order));
1399+
}
1400+
13561401
// Some additional operations need special handling
13571402

13581403
void visitRefTest(RefTest* curr) {

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

Lines changed: 189 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
(type $i64 (struct (field (mut i64))))
88
;; CHECK: (type $struct (struct (field (mut (ref null $struct)))))
99
(type $struct (struct (field (mut (ref null $struct)))))
10-
1110
;; CHECK: (type $1 (func (result i32)))
1211

1312
;; CHECK: (type $2 (func (result i64)))
@@ -16,6 +15,11 @@
1615

1716
;; CHECK: (type $4 (func (param (ref null $struct) (ref null $struct)) (result (ref null $struct))))
1817

18+
;; CHECK: (type $5 (func (param i32) (result i32)))
19+
20+
;; CHECK: (type $arr (array (mut i32)))
21+
(type $arr (array (mut i32)))
22+
1923
;; CHECK: (func $escape-rmw (type $3) (param $0 (ref null $struct)) (result (ref null $struct))
2024
;; CHECK-NEXT: (struct.atomic.rmw.xchg $struct 0
2125
;; CHECK-NEXT: (local.get $0)
@@ -750,4 +754,188 @@
750754
(i32.const 1)
751755
)
752756
)
757+
758+
;; CHECK: (func $array-rmw-add (type $1) (result i32)
759+
;; CHECK-NEXT: (local $0 i32)
760+
;; CHECK-NEXT: (local $1 i32)
761+
;; CHECK-NEXT: (local $2 i32)
762+
;; CHECK-NEXT: (local $3 i32)
763+
;; CHECK-NEXT: (local $4 i32)
764+
;; CHECK-NEXT: (local $5 i32)
765+
;; CHECK-NEXT: (drop
766+
;; CHECK-NEXT: (block (result nullref)
767+
;; CHECK-NEXT: (local.set $2
768+
;; CHECK-NEXT: (i32.const 0)
769+
;; CHECK-NEXT: )
770+
;; CHECK-NEXT: (local.set $3
771+
;; CHECK-NEXT: (i32.const 0)
772+
;; CHECK-NEXT: )
773+
;; CHECK-NEXT: (local.set $0
774+
;; CHECK-NEXT: (local.get $2)
775+
;; CHECK-NEXT: )
776+
;; CHECK-NEXT: (local.set $1
777+
;; CHECK-NEXT: (local.get $3)
778+
;; CHECK-NEXT: )
779+
;; CHECK-NEXT: (ref.null none)
780+
;; CHECK-NEXT: )
781+
;; CHECK-NEXT: )
782+
;; CHECK-NEXT: (local.set $5
783+
;; CHECK-NEXT: (i32.const 1)
784+
;; CHECK-NEXT: )
785+
;; CHECK-NEXT: (local.set $4
786+
;; CHECK-NEXT: (local.get $0)
787+
;; CHECK-NEXT: )
788+
;; CHECK-NEXT: (local.set $0
789+
;; CHECK-NEXT: (i32.add
790+
;; CHECK-NEXT: (local.get $0)
791+
;; CHECK-NEXT: (local.get $5)
792+
;; CHECK-NEXT: )
793+
;; CHECK-NEXT: )
794+
;; CHECK-NEXT: (local.get $4)
795+
;; CHECK-NEXT: )
796+
(func $array-rmw-add (result i32)
797+
;; Array atomic RMW on a non-escaping fixed-size array should be
798+
;; optimized: the array is converted to a struct, then to locals.
799+
(array.atomic.rmw.add $arr
800+
(array.new_fixed $arr 2
801+
(i32.const 0)
802+
(i32.const 0)
803+
)
804+
(i32.const 0)
805+
(i32.const 1)
806+
)
807+
)
808+
809+
;; CHECK: (func $array-cmpxchg (type $1) (result i32)
810+
;; CHECK-NEXT: (local $0 i32)
811+
;; CHECK-NEXT: (local $1 i32)
812+
;; CHECK-NEXT: (local $2 i32)
813+
;; CHECK-NEXT: (local $3 i32)
814+
;; CHECK-NEXT: (local $4 i32)
815+
;; CHECK-NEXT: (local $5 i32)
816+
;; CHECK-NEXT: (local $6 i32)
817+
;; CHECK-NEXT: (drop
818+
;; CHECK-NEXT: (block (result nullref)
819+
;; CHECK-NEXT: (local.set $2
820+
;; CHECK-NEXT: (i32.const 0)
821+
;; CHECK-NEXT: )
822+
;; CHECK-NEXT: (local.set $3
823+
;; CHECK-NEXT: (i32.const 0)
824+
;; CHECK-NEXT: )
825+
;; CHECK-NEXT: (local.set $0
826+
;; CHECK-NEXT: (local.get $2)
827+
;; CHECK-NEXT: )
828+
;; CHECK-NEXT: (local.set $1
829+
;; CHECK-NEXT: (local.get $3)
830+
;; CHECK-NEXT: )
831+
;; CHECK-NEXT: (ref.null none)
832+
;; CHECK-NEXT: )
833+
;; CHECK-NEXT: )
834+
;; CHECK-NEXT: (local.set $5
835+
;; CHECK-NEXT: (i32.const 10)
836+
;; CHECK-NEXT: )
837+
;; CHECK-NEXT: (local.set $6
838+
;; CHECK-NEXT: (i32.const 20)
839+
;; CHECK-NEXT: )
840+
;; CHECK-NEXT: (local.set $4
841+
;; CHECK-NEXT: (local.get $0)
842+
;; CHECK-NEXT: )
843+
;; CHECK-NEXT: (if
844+
;; CHECK-NEXT: (i32.eq
845+
;; CHECK-NEXT: (local.get $0)
846+
;; CHECK-NEXT: (local.get $5)
847+
;; CHECK-NEXT: )
848+
;; CHECK-NEXT: (then
849+
;; CHECK-NEXT: (local.set $0
850+
;; CHECK-NEXT: (local.get $6)
851+
;; CHECK-NEXT: )
852+
;; CHECK-NEXT: )
853+
;; CHECK-NEXT: )
854+
;; CHECK-NEXT: (local.get $4)
855+
;; CHECK-NEXT: )
856+
(func $array-cmpxchg (result i32)
857+
;; Array atomic cmpxchg on a non-escaping fixed-size array should be
858+
;; optimized similarly.
859+
(array.atomic.rmw.cmpxchg $arr
860+
(array.new_fixed $arr 2
861+
(i32.const 0)
862+
(i32.const 0)
863+
)
864+
(i32.const 0)
865+
(i32.const 10)
866+
(i32.const 20)
867+
)
868+
)
869+
870+
;; CHECK: (func $array-rmw-nonconstant-index (type $5) (param $idx i32) (result i32)
871+
;; CHECK-NEXT: (array.atomic.rmw.add $arr
872+
;; CHECK-NEXT: (array.new_fixed $arr 2
873+
;; CHECK-NEXT: (i32.const 0)
874+
;; CHECK-NEXT: (i32.const 0)
875+
;; CHECK-NEXT: )
876+
;; CHECK-NEXT: (local.get $idx)
877+
;; CHECK-NEXT: (i32.const 1)
878+
;; CHECK-NEXT: )
879+
;; CHECK-NEXT: )
880+
(func $array-rmw-nonconstant-index (param $idx i32) (result i32)
881+
;; A non-constant index prevents the optimization, since Array2Struct
882+
;; needs to know which struct field to access at compile time.
883+
(array.atomic.rmw.add $arr
884+
(array.new_fixed $arr 2
885+
(i32.const 0)
886+
(i32.const 0)
887+
)
888+
(local.get $idx)
889+
(i32.const 1)
890+
)
891+
)
892+
893+
;; CHECK: (func $array-rmw-oob (type $1) (result i32)
894+
;; CHECK-NEXT: (drop
895+
;; CHECK-NEXT: (block (result nullref)
896+
;; CHECK-NEXT: (ref.null none)
897+
;; CHECK-NEXT: )
898+
;; CHECK-NEXT: )
899+
;; CHECK-NEXT: (drop
900+
;; CHECK-NEXT: (i32.const 1)
901+
;; CHECK-NEXT: )
902+
;; CHECK-NEXT: (unreachable)
903+
;; CHECK-NEXT: )
904+
(func $array-rmw-oob (result i32)
905+
;; An out-of-bounds index on a zero-size array. The access will always
906+
;; trap, so we emit drops for operands and an unreachable.
907+
(array.atomic.rmw.add $arr
908+
(array.new_default $arr
909+
(i32.const 0)
910+
)
911+
(i32.const 0)
912+
(i32.const 1)
913+
)
914+
)
915+
916+
;; CHECK: (func $array-cmpxchg-oob (type $1) (result i32)
917+
;; CHECK-NEXT: (drop
918+
;; CHECK-NEXT: (block (result nullref)
919+
;; CHECK-NEXT: (ref.null none)
920+
;; CHECK-NEXT: )
921+
;; CHECK-NEXT: )
922+
;; CHECK-NEXT: (drop
923+
;; CHECK-NEXT: (i32.const 10)
924+
;; CHECK-NEXT: )
925+
;; CHECK-NEXT: (drop
926+
;; CHECK-NEXT: (i32.const 20)
927+
;; CHECK-NEXT: )
928+
;; CHECK-NEXT: (unreachable)
929+
;; CHECK-NEXT: )
930+
(func $array-cmpxchg-oob (result i32)
931+
;; As above, but for cmpxchg with an out-of-bounds index.
932+
(array.atomic.rmw.cmpxchg $arr
933+
(array.new_default $arr
934+
(i32.const 0)
935+
)
936+
(i32.const 0)
937+
(i32.const 10)
938+
(i32.const 20)
939+
)
940+
)
753941
)

0 commit comments

Comments
 (0)