diff options
author | Alon Zakai <azakai@google.com> | 2023-03-07 14:26:14 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-03-07 14:26:14 -0800 |
commit | 9dcdd47a255914bae36f146f0561ac3ac89aa14b (patch) | |
tree | 25985de778e92fb271a70b8f64e548d5ff677a13 | |
parent | 789b1422fd5c7356d0af7fa1a16a84132c839021 (diff) | |
download | binaryen-9dcdd47a255914bae36f146f0561ac3ac89aa14b.tar.gz binaryen-9dcdd47a255914bae36f146f0561ac3ac89aa14b.tar.bz2 binaryen-9dcdd47a255914bae36f146f0561ac3ac89aa14b.zip |
CodePushing: Pushing into an if may require non-nullable fixups (#5551)
This became an issue because the timeline was this:
* We added non-nullable locals support. At the time, obviously
CodePushing did not require any fixups for that, since it just moved
code forward in a single block (and not past any uses). So we marked
the pass as not needing such fixups.
* We added pushing of code into ifs. But moving code into an if
can affect non-nullable validation since it is based on block scoping.
So we need to remove the mark on the pass, which will make it check and
apply fixups as necessary. See the testcase for an example.
-rw-r--r-- | src/passes/CodePushing.cpp | 5 | ||||
-rw-r--r-- | test/lit/passes/code-pushing_into_if.wast | 105 |
2 files changed, 76 insertions, 34 deletions
diff --git a/src/passes/CodePushing.cpp b/src/passes/CodePushing.cpp index 2b45dda16..65eedba6b 100644 --- a/src/passes/CodePushing.cpp +++ b/src/passes/CodePushing.cpp @@ -455,11 +455,6 @@ private: struct CodePushing : public WalkerPass<PostWalker<CodePushing>> { bool isFunctionParallel() override { return true; } - // This pass moves code forward in blocks, but a local.set would not be moved - // after a local.get with the same index (effects prevent breaking things that - // way), so validation will be preserved. - bool requiresNonNullableLocalFixups() override { return false; } - std::unique_ptr<Pass> create() override { return std::make_unique<CodePushing>(); } diff --git a/test/lit/passes/code-pushing_into_if.wast b/test/lit/passes/code-pushing_into_if.wast index 06e64dab6..504d4776d 100644 --- a/test/lit/passes/code-pushing_into_if.wast +++ b/test/lit/passes/code-pushing_into_if.wast @@ -1,11 +1,11 @@ ;; NOTE: Assertions have been generated by update_lit_checks.py and should not be edited. -;; RUN: wasm-opt %s --code-pushing --enable-reference-types -S -o - | filecheck %s +;; RUN: wasm-opt %s --code-pushing -all -S -o - | filecheck %s (module ;; CHECK: (import "binaryen-intrinsics" "call.without.effects" (func $call.without.effects (param i32 funcref) (result i32))) (import "binaryen-intrinsics" "call.without.effects" (func $call.without.effects (param i32 funcref) (result i32))) - ;; CHECK: (func $if-nop (param $p i32) + ;; CHECK: (func $if-nop (type $i32_=>_none) (param $p i32) ;; CHECK-NEXT: (local $x i32) ;; CHECK-NEXT: (local.set $x ;; CHECK-NEXT: (i32.const 1) @@ -25,7 +25,7 @@ ) ) - ;; CHECK: (func $if-nop-nop (param $p i32) + ;; CHECK: (func $if-nop-nop (type $i32_=>_none) (param $p i32) ;; CHECK-NEXT: (local $x i32) ;; CHECK-NEXT: (local.set $x ;; CHECK-NEXT: (i32.const 1) @@ -46,7 +46,7 @@ ) ) - ;; CHECK: (func $if-use (param $p i32) + ;; CHECK: (func $if-use (type $i32_=>_none) (param $p i32) ;; CHECK-NEXT: (local $x i32) ;; CHECK-NEXT: (nop) ;; CHECK-NEXT: (if @@ -71,7 +71,7 @@ ) ) - ;; CHECK: (func $if-use-nop (param $p i32) + ;; CHECK: (func $if-use-nop (type $i32_=>_none) (param $p i32) ;; CHECK-NEXT: (local $x i32) ;; CHECK-NEXT: (nop) ;; CHECK-NEXT: (if @@ -97,7 +97,7 @@ ) ) - ;; CHECK: (func $if-else-use (param $p i32) + ;; CHECK: (func $if-else-use (type $i32_=>_none) (param $p i32) ;; CHECK-NEXT: (local $x i32) ;; CHECK-NEXT: (nop) ;; CHECK-NEXT: (if @@ -124,7 +124,7 @@ ) ) - ;; CHECK: (func $unpushed-interference (param $p i32) + ;; CHECK: (func $unpushed-interference (type $i32_=>_none) (param $p i32) ;; CHECK-NEXT: (local $x i32) ;; CHECK-NEXT: (local $y i32) ;; CHECK-NEXT: (local.set $x @@ -154,7 +154,7 @@ ) ) - ;; CHECK: (func $if-use-use (param $p i32) + ;; CHECK: (func $if-use-use (type $i32_=>_none) (param $p i32) ;; CHECK-NEXT: (local $x i32) ;; CHECK-NEXT: (local.set $x ;; CHECK-NEXT: (i32.const 1) @@ -180,7 +180,7 @@ ) ) - ;; CHECK: (func $if-use-after (param $p i32) + ;; CHECK: (func $if-use-after (type $i32_=>_none) (param $p i32) ;; CHECK-NEXT: (local $x i32) ;; CHECK-NEXT: (local.set $x ;; CHECK-NEXT: (i32.const 1) @@ -206,7 +206,7 @@ (drop (local.get $x)) ) - ;; CHECK: (func $if-use-after-nop (param $p i32) + ;; CHECK: (func $if-use-after-nop (type $i32_=>_none) (param $p i32) ;; CHECK-NEXT: (local $x i32) ;; CHECK-NEXT: (local.set $x ;; CHECK-NEXT: (i32.const 1) @@ -233,7 +233,7 @@ (drop (local.get $x)) ) - ;; CHECK: (func $if-else-use-after (param $p i32) + ;; CHECK: (func $if-else-use-after (type $i32_=>_none) (param $p i32) ;; CHECK-NEXT: (local $x i32) ;; CHECK-NEXT: (local.set $x ;; CHECK-NEXT: (i32.const 1) @@ -260,7 +260,7 @@ (drop (local.get $x)) ) - ;; CHECK: (func $if-use-after-unreachable (param $p i32) + ;; CHECK: (func $if-use-after-unreachable (type $i32_=>_none) (param $p i32) ;; CHECK-NEXT: (local $x i32) ;; CHECK-NEXT: (nop) ;; CHECK-NEXT: (if @@ -291,7 +291,7 @@ (drop (local.get $x)) ) - ;; CHECK: (func $if-use-after-unreachable-else (param $p i32) + ;; CHECK: (func $if-use-after-unreachable-else (type $i32_=>_none) (param $p i32) ;; CHECK-NEXT: (local $x i32) ;; CHECK-NEXT: (nop) ;; CHECK-NEXT: (if @@ -321,7 +321,7 @@ (drop (local.get $x)) ) - ;; CHECK: (func $optimize-many (param $p i32) + ;; CHECK: (func $optimize-many (type $i32_=>_none) (param $p i32) ;; CHECK-NEXT: (local $x i32) ;; CHECK-NEXT: (local $y i32) ;; CHECK-NEXT: (local $z i32) @@ -372,7 +372,7 @@ ) ) - ;; CHECK: (func $past-other (param $p i32) + ;; CHECK: (func $past-other (type $i32_=>_none) (param $p i32) ;; CHECK-NEXT: (local $x i32) ;; CHECK-NEXT: (local $t i32) ;; CHECK-NEXT: (nop) @@ -403,7 +403,7 @@ ) ) - ;; CHECK: (func $past-other-no (param $p i32) + ;; CHECK: (func $past-other-no (type $i32_=>_none) (param $p i32) ;; CHECK-NEXT: (local $x i32) ;; CHECK-NEXT: (local $t i32) ;; CHECK-NEXT: (local.set $x @@ -433,7 +433,7 @@ ) ) - ;; CHECK: (func $past-condition-no (param $p i32) + ;; CHECK: (func $past-condition-no (type $i32_=>_none) (param $p i32) ;; CHECK-NEXT: (local $x i32) ;; CHECK-NEXT: (local $t i32) ;; CHECK-NEXT: (local.set $x @@ -459,7 +459,7 @@ ) ) - ;; CHECK: (func $past-condition-no-2 + ;; CHECK: (func $past-condition-no-2 (type $none_=>_none) ;; CHECK-NEXT: (local $x i32) ;; CHECK-NEXT: (local $t i32) ;; CHECK-NEXT: (local.set $x @@ -483,7 +483,7 @@ ) ) - ;; CHECK: (func $past-condition-no-3 (param $p i32) + ;; CHECK: (func $past-condition-no-3 (type $i32_=>_none) (param $p i32) ;; CHECK-NEXT: (local $x i32) ;; CHECK-NEXT: (local $t i32) ;; CHECK-NEXT: (local.set $x @@ -509,7 +509,7 @@ ) ) - ;; CHECK: (func $if-condition-return (param $p i32) + ;; CHECK: (func $if-condition-return (type $i32_=>_none) (param $p i32) ;; CHECK-NEXT: (local $x i32) ;; CHECK-NEXT: (nop) ;; CHECK-NEXT: (if @@ -541,7 +541,7 @@ ) ) - ;; CHECK: (func $if-condition-break-used (param $p i32) + ;; CHECK: (func $if-condition-break-used (type $i32_=>_none) (param $p i32) ;; CHECK-NEXT: (local $x i32) ;; CHECK-NEXT: (local.set $x ;; CHECK-NEXT: (i32.const 1) @@ -580,7 +580,7 @@ (drop (local.get $x)) ) - ;; CHECK: (func $one-push-prevents-another (param $p i32) + ;; CHECK: (func $one-push-prevents-another (type $i32_=>_none) (param $p i32) ;; CHECK-NEXT: (local $x i32) ;; CHECK-NEXT: (local $y i32) ;; CHECK-NEXT: (local.set $x @@ -616,7 +616,7 @@ ) ) - ;; CHECK: (func $one-push-prevents-another-flipped (param $p i32) + ;; CHECK: (func $one-push-prevents-another-flipped (type $i32_=>_none) (param $p i32) ;; CHECK-NEXT: (local $x i32) ;; CHECK-NEXT: (local $y i32) ;; CHECK-NEXT: (local.set $x @@ -652,7 +652,7 @@ ) ) - ;; CHECK: (func $sink-call (param $p i32) (result i32) + ;; CHECK: (func $sink-call (type $i32_=>_i32) (param $p i32) (result i32) ;; CHECK-NEXT: (local $temp i32) ;; CHECK-NEXT: (nop) ;; CHECK-NEXT: (if @@ -691,7 +691,7 @@ (i32.const 0) ) - ;; CHECK: (func $no-sink-call (param $p i32) (result i32) + ;; CHECK: (func $no-sink-call (type $i32_=>_i32) (param $p i32) (result i32) ;; CHECK-NEXT: (local $temp i32) ;; CHECK-NEXT: (local.set $temp ;; CHECK-NEXT: (call $call.without.effects @@ -728,7 +728,7 @@ (local.get $temp) ;; this line changed. ) - ;; CHECK: (func $no-sink-call-2 (param $p i32) (result i32) + ;; CHECK: (func $no-sink-call-2 (type $i32_=>_i32) (param $p i32) (result i32) ;; CHECK-NEXT: (local $temp i32) ;; CHECK-NEXT: (local.set $temp ;; CHECK-NEXT: (call $call.without.effects @@ -766,7 +766,7 @@ (local.get $temp) ) - ;; CHECK: (func $no-sink-call-3 (param $p i32) (result i32) + ;; CHECK: (func $no-sink-call-3 (type $i32_=>_i32) (param $p i32) (result i32) ;; CHECK-NEXT: (local $temp i32) ;; CHECK-NEXT: (local.set $temp ;; CHECK-NEXT: (call $call.without.effects @@ -812,7 +812,7 @@ (i32.const 0) ;; this line was added. ) - ;; CHECK: (func $sink-call-3 (param $p i32) (result i32) + ;; CHECK: (func $sink-call-3 (type $i32_=>_i32) (param $p i32) (result i32) ;; CHECK-NEXT: (local $temp i32) ;; CHECK-NEXT: (nop) ;; CHECK-NEXT: (if @@ -862,7 +862,7 @@ (i32.const 0) ) - ;; CHECK: (func $no-sink-call-sub (param $p i32) (result i32) + ;; CHECK: (func $no-sink-call-sub (type $i32_=>_i32) (param $p i32) (result i32) ;; CHECK-NEXT: (local $temp i32) ;; CHECK-NEXT: (local $other i32) ;; CHECK-NEXT: (local.set $temp @@ -903,4 +903,51 @@ ) (i32.const 0) ) + + ;; CHECK: (func $ref-into-if (type $ref|any|_=>_none) (param $0 (ref any)) + ;; CHECK-NEXT: (local $1 anyref) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: (block $label$3 + ;; CHECK-NEXT: (local.set $1 + ;; CHECK-NEXT: (local.get $0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (ref.as_non_null + ;; CHECK-NEXT: (local.get $1) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (ref.as_non_null + ;; CHECK-NEXT: (local.get $1) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $ref-into-if (param $0 (ref any)) + (local $1 (ref any)) + ;; This can be pushed into the reachable if arm. After doing so, however, + ;; the local $1 no longer has a single set that dominates it in the sense of + ;; the wasm validation rules for non-nullable locals, so the local must be + ;; turned into a nullable one. + (local.set $1 + (local.get $0) + ) + (if + (i32.const 1) + (unreachable) + (block $label$3 + (drop + (local.get $1) + ) + ) + ) + (drop + (local.get $1) + ) + ) + ) |