[BranchHints] Fuzz branch hints #7704
Conversation
|
This should now be ready for review: feedback so far addressed, and necessary fixes so this actually works, namely:
|
scripts/fuzz_opt.py
Outdated
| # * HeapStoreOptimization moves struct.sets closer to struct.news. | ||
| '--skip-pass=heap-store-optimization', |
There was a problem hiding this comment.
Can you elaborate in the comment on how this affects branch hints?
There was a problem hiding this comment.
Done. (It's the same reason as LICM, above.)
| ;; CHECK-NEXT: ) | ||
| ;; CHECK-NEXT: (f32.const 0) | ||
| ;; CHECK-NEXT: ) | ||
| (func $different (param $x i32) (param $y i32) (result f32) |
There was a problem hiding this comment.
It might be interesting to have the reversed test as well.
| ;; NO_FO-NEXT: ) | ||
| ;; NO_FO-NEXT: ) | ||
| (func $yes-fold (param $x i32) (param $y i32) | ||
| ;; Now the hints match, so we definitely fold (without the flag). |
There was a problem hiding this comment.
Would it be worth having the flag only disable folding when the metadata is different?
There was a problem hiding this comment.
I guess that would show the fuzzer finds no issues when folding identical metadata, but I'm not sure it's worth the complexity in the code to be that precise. We know that is safe to do, so it would only be checking that we wrote that condition on the flag properly.
|
After 100K iterations I was fairly confident, but the fuzzer found that optimize-instructions reorders, so we need to prevent that too, with the flag. |
| // As neverFold, but for reordering code. If we move a branch hint around code | ||
| // that might trap, and the trap happens later, the branch hint might start to | ||
| // execute, and it could be wrong. | ||
| bool neverReorder; |
There was a problem hiding this comment.
Can we merge these two bools just like we merge the flag? They're always the same, right?
There was a problem hiding this comment.
Yeah, I just thought it reads a little simpler in each place in the code, "if (!neverFold) fold()" as opposed to "if (!neverFoldOrSomethingElse) fold()" - ?
|
Fuzzer found another (trivial) source of issues in RemoveUnusedBrs (edit: just for fuzzing) |
|
135K iterations of this without any issues. There might be findings in the future, but given we'll be fuzzing this at 0.1 the speed I have been locally, I don't think it will be disruptive. |
2 participants
Add two helper passes, one to delete specific branch hints by their
instrumentation ID (as added by InstrumentBranchHints), and one to
remove all instrumentation. The new fuzzer then
The idea is that once we have a wasm with only correct hints, the
optimizer is allowed to remove some (e.g. in DCE), but it should
never emit an invalid branch hint (e.g. by forgetting to flip a hint
when it flips an if).