Skip to content

Commit c450372

Browse files
committed
Refine branch hinting: lazy decode and review fixes
- Decode the branch-hint section lazily via per-function readers instead of eagerly decoding and heap-allocating every hint - Discard malformed sections explicitly; keep first of duplicate entries - take_branch_hint yields BranchHint; init hints in FuncEnvironment::new - Move the runtime test to misc_testsuite/*.wast; add a malformed-section test - Add a fuzzing knob; tidy docs and comments Assisted-by: Claude Code:claude-opus-4-7
1 parent d6e78df commit c450372

13 files changed

Lines changed: 182 additions & 176 deletions

File tree

crates/cranelift/src/compiler.rs

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,20 @@ impl wasmtime_environ::Compiler for Compiler {
262262
context.func.collect_debug_info();
263263
}
264264

265-
let mut func_env = FuncEnvironment::new(self, translation, types, wasm_func_ty, key);
265+
// Branch hints are keyed by function-body-relative offset, so the body's
266+
// module-relative start is needed to convert source locations later.
267+
let FunctionBodyData { validator, body } = input;
268+
let func_body_offset = body.get_binary_reader().original_position();
269+
270+
let mut func_env = FuncEnvironment::new(
271+
self,
272+
translation,
273+
types,
274+
wasm_func_ty,
275+
key,
276+
func_index,
277+
func_body_offset,
278+
);
266279

267280
// The `stack_limit` global value below is the implementation of stack
268281
// overflow checks in Wasmtime.
@@ -318,19 +331,6 @@ impl wasmtime_environ::Compiler for Compiler {
318331
func_env.stack_limit_at_function_entry = Some(stack_limit);
319332
}
320333
}
321-
let FunctionBodyData { validator, body } = input;
322-
323-
// Branch hints are keyed by function-body-relative offset, so record the
324-
// body's module-relative start to convert source locations later.
325-
func_env.set_branch_hints(
326-
translation
327-
.branch_hints
328-
.get(&func_index)
329-
.map(|hints| &hints[..])
330-
.unwrap_or(&[]),
331-
body.get_binary_reader().original_position(),
332-
);
333-
334334
let mut validator =
335335
validator.into_validator(mem::take(&mut compiler.cx.validator_allocations));
336336
compiler.cx.func_translator.translate_body(

crates/cranelift/src/func_environ.rs

Lines changed: 53 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,12 @@ use cranelift_frontend::Variable;
2626
use cranelift_frontend::{FuncInstBuilder, FunctionBuilder};
2727
use smallvec::{SmallVec, smallvec};
2828
use std::mem;
29-
use wasmparser::{FuncValidator, Operator, WasmFeatures, WasmModuleResources};
29+
use wasmparser::{
30+
BranchHint, FuncValidator, Operator, SectionLimitedIntoIter, WasmFeatures, WasmModuleResources,
31+
};
3032
use wasmtime_core::math::f64_cvt_to_int_bounds;
3133
use wasmtime_environ::{
32-
BranchHint, BuiltinFunctionIndex, ComponentPC, DataIndex, DefinedFuncIndex, ElemIndex,
34+
BuiltinFunctionIndex, ComponentPC, DataIndex, DefinedFuncIndex, ElemIndex,
3335
EngineOrModuleTypeIndex, FrameStateSlotBuilder, FrameValType, FuncIndex, FuncKey,
3436
GlobalConstValue, GlobalIndex, IndexType, Memory, MemoryIndex, MemoryTunables, Module,
3537
ModuleInternedTypeIndex, ModuleTranslation, ModuleTypesBuilder, PtrSize, Table, TableIndex,
@@ -233,11 +235,12 @@ pub struct FuncEnvironment<'module_environment> {
233235
/// to e.g. record the return-address of a callsite for debuginfo.
234236
pub(crate) next_srcloc: ir::SourceLoc,
235237

236-
/// Branch hints for the current function, consumed in program-counter order
237-
/// by `take_branch_hint`.
238-
branch_hints: &'module_environment [BranchHint],
239-
/// Forward cursor into `branch_hints`.
240-
branch_hint_cursor: usize,
238+
/// Lazily-decoded branch hints for the current function, in ascending
239+
/// `func_offset` order (as the proposal requires). `None` once exhausted or
240+
/// when the function carries no hints.
241+
branch_hints: Option<SectionLimitedIntoIter<'module_environment, BranchHint>>,
242+
/// One-item lookahead into `branch_hints`, consumed by `take_branch_hint`.
243+
peeked_hint: Option<BranchHint>,
241244
/// Module-relative byte offset of the current function body's start.
242245
func_body_offset: usize,
243246
}
@@ -249,10 +252,19 @@ impl<'module_environment> FuncEnvironment<'module_environment> {
249252
types: &'module_environment ModuleTypesBuilder,
250253
wasm_func_ty: &'module_environment WasmFuncType,
251254
key: FuncKey,
255+
func_index: FuncIndex,
256+
func_body_offset: usize,
252257
) -> Self {
253258
let tunables = compiler.tunables();
254259
let builtin_functions = BuiltinFunctions::new(compiler);
255260

261+
// Resolve the lazy branch-hint decoder for this function, if any.
262+
// `func_body_offset` lets `take_branch_hint` convert source locations to
263+
// the function-body-relative offsets the hints use.
264+
let branch_hints = translation
265+
.branch_hints(func_index)
266+
.map(|reader| reader.into_iter());
267+
256268
// This isn't used during translation, so squash the warning about this
257269
// being unused from the compiler.
258270
let _ = BuiltinFunctions::raise;
@@ -303,46 +315,48 @@ impl<'module_environment> FuncEnvironment<'module_environment> {
303315
next_srcloc: ir::SourceLoc::default(),
304316
wasm_module_offset: translation.wasm_module_offset,
305317

306-
branch_hints: &[],
307-
branch_hint_cursor: 0,
308-
func_body_offset: 0,
318+
branch_hints,
319+
peeked_hint: None,
320+
func_body_offset,
309321
}
310322
}
311323

312-
/// Set the branch hints and the module-relative start offset for the
313-
/// function about to be translated. Hints are expected in ascending
314-
/// `func_offset` order (as the proposal requires); `take_branch_hint`
315-
/// simply skips any that are out of order.
316-
pub(crate) fn set_branch_hints(
317-
&mut self,
318-
hints: &'module_environment [BranchHint],
319-
func_body_offset: usize,
320-
) {
321-
self.branch_hints = hints;
322-
self.branch_hint_cursor = 0;
323-
self.func_body_offset = func_body_offset;
324-
}
325-
326324
/// Consume the branch hint for the instruction at module-relative `offset`
327-
/// (i.e. `builder.srcloc().bits()`), if any; `Some(true)` means likely
328-
/// taken. The cursor only advances, making this O(n) over a function body.
329-
pub(crate) fn take_branch_hint(&mut self, offset: usize) -> Option<bool> {
330-
if self.branch_hints.is_empty() {
325+
/// (i.e. `builder.srcloc().bits()`), if any. The lazy decoder only moves
326+
/// forward, making this O(n) over a function body.
327+
pub(crate) fn take_branch_hint(&mut self, offset: usize) -> Option<BranchHint> {
328+
// Fast path for the common case of a function with no hints (always so
329+
// when the proposal is disabled): this is called for every `if`/`br_if`.
330+
if self.branch_hints.is_none() {
331331
return None;
332332
}
333333
let rel = u32::try_from(offset.checked_sub(self.func_body_offset)?).ok()?;
334-
// Skip hints that don't line up with this (or a later) branch.
335-
while matches!(
336-
self.branch_hints.get(self.branch_hint_cursor),
337-
Some(h) if h.func_offset < rel,
338-
) {
339-
self.branch_hint_cursor += 1;
334+
loop {
335+
// Refill the one-item lookahead from the lazy decoder.
336+
if self.peeked_hint.is_none() {
337+
self.peeked_hint = self.next_branch_hint();
338+
}
339+
let hint = self.peeked_hint?;
340+
if hint.func_offset < rel {
341+
// Hint precedes this branch (or never lined up); drop it.
342+
self.peeked_hint = None;
343+
continue;
344+
}
345+
if hint.func_offset == rel {
346+
self.peeked_hint = None;
347+
return Some(hint);
348+
}
349+
// The next hint is for a later offset; nothing for this branch.
350+
return None;
340351
}
341-
let hint = self.branch_hints.get(self.branch_hint_cursor)?;
342-
(hint.func_offset == rel).then(|| {
343-
self.branch_hint_cursor += 1;
344-
hint.taken
345-
})
352+
}
353+
354+
/// Decode the next hint for the current function, if any.
355+
fn next_branch_hint(&mut self) -> Option<BranchHint> {
356+
// These bytes were already validated when the section was decoded into
357+
// per-function readers, so this re-decode cannot fail; defensively treat
358+
// an unexpected error as the end of the hints.
359+
self.branch_hints.as_mut()?.next()?.ok()
346360
}
347361

348362
pub(crate) fn pointer_type(&self) -> ir::Type {

crates/cranelift/src/translate/code_translator.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -335,13 +335,13 @@ pub fn translate_operator(
335335

336336
// Mark the unlikely successor cold per the branch hint. A likely
337337
// condition makes the else block cold; when it is allocated lazily
338-
// (`NoElse`) defer that to `Operator::Else` via `cold_else`.
339-
let cold_else = match branch_hint {
340-
Some(false) => {
338+
// (`NoElse`) defer that to `Operator::Else` via `else_is_cold`.
339+
let else_is_cold = match branch_hint {
340+
Some(hint) if !hint.taken => {
341341
builder.set_cold_block(next_block);
342342
false
343343
}
344-
Some(true) => match &else_data {
344+
Some(_) => match &else_data {
345345
ElseData::WithElse { else_block } => {
346346
builder.set_cold_block(*else_block);
347347
false
@@ -366,7 +366,7 @@ pub fn translate_operator(
366366
params.len(),
367367
results.len(),
368368
*blockty,
369-
cold_else,
369+
else_is_cold,
370370
);
371371
}
372372
Operator::Else => {
@@ -4018,9 +4018,9 @@ fn translate_br_if(
40184018
let (br_destination, inputs) = translate_br_if_args(relative_depth, env);
40194019
let next_block = builder.create_block();
40204020

4021-
if let Some(taken) = branch_hint {
4021+
if let Some(hint) = branch_hint {
40224022
// Likely taken => the fallthrough is cold, else the branch target is.
4023-
builder.set_cold_block(if taken { next_block } else { br_destination });
4023+
builder.set_cold_block(if hint.taken { next_block } else { br_destination });
40244024
}
40254025

40264026
canonicalise_brif(builder, val, br_destination, inputs, next_block, &[]);

crates/environ/src/compile/module_environ.rs

Lines changed: 33 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -123,21 +123,19 @@ pub struct ModuleTranslation<'data> {
123123
/// validation process.
124124
types: Option<Types>,
125125

126-
/// Branch hints parsed from the `metadata.code.branch_hint` custom section,
126+
/// Per-function readers into the `metadata.code.branch_hint` custom section,
127127
/// keyed by module-level function index. Only populated when
128-
/// [`Tunables::branch_hinting`] is enabled.
129-
pub branch_hints: HashMap<FuncIndex, Box<[BranchHint]>>,
128+
/// [`Tunables::branch_hinting`] is enabled. The hints are decoded lazily
129+
/// during compilation, so this holds the section's per-function sub-readers
130+
/// rather than fully-decoded hints; access them via
131+
/// [`ModuleTranslation::branch_hints`].
132+
branch_hints: HashMap<FuncIndex, BranchHintReader<'data>>,
130133
}
131134

132-
/// A single branch hint from the `metadata.code.branch_hint` custom section
135+
/// Lazy decoder over the branch hints attached to a single function in the
136+
/// `metadata.code.branch_hint` custom section
133137
/// ([branch-hinting proposal](https://github.com/WebAssembly/branch-hinting)).
134-
#[derive(Debug, Copy, Clone)]
135-
pub struct BranchHint {
136-
/// Byte offset of the hinted `br_if`/`if` from the start of the function body.
137-
pub func_offset: u32,
138-
/// Whether the branch's condition is hinted to be true.
139-
pub taken: bool,
140-
}
138+
pub type BranchHintReader<'a> = wasmparser::SectionLimited<'a, wasmparser::BranchHint>;
141139

142140
impl<'data> ModuleTranslation<'data> {
143141
/// Create a new translation for the module with the given index.
@@ -164,6 +162,13 @@ impl<'data> ModuleTranslation<'data> {
164162
}
165163
}
166164

165+
/// Returns a lazy decoder over the branch hints for `func`, if the
166+
/// `metadata.code.branch_hint` section attached any. Hints are decoded on
167+
/// demand during compilation rather than eagerly during parsing.
168+
pub fn branch_hints(&self, func: FuncIndex) -> Option<BranchHintReader<'data>> {
169+
self.branch_hints.get(&func).cloned()
170+
}
171+
167172
/// Returns a reference to the type information of the current module.
168173
pub fn get_types(&self) -> &Types {
169174
self.types
@@ -774,30 +779,23 @@ and for re-adding support for interface types you can see this issue:
774779
}
775780
}
776781
KnownCustom::BranchHints(reader) if self.tunables.branch_hinting => {
777-
// Compilation relies on the proposal's guarantee that hints are
778-
// in ascending `func_offset` order; we trust it rather than
779-
// re-sort (validating malformed sections is not yet done). Hints
780-
// are advisory, so skip entries that fail to parse.
781-
for func in reader.into_iter().flatten() {
782-
let hints = func
783-
.hints
784-
.into_iter()
785-
.flatten()
786-
.map(|h| BranchHint {
787-
func_offset: h.func_offset,
788-
taken: h.taken,
789-
})
790-
.collect::<Box<[_]>>();
791-
if !hints.is_empty() {
792-
// A well-formed section lists each function at most once;
793-
// if a malformed one repeats a function, keep the first
794-
// entry deterministically rather than silently
795-
// overwriting it.
796-
self.result
797-
.branch_hints
798-
.entry(FuncIndex::from_u32(func.func))
799-
.or_insert(hints);
800-
}
782+
// Branch hints are advisory and this section is never validated;
783+
// it is decoded lazily during compilation, so record only the
784+
// per-function sub-readers here. Discard the whole section if any
785+
// entry is malformed rather than applying it partially.
786+
let mut hints = HashMap::new();
787+
let result: wasmparser::Result<()> = reader.into_iter().try_for_each(|func| {
788+
let func = func?;
789+
// A well-formed section lists each function at most once; keep
790+
// the first entry deterministically if it repeats.
791+
hints
792+
.entry(FuncIndex::from_u32(func.func))
793+
.or_insert(func.hints);
794+
Ok(())
795+
});
796+
match result {
797+
Ok(()) => self.result.branch_hints = hints,
798+
Err(e) => log::warn!("failed to parse branch-hint section {e:?}"),
801799
}
802800
}
803801
_ => {

crates/fuzzing/src/generators/config.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ impl Config {
138138
tail_call,
139139
extended_const,
140140
wide_arithmetic,
141+
branch_hinting,
141142
component_model_async,
142143
component_model_more_async_builtins,
143144
component_model_async_stackful,
@@ -177,6 +178,7 @@ impl Config {
177178
component_model_fixed_length_lists.unwrap_or(false);
178179
self.module_config.component_model_implements = component_model_implements.unwrap_or(false);
179180
self.module_config.stack_switching = stack_switching.unwrap_or(false);
181+
self.wasmtime.branch_hinting = branch_hinting.unwrap_or(false);
180182

181183
// Enable/disable proposals that wasm-smith has knobs for which will be
182184
// read when creating `wasmtime::Config`.
@@ -346,6 +348,7 @@ impl Config {
346348
cfg.wasm.shared_everything_threads =
347349
Some(self.module_config.config.shared_everything_threads_enabled);
348350
cfg.wasm.wide_arithmetic = Some(self.module_config.config.wide_arithmetic_enabled);
351+
cfg.wasm.branch_hinting = Some(self.wasmtime.branch_hinting);
349352
cfg.wasm.exceptions = Some(self.module_config.config.exceptions_enabled);
350353
cfg.wasm.stack_switching = Some(self.module_config.stack_switching);
351354
cfg.wasm.shared_memory = Some(self.module_config.shared_memory);
@@ -608,6 +611,10 @@ pub struct WasmtimeConfig {
608611
table_lazy_init: bool,
609612
metadata_for_internal_asserts: bool,
610613
metadata_for_gc_heap_corruption: bool,
614+
/// Whether the branch-hinting proposal is enabled. wasm-smith does not emit
615+
/// `metadata.code.branch_hint` sections, so for generated modules this only
616+
/// toggles the (otherwise no-op) parsing path.
617+
branch_hinting: bool,
611618

612619
/// Configuration for whether wasm is invoked in an async fashion and how
613620
/// it's cooperatively time-sliced.

crates/test-util/src/wasmtime_wast.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ pub fn apply_test_config(config: &mut Config, test_config: &wast::TestConfig) {
3939
tail_call,
4040
extended_const,
4141
wide_arithmetic,
42+
branch_hinting,
4243
component_model_async,
4344
component_model_more_async_builtins,
4445
component_model_async_stackful,
@@ -72,6 +73,7 @@ pub fn apply_test_config(config: &mut Config, test_config: &wast::TestConfig) {
7273
let tail_call = tail_call.unwrap_or(false);
7374
let extended_const = extended_const.unwrap_or(false);
7475
let wide_arithmetic = wide_arithmetic.unwrap_or(false);
76+
let branch_hinting = branch_hinting.unwrap_or(false);
7577
let component_model_async = component_model_async.unwrap_or(false);
7678
let component_model_more_async_builtins = component_model_more_async_builtins.unwrap_or(false);
7779
let component_model_async_stackful = component_model_async_stackful.unwrap_or(false);
@@ -115,6 +117,7 @@ pub fn apply_test_config(config: &mut Config, test_config: &wast::TestConfig) {
115117
.wasm_custom_page_sizes(custom_page_sizes)
116118
.wasm_extended_const(extended_const)
117119
.wasm_wide_arithmetic(wide_arithmetic)
120+
.wasm_branch_hinting(branch_hinting)
118121
.wasm_component_model_async(component_model_async)
119122
.wasm_component_model_more_async_builtins(component_model_more_async_builtins)
120123
.wasm_component_model_async_stackful(component_model_async_stackful)

crates/test-util/src/wast.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,7 @@ macro_rules! foreach_config_option {
273273
tail_call
274274
extended_const
275275
wide_arithmetic
276+
branch_hinting
276277
hogs_memory
277278
nan_canonicalization
278279
component_model_async

docs/stability-wasm-proposals.md

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -74,17 +74,14 @@ The emoji legend is:
7474

7575
| Proposal | Phase 4 | Tests | Finished | Fuzzed | API | C API |
7676
|-----------------------------|---------|-------|----------|--------|-----|-------|
77-
| [`branch-hinting`] [^12] | | | 🚧 | ❌ | ✅ | ✅ |
77+
| [`branch-hinting`] [^12] | | | | ❌ | ✅ | ✅ |
7878
| [`stack-switching`] [^11] | ❌ | 🚧 | 🚧 | ❌ | ❌ | ❌ |
7979

8080
[^11]: The stack-switching proposal is a work-in-progress being tracked
8181
at [#9465](https://github.com/bytecodealliance/wasmtime/issues/9465).
8282
Currently the implementation is only for x86\_64 Linux.
83-
[^12]: Branch hinting parses the `metadata.code.branch_hint` custom section and
84-
uses it to mark cold blocks during Cranelift compilation. It is disabled by
85-
default (`Config::wasm_branch_hinting`) until it has been fuzzed; spec-test
86-
enablement and validation of malformed sections are still pending. Tracked
87-
at [#9463](https://github.com/bytecodealliance/wasmtime/issues/9463).
83+
[^12]: Disabled by default (`Config::wasm_branch_hinting`) pending fuzzing;
84+
tracked at [#9463](https://github.com/bytecodealliance/wasmtime/issues/9463).
8885

8986
## Unimplemented proposals
9087

0 commit comments

Comments
 (0)