Stream: git-wasmtime

Topic: wasmtime / issue #6141 ISLE: simplify select/bitself when...


view this post on Zulip Wasmtime GitHub notifications bot (Apr 04 2023 at 18:12):

jameysharp commented on issue #6141:

I like this commit. (Note for other reviewers, this PR is based on #6140 so at the moment only ISLE: simplify select/bitself when both choices are the same is necessary to review here.)

I'd like to double-check with @cfallin that these new rules are correct, but I think they are. I don't think there's any subtlety to select or bitselect that would make this optimization invalid.

I think this particular optimization might even be legal for select_spectre_guard, although that definitely has subtlety and also I wouldn't expect such a rule to ever fire.

There's a typo in the commit message and PR description ("bitself"→"bitselect") but once #6140 lands and this PR is rebased, I think this is a fine change.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 04 2023 at 18:22):

cfallin commented on issue #6141:

Yep, this looks right to me -- no hidden subtleties!

I agree that the Spectre-guard variant would also be eligible (and would not lose any speculative safety), but I'm hesitant for us to do that, at least for now; IMHO a better approach for optimizations that touch select_spectre_guard is probably to (i) make changes that affect it only in concert with updates or optimizations to bounds-checking as a whole (i.e., when looking at generated code and improving it), when we're explicitly thinking about these issues; and (ii) do it in a separate PR, with a detailed argument around its speculative safety, probably also adding documentation on the topic.


Last updated: Dec 23 2024 at 12:05 UTC