[BranchHinting] Do not reorder traps in OptimizeInstructions when neverReorder is set (#9096)

In the neverReorder testing mode, no code can be reordered, as reordering it
past branch hints leads to fuzzer errors.

Author: kripken

src/passes/OptimizeInstructions.cpp

@@ -1611,6 +1611,17 @@ struct OptimizeInstructions
   // ref.as_non_null then the struct.set will still trap, of course, but that
   // will only happen *after* the call, which is wrong.
   void skipNonNullCast(Expression*& input, Expression* parent) {
+    // If we must never reorder code, then we cannot remove a non-null cast.
+    // Such removals are valid because we move the trap later (see the
+    // struct.set in the example above: we can remove the ref.as_non_null
+    // because the set will trap anyhow, so we are pushing the trap onward; in
+    // the example above we have a call we can't move past, and in neverReorder
+    // mode we need to care about things like branch hints, which do not have
+    // effects, hence the need for the special neverReorder flag).
+    if (neverReorder) {
+      return;
+    }
+
     // Check the other children for the ordering problem only if we find a
     // possible optimization, to avoid wasted work.
     bool checkedSiblings = false;

test/lit/passes/optimize-instructions_branch-hints-fold.wast

@@ -12,7 +12,11 @@
 ;; RUN:   | filecheck %s --check-prefix=NO_FO
 
 (module
- ;; CHECK:      (func $conditionals (type $1) (param $x i32) (result i32)
+ ;; CHECK:      (type $struct (struct (field (mut i32))))
+ ;; NO_FO:      (type $struct (struct (field (mut i32))))
+ (type $struct (struct (mut i32)))
+
+ ;; CHECK:      (func $conditionals (type $2) (param $x i32) (result i32)
  ;; CHECK-NEXT:  (@metadata.code.branch_hint "\01")
  ;; CHECK-NEXT:  (if (result i32)
  ;; CHECK-NEXT:   (local.get $x)
@@ -24,7 +28,7 @@
  ;; CHECK-NEXT:   )
  ;; CHECK-NEXT:  )
  ;; CHECK-NEXT: )
- ;; NO_FO:      (func $conditionals (type $1) (param $x i32) (result i32)
+ ;; NO_FO:      (func $conditionals (type $2) (param $x i32) (result i32)
  ;; NO_FO-NEXT:  (@metadata.code.branch_hint "\01")
  ;; NO_FO-NEXT:  (if (result i32)
  ;; NO_FO-NEXT:   (local.get $x)
@@ -52,7 +56,7 @@
   )
  )
 
- ;; CHECK:      (func $still-fold (type $0) (param $x i32) (param $y i32)
+ ;; CHECK:      (func $still-fold (type $1) (param $x i32) (param $y i32)
  ;; CHECK-NEXT:  (@metadata.code.branch_hint "\00")
  ;; CHECK-NEXT:  (if
  ;; CHECK-NEXT:   (local.get $y)
@@ -61,7 +65,7 @@
  ;; CHECK-NEXT:   )
  ;; CHECK-NEXT:  )
  ;; CHECK-NEXT: )
- ;; NO_FO:      (func $still-fold (type $0) (param $x i32) (param $y i32)
+ ;; NO_FO:      (func $still-fold (type $1) (param $x i32) (param $y i32)
  ;; NO_FO-NEXT:  (if
  ;; NO_FO-NEXT:   (local.get $x)
  ;; NO_FO-NEXT:   (then
@@ -110,7 +114,7 @@
   )
  )
 
- ;; CHECK:      (func $yes-fold (type $0) (param $x i32) (param $y i32)
+ ;; CHECK:      (func $yes-fold (type $1) (param $x i32) (param $y i32)
  ;; CHECK-NEXT:  (@metadata.code.branch_hint "\01")
  ;; CHECK-NEXT:  (if
  ;; CHECK-NEXT:   (local.get $y)
@@ -119,7 +123,7 @@
  ;; CHECK-NEXT:   )
  ;; CHECK-NEXT:  )
  ;; CHECK-NEXT: )
- ;; NO_FO:      (func $yes-fold (type $0) (param $x i32) (param $y i32)
+ ;; NO_FO:      (func $yes-fold (type $1) (param $x i32) (param $y i32)
  ;; NO_FO-NEXT:  (if
  ;; NO_FO-NEXT:   (local.get $x)
  ;; NO_FO-NEXT:   (then
@@ -167,7 +171,7 @@
   )
  )
 
- ;; CHECK:      (func $always-fold-select (type $2) (param $x i32) (param $y i32) (result i32)
+ ;; CHECK:      (func $always-fold-select (type $3) (param $x i32) (param $y i32) (result i32)
  ;; CHECK-NEXT:  (drop
  ;; CHECK-NEXT:   (local.get $y)
  ;; CHECK-NEXT:  )
@@ -182,7 +186,7 @@
  ;; CHECK-NEXT:   )
  ;; CHECK-NEXT:  )
  ;; CHECK-NEXT: )
- ;; NO_FO:      (func $always-fold-select (type $2) (param $x i32) (param $y i32) (result i32)
+ ;; NO_FO:      (func $always-fold-select (type $3) (param $x i32) (param $y i32) (result i32)
  ;; NO_FO-NEXT:  (local $2 i32)
  ;; NO_FO-NEXT:  (local.set $2
  ;; NO_FO-NEXT:   (@metadata.code.branch_hint "\00")
@@ -230,7 +234,7 @@
   )
  )
 
- ;; CHECK:      (func $ordering (type $3) (param $x i32)
+ ;; CHECK:      (func $ordering (type $4) (param $x i32)
  ;; CHECK-NEXT:  (drop
  ;; CHECK-NEXT:   (i32.add
  ;; CHECK-NEXT:    (local.get $x)
@@ -244,7 +248,7 @@
  ;; CHECK-NEXT:   )
  ;; CHECK-NEXT:  )
  ;; CHECK-NEXT: )
- ;; NO_FO:      (func $ordering (type $3) (param $x i32)
+ ;; NO_FO:      (func $ordering (type $4) (param $x i32)
  ;; NO_FO-NEXT:  (drop
  ;; NO_FO-NEXT:   (i32.add
  ;; NO_FO-NEXT:    (local.get $x)
@@ -275,4 +279,32 @@
    )
   )
  )
+
+ ;; CHECK:      (func $struct-set (type $5) (param $ref (ref null $struct)) (param $val i32)
+ ;; CHECK-NEXT:  (struct.set $struct 0
+ ;; CHECK-NEXT:   (local.get $ref)
+ ;; CHECK-NEXT:   (local.get $val)
+ ;; CHECK-NEXT:  )
+ ;; CHECK-NEXT: )
+ ;; NO_FO:      (func $struct-set (type $5) (param $ref (ref null $struct)) (param $val i32)
+ ;; NO_FO-NEXT:  (struct.set $struct 0
+ ;; NO_FO-NEXT:   (ref.as_non_null
+ ;; NO_FO-NEXT:    (local.get $ref)
+ ;; NO_FO-NEXT:   )
+ ;; NO_FO-NEXT:   (local.get $val)
+ ;; NO_FO-NEXT:  )
+ ;; NO_FO-NEXT: )
+ (func $struct-set (param $ref (ref null $struct)) (param $val i32)
+  ;; Normally we can skip the ref.as_non_null because struct.set will trap on
+  ;; null anyway. But when never-fold-or-reorder is passed, we must not move the
+  ;; trap after subsequent children (which might reorder across branch hints, if
+  ;; the struct.set's value had a branch hint).
+  (struct.set $struct 0
+   (ref.as_non_null
+    (local.get $ref)
+   )
+   (local.get $val)
+  )
+ )
 )
+