Implement branch hinting proposal support #12483
Conversation
This commit adds support for the WebAssembly branch hinting proposal by parsing the `metadata.code.branch_hint` custom section and using the hints to mark cold blocks during Cranelift code generation. Implementation details: - Parse branch hints in `ModuleTranslation` from the custom section using wasmparser's `KnownCustom::BranchHints` reader - Store hints as a map from function index to (offset, taken) pairs - Add `get_branch_hint()` helper in `FuncEnvironment` to look up hints by converting absolute byte offsets to function-relative offsets - Apply hints in `translate_br_if()`: when a branch is marked unlikely (`taken=false`), mark the branch target as cold; when marked likely (`taken=true`), mark the fallthrough block as cold Branch hints are always parsed and applied when present in a module; no configuration flag is required. A disassembly test is included to verify cold block annotations, but official WebAssembly spec tests for this proposal are not yet available. Closes bytecodealliance#9463 Co-Authored-By: Claude Opus 4.5 <[email protected]>
github-actions
bot
added
the
wasmtime:docs
crates/cranelift/src/func_environ.rs
Outdated
| self.translation | ||
| .branch_hints | ||
| .get(&func_index.as_u32()) | ||
| .and_then(|hints| { | ||
| hints | ||
| .iter() | ||
| .find(|(o, _)| *o as usize == relative_offset) | ||
| .map(|(_, taken)| *taken) | ||
| }) |
There was a problem hiding this comment.
This loop I think might be good to optimize in terms of translation will only lookup branch hints (I think?) in increasing order of offsets. That means that currently this code is an .peekable() iterator over the raw wasm itself. Looking up via get_branch_hint would advance the iterator if it's at the matching position.
Regardless I think it'll be needed to remove the quadratic behavior here, and I think ideally it'd be via an iterator-like approach to avoid the logarithmic nature of a binary search.
There was a problem hiding this comment.
Ah, you are right.
How about bc62088 for O(n) complexity?
There was a problem hiding this comment.
What Alex is getting at, I think, is that the spec states that the hints are given in PC order. So why not traverse hints in order, taking them when they match as we visit increasing PC offsets? We should be able to that in O(n) time with O(1) space overhead, i.e., not building a HashMap at all.
There was a problem hiding this comment.
Question: should I add a knob to enable/disable this feature? Currently, this is automatically turned on. I'm not sure I should add an option for it.
Yes, I think we should have a config knob for this.
And to follow our rules by the letter, we we shouldn't enable it by default until it is fuzzed (and has been fuzzed for a couple weeks). This would most likely involve adding support to wasm-smith for emitting branch hints.
Change branch hints storage from Vec<(u32, bool)> to HashMap<u32, bool> for O(1) lookup instead of O(n) linear search per lookup. The previous implementation had O(n²) complexity when processing many branch hints since each lookup scanned the entire vector. The HashMap approach also handles untrusted input robustly without assumptions about hint ordering in the custom section. Co-Authored-By: Claude Opus 4.5 <[email protected]>
4 participants
This PR adds support for the WebAssembly branch hinting proposal by parsing the
metadata.code.branch_hintcustom section and using the hints to mark cold blocks during Cranelift code generation.Implementation details:
ModuleTranslationfrom the custom section using wasmparser'sKnownCustom::BranchHintsreaderget_branch_hint()helper inFuncEnvironmentto look up hints by converting absolute byte offsets to function-relative offsetstranslate_br_if(): when a branch is marked unlikely (taken=false), mark the branch target as cold; when marked likely (taken=true), mark the fallthrough block as coldBranch hints are always parsed and applied when present in a module; no configuration flag is required.
A disassembly test is included to verify cold block annotations, but official WebAssembly spec tests for this proposal are not yet available.
Closes #9463
Question: should I add a knob to enable/disable this feature? Currently, this is automatically turned on. I'm not sure I should add an option for it.