Skip to content

Commit 05aac53

Browse files
kripkenkripken
authored
Precompute: Do not cache calculations that ignore effects (#7808)
Precompute has two modes, one where we ignore effects. We were caching the effectless results for where the normal mode (which cares about effects) would reuse them, leading to bugs. Fixes a regression from #7763, another after #7766. We were not using the cache fully, before, and caches are bug-prone, sadly.
1 parent 8d72140 commit 05aac53

File tree

2 files changed

+122
-29
lines changed

2 files changed

+122
-29
lines changed

‎src/passes/Precompute.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,14 @@ class PrecomputingExpressionRunner
200200
if (flow.breaking()) {
201201
return flow;
202202
}
203-
heapValues[curr] = flow.getSingleValue().getGCData();
203+
// If we preserve side effects then we can cache the results, but if we
204+
// ignore them then the result we compute does not contain them, and later
205+
// reads from the cache that do care about side effects would be wrong.
206+
// TODO: use a separate cache for the two modes, but the other mode is
207+
// only used in propagateLocals, which is far less used
208+
if (flags & FlagValues::PRESERVE_SIDEEFFECTS) {
209+
heapValues[curr] = flow.getSingleValue().getGCData();
210+
}
204211
return flow;
205212
}
206213

‎test/lit/passes/precompute-gc.wast

Lines changed: 114 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,21 @@
1515

1616
;; CHECK: (type $func-return-i32 (func (result i32)))
1717

18+
;; CHECK: (type $array-i32 (array (mut i32)))
19+
1820
;; CHECK: (type $B (struct (field (mut f64))))
1921
(type $B (struct (field (mut f64))))
2022

2123
(type $struct_i8 (struct (field i8)))
2224

2325
(type $func-return-i32 (func (result i32)))
2426

25-
;; CHECK: (import "fuzzing-support" "log-i32" (func $log (type $4) (param i32)))
27+
(type $array-i32 (array (mut i32)))
28+
29+
;; CHECK: (type $array-ref (array (mut (ref null $array-i32))))
30+
(type $array-ref (array (mut (ref null $array-i32))))
31+
32+
;; CHECK: (import "fuzzing-support" "log-i32" (func $log (type $5) (param i32)))
2633
(import "fuzzing-support" "log-i32" (func $log (param i32)))
2734

2835
;; CHECK: (func $test-fallthrough (type $func-return-i32) (result i32)
@@ -57,7 +64,7 @@
5764
)
5865
)
5966

60-
;; CHECK: (func $load-from-struct (type $3)
67+
;; CHECK: (func $load-from-struct (type $2)
6168
;; CHECK-NEXT: (local $x (ref null $struct))
6269
;; CHECK-NEXT: (local.set $x
6370
;; CHECK-NEXT: (struct.new $struct
@@ -119,7 +126,7 @@
119126
(struct.get $struct 0 (local.get $x))
120127
)
121128
)
122-
;; CHECK: (func $load-from-struct-bad-merge (type $4) (param $i i32)
129+
;; CHECK: (func $load-from-struct-bad-merge (type $5) (param $i i32)
123130
;; CHECK-NEXT: (local $x (ref null $struct))
124131
;; CHECK-NEXT: (if
125132
;; CHECK-NEXT: (local.get $i)
@@ -168,7 +175,7 @@
168175
(struct.get $struct 0 (local.get $x))
169176
)
170177
)
171-
;; CHECK: (func $modify-gc-heap (type $5) (param $x (ref null $struct))
178+
;; CHECK: (func $modify-gc-heap (type $6) (param $x (ref null $struct))
172179
;; CHECK-NEXT: (struct.set $struct 0
173180
;; CHECK-NEXT: (local.get $x)
174181
;; CHECK-NEXT: (i32.add
@@ -192,7 +199,7 @@
192199
)
193200
;; --fuzz-exec verifies the output of this function, checking that the change
194201
;; makde in modify-gc-heap is not ignored
195-
;; CHECK: (func $load-from-struct-bad-escape (type $3)
202+
;; CHECK: (func $load-from-struct-bad-escape (type $2)
196203
;; CHECK-NEXT: (local $x (ref null $struct))
197204
;; CHECK-NEXT: (local.set $x
198205
;; CHECK-NEXT: (struct.new $struct
@@ -222,7 +229,7 @@
222229
(struct.get $struct 0 (local.get $x))
223230
)
224231
)
225-
;; CHECK: (func $load-from-struct-bad-arrive (type $5) (param $x (ref null $struct))
232+
;; CHECK: (func $load-from-struct-bad-arrive (type $6) (param $x (ref null $struct))
226233
;; CHECK-NEXT: (call $log
227234
;; CHECK-NEXT: (struct.get $struct 0
228235
;; CHECK-NEXT: (local.get $x)
@@ -235,7 +242,7 @@
235242
(struct.get $struct 0 (local.get $x))
236243
)
237244
)
238-
;; CHECK: (func $ref-comparisons (type $11) (param $x (ref null $struct)) (param $y (ref null $struct))
245+
;; CHECK: (func $ref-comparisons (type $12) (param $x (ref null $struct)) (param $y (ref null $struct))
239246
;; CHECK-NEXT: (local $z (ref null $struct))
240247
;; CHECK-NEXT: (local $w (ref null $struct))
241248
;; CHECK-NEXT: (call $log
@@ -388,7 +395,7 @@
388395
(local.get $tempresult)
389396
)
390397

391-
;; CHECK: (func $propagate-uncertain-param (type $6) (param $input (ref $empty)) (result i32)
398+
;; CHECK: (func $propagate-uncertain-param (type $7) (param $input (ref $empty)) (result i32)
392399
;; CHECK-NEXT: (local $tempresult i32)
393400
;; CHECK-NEXT: (local $tempref (ref null $empty))
394401
;; CHECK-NEXT: (local.set $tempresult
@@ -413,7 +420,7 @@
413420
(local.get $tempresult)
414421
)
415422

416-
;; CHECK: (func $propagate-different-params (type $12) (param $input1 (ref $empty)) (param $input2 (ref $empty)) (result i32)
423+
;; CHECK: (func $propagate-different-params (type $13) (param $input1 (ref $empty)) (param $input2 (ref $empty)) (result i32)
417424
;; CHECK-NEXT: (local $tempresult i32)
418425
;; CHECK-NEXT: (local.set $tempresult
419426
;; CHECK-NEXT: (ref.eq
@@ -435,7 +442,7 @@
435442
(local.get $tempresult)
436443
)
437444

438-
;; CHECK: (func $propagate-same-param (type $6) (param $input (ref $empty)) (result i32)
445+
;; CHECK: (func $propagate-same-param (type $7) (param $input (ref $empty)) (result i32)
439446
;; CHECK-NEXT: (local $tempresult i32)
440447
;; CHECK-NEXT: (local.set $tempresult
441448
;; CHECK-NEXT: (ref.eq
@@ -517,7 +524,7 @@
517524
(local.get $tempresult)
518525
)
519526

520-
;; CHECK: (func $propagate-uncertain-loop (type $3)
527+
;; CHECK: (func $propagate-uncertain-loop (type $2)
521528
;; CHECK-NEXT: (local $tempresult i32)
522529
;; CHECK-NEXT: (local $tempref (ref null $empty))
523530
;; CHECK-NEXT: (local $stashedref (ref null $empty))
@@ -574,7 +581,7 @@
574581
)
575582
)
576583

577-
;; CHECK: (func $propagate-certain-loop (type $3)
584+
;; CHECK: (func $propagate-certain-loop (type $2)
578585
;; CHECK-NEXT: (local $tempresult i32)
579586
;; CHECK-NEXT: (local $tempref (ref null $empty))
580587
;; CHECK-NEXT: (local $stashedref (ref null $empty))
@@ -622,7 +629,7 @@
622629
)
623630
)
624631

625-
;; CHECK: (func $propagate-certain-loop-2 (type $3)
632+
;; CHECK: (func $propagate-certain-loop-2 (type $2)
626633
;; CHECK-NEXT: (local $tempresult i32)
627634
;; CHECK-NEXT: (local $tempref (ref null $empty))
628635
;; CHECK-NEXT: (local $stashedref (ref null $empty))
@@ -670,7 +677,7 @@
670677
)
671678
)
672679

673-
;; CHECK: (func $propagate-possibly-certain-loop (type $3)
680+
;; CHECK: (func $propagate-possibly-certain-loop (type $2)
674681
;; CHECK-NEXT: (local $tempresult i32)
675682
;; CHECK-NEXT: (local $tempref (ref null $empty))
676683
;; CHECK-NEXT: (local $stashedref (ref null $empty))
@@ -737,14 +744,14 @@
737744
)
738745
)
739746

740-
;; CHECK: (func $helper (type $13) (param $0 i32) (result i32)
747+
;; CHECK: (func $helper (type $14) (param $0 i32) (result i32)
741748
;; CHECK-NEXT: (unreachable)
742749
;; CHECK-NEXT: )
743750
(func $helper (param i32) (result i32)
744751
(unreachable)
745752
)
746753

747-
;; CHECK: (func $odd-cast-and-get (type $3)
754+
;; CHECK: (func $odd-cast-and-get (type $2)
748755
;; CHECK-NEXT: (local $temp (ref null $B))
749756
;; CHECK-NEXT: (local.set $temp
750757
;; CHECK-NEXT: (ref.null none)
@@ -778,7 +785,7 @@
778785
)
779786
)
780787

781-
;; CHECK: (func $odd-cast-and-get-tuple (type $3)
788+
;; CHECK: (func $odd-cast-and-get-tuple (type $2)
782789
;; CHECK-NEXT: (local $temp (tuple (ref null $B) i32))
783790
;; CHECK-NEXT: (local.set $temp
784791
;; CHECK-NEXT: (tuple.make 2
@@ -815,14 +822,14 @@
815822
)
816823
)
817824

818-
;; CHECK: (func $receive-f64 (type $14) (param $0 f64)
825+
;; CHECK: (func $receive-f64 (type $15) (param $0 f64)
819826
;; CHECK-NEXT: (unreachable)
820827
;; CHECK-NEXT: )
821828
(func $receive-f64 (param f64)
822829
(unreachable)
823830
)
824831

825-
;; CHECK: (func $odd-cast-and-get-non-null (type $15) (param $temp (ref $func-return-i32))
832+
;; CHECK: (func $odd-cast-and-get-non-null (type $16) (param $temp (ref $func-return-i32))
826833
;; CHECK-NEXT: (local.set $temp
827834
;; CHECK-NEXT: (ref.cast (ref nofunc)
828835
;; CHECK-NEXT: (ref.func $receive-f64)
@@ -850,7 +857,7 @@
850857
)
851858
)
852859

853-
;; CHECK: (func $new_block_unreachable (type $8) (result anyref)
860+
;; CHECK: (func $new_block_unreachable (type $9) (result anyref)
854861
;; CHECK-NEXT: (block ;; (replaces unreachable StructNew we can't emit)
855862
;; CHECK-NEXT: (drop
856863
;; CHECK-NEXT: (block
@@ -871,7 +878,7 @@
871878
)
872879
)
873880

874-
;; CHECK: (func $br_on_cast-on-creation (type $16) (result (ref $empty))
881+
;; CHECK: (func $br_on_cast-on-creation (type $17) (result (ref $empty))
875882
;; CHECK-NEXT: (block $label (result (ref (exact $empty)))
876883
;; CHECK-NEXT: (drop
877884
;; CHECK-NEXT: (br_on_cast $label (ref (exact $empty)) (ref (exact $empty))
@@ -892,7 +899,7 @@
892899
)
893900
)
894901

895-
;; CHECK: (func $ref.is_null (type $4) (param $param i32)
902+
;; CHECK: (func $ref.is_null (type $5) (param $param i32)
896903
;; CHECK-NEXT: (local $ref (ref null $empty))
897904
;; CHECK-NEXT: (local.set $ref
898905
;; CHECK-NEXT: (struct.new_default $empty)
@@ -970,7 +977,7 @@
970977
)
971978
)
972979

973-
;; CHECK: (func $remove-set (type $17) (result (ref func))
980+
;; CHECK: (func $remove-set (type $18) (result (ref func))
974981
;; CHECK-NEXT: (local $nn funcref)
975982
;; CHECK-NEXT: (local $i i32)
976983
;; CHECK-NEXT: (loop $loop
@@ -1013,7 +1020,7 @@
10131020
)
10141021
)
10151022

1016-
;; CHECK: (func $strings (type $18) (param $param (ref string))
1023+
;; CHECK: (func $strings (type $19) (param $param (ref string))
10171024
;; CHECK-NEXT: (local $s (ref string))
10181025
;; CHECK-NEXT: (local.set $s
10191026
;; CHECK-NEXT: (string.const "hello, world")
@@ -1052,7 +1059,7 @@
10521059
)
10531060
)
10541061

1055-
;; CHECK: (func $get-nonnullable-in-unreachable (type $8) (result anyref)
1062+
;; CHECK: (func $get-nonnullable-in-unreachable (type $9) (result anyref)
10561063
;; CHECK-NEXT: (local $x (ref any))
10571064
;; CHECK-NEXT: (local.tee $x
10581065
;; CHECK-NEXT: (unreachable)
@@ -1089,7 +1096,7 @@
10891096
(local.get $x)
10901097
)
10911098

1092-
;; CHECK: (func $get-nonnullable-in-unreachable-entry (type $9) (param $x i32) (param $y (ref any))
1099+
;; CHECK: (func $get-nonnullable-in-unreachable-entry (type $10) (param $x i32) (param $y (ref any))
10931100
;; CHECK-NEXT: (local $0 (ref any))
10941101
;; CHECK-NEXT: (unreachable)
10951102
;; CHECK-NEXT: (local.set $0
@@ -1123,7 +1130,7 @@
11231130
)
11241131
)
11251132

1126-
;; CHECK: (func $get-nonnullable-in-unreachable-later-loop (type $9) (param $x i32) (param $y (ref any))
1133+
;; CHECK: (func $get-nonnullable-in-unreachable-later-loop (type $10) (param $x i32) (param $y (ref any))
11271134
;; CHECK-NEXT: (local $0 (ref any))
11281135
;; CHECK-NEXT: (if
11291136
;; CHECK-NEXT: (local.get $x)
@@ -1168,7 +1175,7 @@
11681175
)
11691176
)
11701177

1171-
;; CHECK: (func $get-nonnullable-in-unreachable-tuple (type $19) (result anyref i32)
1178+
;; CHECK: (func $get-nonnullable-in-unreachable-tuple (type $20) (result anyref i32)
11721179
;; CHECK-NEXT: (local $x (tuple (ref any) i32))
11731180
;; CHECK-NEXT: (local.tee $x
11741181
;; CHECK-NEXT: (unreachable)
@@ -1196,4 +1203,83 @@
11961203
)
11971204
(local.get $x)
11981205
)
1206+
1207+
;; CHECK: (func $propagate-array-effects (type $2)
1208+
;; CHECK-NEXT: (local $1 (ref $array-i32))
1209+
;; CHECK-NEXT: (local $2 i32)
1210+
;; CHECK-NEXT: (local $3 (ref $array-i32))
1211+
;; CHECK-NEXT: (drop
1212+
;; CHECK-NEXT: (i32.lt_u
1213+
;; CHECK-NEXT: (local.tee $2
1214+
;; CHECK-NEXT: (select
1215+
;; CHECK-NEXT: (i32.const 0)
1216+
;; CHECK-NEXT: (block (result i32)
1217+
;; CHECK-NEXT: (drop
1218+
;; CHECK-NEXT: (array.new $array-ref
1219+
;; CHECK-NEXT: (local.tee $1
1220+
;; CHECK-NEXT: (array.new_default $array-i32
1221+
;; CHECK-NEXT: (i32.const 0)
1222+
;; CHECK-NEXT: )
1223+
;; CHECK-NEXT: )
1224+
;; CHECK-NEXT: (i32.const 0)
1225+
;; CHECK-NEXT: )
1226+
;; CHECK-NEXT: )
1227+
;; CHECK-NEXT: (i32.const 0)
1228+
;; CHECK-NEXT: )
1229+
;; CHECK-NEXT: (i32.const 0)
1230+
;; CHECK-NEXT: )
1231+
;; CHECK-NEXT: )
1232+
;; CHECK-NEXT: (array.len
1233+
;; CHECK-NEXT: (local.tee $3
1234+
;; CHECK-NEXT: (local.get $1)
1235+
;; CHECK-NEXT: )
1236+
;; CHECK-NEXT: )
1237+
;; CHECK-NEXT: )
1238+
;; CHECK-NEXT: )
1239+
;; CHECK-NEXT: )
1240+
(func $propagate-array-effects
1241+
(local $1 (ref $array-i32))
1242+
(local $2 i32)
1243+
(local $3 (ref $array-i32))
1244+
;; This dropped expression has several local operations that lead to
1245+
;; propagateLocals doing several passes. While doing so, it will try to
1246+
;; precompute the value of the outer $array.new. That contains a nested
1247+
;; tee, so we cannot replace it with the value we compute, but we do know its
1248+
;; value (the canonical allocation for that array.new location). We ignore the
1249+
;; local.tee's effect when computing that, but we should not cache the result
1250+
;; of that computation and use it for when we *do* care about effects, because
1251+
;; if we did then we'd think that entire array.new has no effects, and can be
1252+
;; optimized away, together with large chunks of the rest of the code.
1253+
;;
1254+
;; We should not succeed in optimizing anything away here.
1255+
(drop
1256+
(i32.lt_u
1257+
(local.tee $2
1258+
(select
1259+
(i32.const 0)
1260+
(block (result i32)
1261+
(drop
1262+
(array.new $array-ref
1263+
(local.tee $1
1264+
(array.new_default $array-i32
1265+
(i32.const 0)
1266+
)
1267+
)
1268+
(i32.const 0)
1269+
)
1270+
)
1271+
(i32.const 0)
1272+
)
1273+
(i32.const 0)
1274+
)
1275+
)
1276+
(array.len
1277+
(local.tee $3
1278+
(local.get $1)
1279+
)
1280+
)
1281+
)
1282+
)
1283+
)
11991284
)
1285+

0 commit comments

Comments
 (0)