Skip to content

Commit e3e5aa6

Browse files
authored
Refactor how inlining is configured in Wasmtime (#13250)
* Refactor how inlining is configured in Wasmtime Fold the `inlining` boolean and the `inlining_intra_module` options into a single `Inlining` enum option. This option encompasses both and has an additional mode which is "only intrinsics" where inter-module and intra-module calls are never inlined, but intrinsics for Wasmtime are allowed to be inlined. This is inspired from discussion on #13214 and after some performance work and/or confirmations the expectation is to turn the default inlining mode to `Inlining::Intrinsics`. * Fix doc example * Review comments * Fix benchmark tests
1 parent 85ae784 commit e3e5aa6

16 files changed

Lines changed: 162 additions & 92 deletions

File tree

benches/compile_time_builtins.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -88,11 +88,8 @@ impl ExposedBuf {
8888

8989
fn make_engine() -> Result<Engine, Error> {
9090
let mut config = Config::new();
91-
config.compiler_inlining(true);
91+
config.compiler_inlining(wasmtime::Inlining::Yes);
9292
config.concurrency_support(false);
93-
unsafe {
94-
config.cranelift_flag_set("wasmtime_inlining_intra_module", "yes");
95-
}
9693
let engine = Engine::new(&config)?;
9794
Ok(engine)
9895
}

crates/cli-flags/src/lib.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,9 @@ wasmtime_option_group! {
262262
pub native_unwind_info: Option<bool>,
263263

264264
/// Whether to perform function inlining during compilation.
265-
pub inlining: Option<bool>,
265+
#[serde(default)]
266+
#[serde(deserialize_with = "crate::opt::cli_parse_wrapper")]
267+
pub inlining: Option<wasmtime::Inlining>,
266268

267269
#[prefixed = "cranelift"]
268270
#[serde(default)]

crates/cli-flags/src/opt.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -590,6 +590,20 @@ impl WasmtimeOptionValue for wasmtime::Enabled {
590590
}
591591
}
592592

593+
impl WasmtimeOptionValue for wasmtime::Inlining {
594+
const VAL_HELP: &'static str = "[=y|n|gc|inter-module|intrinsics]";
595+
fn parse(val: Option<&str>) -> Result<Self> {
596+
match val {
597+
None => Ok(wasmtime::Inlining::Yes),
598+
Some(val) => val.parse(),
599+
}
600+
}
601+
602+
fn display(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
603+
write!(f, "{self}")
604+
}
605+
}
606+
593607
impl WasmtimeOptionValue for WasiNnGraph {
594608
const VAL_HELP: &'static str = "=<format>::<dir>";
595609
fn parse(val: Option<&str>) -> Result<Self> {

crates/cranelift/src/builder.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,6 @@ impl CompilerBuilder for Builder {
8787
"wasmtime_linkopt_force_jump_veneer" => {
8888
self.linkopts.force_jump_veneers = value.parse()?;
8989
}
90-
"wasmtime_inlining_intra_module" => {
91-
self.tunables.as_mut().unwrap().inlining_intra_module = value.parse()?;
92-
}
9390
"wasmtime_inlining_small_callee_size" => {
9491
self.tunables.as_mut().unwrap().inlining_small_callee_size = value.parse()?;
9592
}

crates/cranelift/src/compiler.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,10 @@ use wasmtime_environ::obj::{ELF_WASMTIME_EXCEPTIONS, ELF_WASMTIME_FRAMES};
3636
use wasmtime_environ::{
3737
Abi, AddressMapSection, BuiltinFunctionIndex, CacheStore, CompileError, CompiledFunctionBody,
3838
DefinedFuncIndex, FlagValue, FrameInstPos, FrameStackShape, FrameStateSlotBuilder,
39-
FrameTableBuilder, FuncKey, FunctionBodyData, FunctionLoc, HostCall, InliningCompiler,
40-
ModulePC, ModuleTranslation, ModuleTypesBuilder, PtrSize, StackMapSection, StaticModuleIndex,
41-
TrapEncodingBuilder, TrapSentinel, TripleExt, Tunables, WasmFuncType, WasmValType, prelude::*,
39+
FrameTableBuilder, FuncKey, FunctionBodyData, FunctionLoc, HostCall, Inlining,
40+
InliningCompiler, ModulePC, ModuleTranslation, ModuleTypesBuilder, PtrSize, StackMapSection,
41+
StaticModuleIndex, TrapEncodingBuilder, TrapSentinel, TripleExt, Tunables, WasmFuncType,
42+
WasmValType, prelude::*,
4243
};
4344
use wasmtime_unwinder::ExceptionTableBuilder;
4445

@@ -325,7 +326,7 @@ impl wasmtime_environ::Compiler for Compiler {
325326
&mut func_env,
326327
)?;
327328

328-
if self.tunables.inlining {
329+
if self.tunables.inlining != Inlining::No {
329330
compiler
330331
.cx
331332
.codegen_context

crates/environ/src/tunables.rs

Lines changed: 48 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -133,10 +133,7 @@ define_tunables! {
133133

134134
/// Whether to enable inlining in Wasmtime's compilation orchestration
135135
/// or not.
136-
pub inlining: bool,
137-
138-
/// Whether to inline calls within the same core Wasm module or not.
139-
pub inlining_intra_module: IntraModuleInlining,
136+
pub inlining: Inlining,
140137

141138
/// The size of "small callees" that can be inlined regardless of the
142139
/// caller's size.
@@ -255,8 +252,7 @@ impl Tunables {
255252
winch_callable: false,
256253
signals_based_traps: false,
257254
memory_init_cow: true,
258-
inlining: false,
259-
inlining_intra_module: IntraModuleInlining::WhenUsingGc,
255+
inlining: Inlining::No,
260256
inlining_small_callee_size: 50,
261257
inlining_sum_size_threshold: 2000,
262258
debug_guest: false,
@@ -426,31 +422,70 @@ impl fmt::Display for Collector {
426422
}
427423
}
428424

429-
/// Whether to inline function calls within the same module.
425+
/// Inlining modes supported by Wasmtime.
430426
#[derive(Clone, Copy, Hash, Serialize, Deserialize, Debug, PartialEq, Eq)]
431-
#[expect(missing_docs, reason = "self-describing variants")]
432-
pub enum IntraModuleInlining {
427+
pub enum Inlining {
428+
/// All inlining is enabled wherever possible.
429+
///
430+
/// This includes inter-module inlining (across modules) as well as
431+
/// intra-module inlining (within a module).
432+
///
433+
/// Note that backtraces may omit inlined stack frames.
433434
Yes,
435+
436+
/// Inter-module inlining (across modules) is allowed, but intra-module
437+
/// (within a module) is only allowed when the module is using GC.
438+
///
439+
/// Note that backtraces may omit inlined stack frames.
440+
InterModuleAndIntraGc,
441+
442+
/// Inter-module inlining (across modules) is allowed, but intra-module
443+
/// (within a module) is not allowed.
444+
///
445+
/// Note that backtraces may omit inlined stack frames.
446+
InterModule,
447+
448+
/// No module inlining is allowed, either inter- or intra-module. Only
449+
/// inlining Wasmtime's intrinsics are allowed.
450+
///
451+
/// This option, for example, never emits WebAssembly stack frames from
452+
/// backtraces.
453+
Intrinsics,
454+
455+
/// Inlining is disabled entirely.
434456
No,
435-
WhenUsingGc,
436457
}
437458

438-
impl FromStr for IntraModuleInlining {
459+
impl FromStr for Inlining {
439460
type Err = Error;
440461

441462
fn from_str(s: &str) -> Result<Self, Self::Err> {
442463
match s {
443464
"y" | "yes" | "true" => Ok(Self::Yes),
444465
"n" | "no" | "false" => Ok(Self::No),
445-
"gc" => Ok(Self::WhenUsingGc),
466+
"gc" => Ok(Self::InterModuleAndIntraGc),
467+
"inter-module" => Ok(Self::InterModuleAndIntraGc),
468+
"intrinsics" => Ok(Self::Intrinsics),
446469
_ => bail!(
447470
"invalid intra-module inlining option string: `{s}`, \
448-
only yes,no,gc accepted"
471+
only yes,no,gc,inter-module,intrinsics accepted"
449472
),
450473
}
451474
}
452475
}
453476

477+
impl fmt::Display for Inlining {
478+
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
479+
match self {
480+
Inlining::Yes => write!(f, "yes"),
481+
Inlining::InterModuleAndIntraGc => write!(f, "gc"),
482+
Inlining::InterModule => write!(f, "inter-module"),
483+
Inlining::Intrinsics => write!(f, "intrinsics"),
484+
Inlining::No => write!(f, "no"),
485+
}
486+
}
487+
}
488+
454489
/// The cost of each operator.
455490
///
456491
/// Note: a more dynamic approach (e.g. a user-supplied callback) can be

crates/fuzzing/src/generators/config.rs

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -350,7 +350,7 @@ impl Config {
350350

351351
self.wasmtime.codegen.configure(&mut cfg);
352352

353-
cfg.codegen.inlining = self.wasmtime.inlining;
353+
cfg.codegen.inlining = self.wasmtime.inlining.map(|i| i.into());
354354

355355
// If the wasm-smith-generated module use nan canonicalization then we
356356
// don't need to enable it, but if it doesn't enable it already then we
@@ -360,12 +360,6 @@ impl Config {
360360
// Only set cranelift specific flags when the Cranelift strategy is
361361
// chosen.
362362
if cranelift_strategy {
363-
if let Some(option) = self.wasmtime.inlining_intra_module {
364-
cfg.codegen.cranelift.push((
365-
"wasmtime_inlining_intra_module".to_string(),
366-
Some(option.to_string()),
367-
));
368-
}
369363
if let Some(size) = self.wasmtime.inlining_small_callee_size {
370364
cfg.codegen.cranelift.push((
371365
"wasmtime_inlining_small_callee_size".to_string(),
@@ -579,8 +573,7 @@ pub struct WasmtimeConfig {
579573
force_jump_veneers: bool,
580574
memory_init_cow: bool,
581575
memory_guaranteed_dense_image_size: u64,
582-
inlining: Option<bool>,
583-
inlining_intra_module: Option<IntraModuleInlining>,
576+
inlining: Option<Inlining>,
584577
inlining_small_callee_size: Option<u32>,
585578
inlining_sum_size_threshold: Option<u32>,
586579
use_precompiled_cwasm: bool,
@@ -892,18 +885,33 @@ impl RegallocAlgorithm {
892885
}
893886

894887
#[derive(Arbitrary, Clone, Copy, Debug, PartialEq, Eq, Hash)]
895-
enum IntraModuleInlining {
888+
enum Inlining {
896889
Yes,
890+
InterModuleAndIntraGc,
891+
InterModule,
892+
Intrinsics,
897893
No,
898-
WhenUsingGc,
899894
}
900895

901-
impl std::fmt::Display for IntraModuleInlining {
902-
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
903-
match self {
904-
IntraModuleInlining::Yes => write!(f, "yes"),
905-
IntraModuleInlining::No => write!(f, "no"),
906-
IntraModuleInlining::WhenUsingGc => write!(f, "gc"),
896+
impl From<Inlining> for wasmtime::Inlining {
897+
fn from(i: Inlining) -> Self {
898+
let ret = match i {
899+
Inlining::Yes => wasmtime::Inlining::Yes,
900+
Inlining::InterModuleAndIntraGc => wasmtime::Inlining::InterModuleAndIntraGc,
901+
Inlining::InterModule => wasmtime::Inlining::InterModule,
902+
Inlining::Intrinsics => wasmtime::Inlining::Intrinsics,
903+
Inlining::No => wasmtime::Inlining::No,
904+
};
905+
906+
match ret {
907+
wasmtime::Inlining::Yes
908+
| wasmtime::Inlining::No
909+
| wasmtime::Inlining::InterModuleAndIntraGc
910+
| wasmtime::Inlining::InterModule
911+
| wasmtime::Inlining::Intrinsics
912+
// NOTE: if you add another arm here, be sure to update the
913+
// `Inlining` enum above.
914+
=> ret,
907915
}
908916
}
909917
}

crates/wasmtime/src/compile.rs

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ use std::{any::Any, borrow::Cow, collections::BTreeMap, mem, ops::Range};
3030
use wasmtime_environ::{
3131
Abi, CompiledFunctionBody, CompiledFunctionsTable, CompiledFunctionsTableBuilder,
3232
CompiledModuleInfo, Compiler, DefinedFuncIndex, FilePos, FinishedObject, FuncKey,
33-
FunctionBodyData, InliningCompiler, IntraModuleInlining, ModuleEnvironment, ModuleTranslation,
33+
FunctionBodyData, Inlining, InliningCompiler, ModuleEnvironment, ModuleTranslation,
3434
ModuleTypes, ModuleTypesBuilder, ObjectKind, PrimaryMap, StaticModuleIndex, Tunables,
3535
graphs::{EntityGraph, Graph as _},
3636
};
@@ -560,7 +560,7 @@ the use case.
560560
}
561561

562562
let mut raw_outputs = if let Some(inlining_compiler) = compiler.inlining_compiler() {
563-
if engine.tunables().inlining {
563+
if engine.tunables().inlining != Inlining::No {
564564
self.compile_with_inlining(engine, compiler, inlining_compiler)?
565565
} else {
566566
// Inlining compiler but inlining is disabled: compile each
@@ -803,7 +803,7 @@ the use case.
803803
);
804804

805805
debug_assert!(
806-
tunables.inlining,
806+
tunables.inlining != Inlining::No,
807807
"shouldn't even call this method if we aren't configured for inlining"
808808
);
809809
debug_assert_ne!(caller_key, callee_key, "we never inline recursion");
@@ -841,29 +841,30 @@ the use case.
841841
(
842842
FuncKey::DefinedWasmFunction(caller_module, _),
843843
FuncKey::DefinedWasmFunction(callee_module, _),
844-
) => {
845-
if caller_module == callee_module {
846-
match tunables.inlining_intra_module {
847-
IntraModuleInlining::Yes => {}
848-
849-
IntraModuleInlining::WhenUsingGc
850-
if caller_needs_gc_heap || callee_needs_gc_heap => {}
851-
852-
IntraModuleInlining::WhenUsingGc => {
853-
log::trace!(
854-
" --> not inlining: intra-module call that does not use GC"
855-
);
856-
return false;
857-
}
844+
) => match tunables.inlining {
845+
Inlining::Yes => {}
858846

859-
IntraModuleInlining::No => {
860-
log::trace!(" --> not inlining: intra-module call");
861-
return false;
862-
}
847+
Inlining::InterModuleAndIntraGc => {
848+
if caller_module == callee_module && !caller_needs_gc_heap {
849+
log::trace!(" --> not inlining: intra-module call where GC is not used");
850+
return false;
863851
}
864852
}
865-
}
866853

854+
Inlining::InterModule => {
855+
if caller_module == callee_module {
856+
log::trace!(" --> not inlining: only inter-module calls inlined");
857+
return false;
858+
}
859+
}
860+
861+
Inlining::Intrinsics => {
862+
log::trace!(" --> not inlining: only inlining intrinsics");
863+
return false;
864+
}
865+
866+
Inlining::No => unreachable!(),
867+
},
867868
_ => {}
868869
}
869870

crates/wasmtime/src/compile/code_builder.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -456,7 +456,7 @@ impl<'a> CodeBuilder<'a> {
456456
/// // Enable function inlining during compilation. If you are using unsafe intrinsics, you
457457
/// // almost assuredly want them inlined to avoid function call overheads.
458458
/// let mut config = Config::new();
459-
/// config.compiler_inlining(true);
459+
/// config.compiler_inlining(wasmtime::Inlining::Yes);
460460
///
461461
/// let engine = Engine::new(&config)?;
462462
/// let linker = wasmtime::component::Linker::new(&engine);

crates/wasmtime/src/config.rs

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ pub use crate::runtime::code_memory::CustomCodeMemory;
3131
pub use wasmtime_cache::{Cache, CacheConfig};
3232
#[cfg(all(feature = "incremental-cache", feature = "cranelift"))]
3333
pub use wasmtime_environ::CacheStore;
34+
pub use wasmtime_environ::Inlining;
3435

3536
pub(crate) const DEFAULT_WASM_BACKTRACE_MAX_FRAMES: NonZeroUsize = NonZeroUsize::new(20).unwrap();
3637

@@ -2262,9 +2263,8 @@ impl Config {
22622263
/// when using a compilation strategy that does not support inlining, like
22632264
/// Winch.
22642265
///
2265-
/// Note that inlining is still somewhat experimental at the moment (as of
2266-
/// the Wasmtime version 36).
2267-
pub fn compiler_inlining(&mut self, inlining: bool) -> &mut Self {
2266+
/// The default value for this is `Inlining::No`.
2267+
pub fn compiler_inlining(&mut self, inlining: Inlining) -> &mut Self {
22682268
self.tunables.inlining = Some(inlining);
22692269
self
22702270
}
@@ -2564,6 +2564,17 @@ impl Config {
25642564
tunables.signals_based_traps = false;
25652565
}
25662566

2567+
// Inlining currently falls over with the `stack_switch` instruction.
2568+
#[cfg(any(feature = "cranelift", feature = "winch"))]
2569+
if features.contains(WasmFeatures::STACK_SWITCHING) {
2570+
if let Some(inlining) = self.tunables.inlining
2571+
&& inlining != Inlining::No
2572+
{
2573+
bail!("cannot enable compiler inlining when stack switching is enabled");
2574+
}
2575+
tunables.inlining = Inlining::No;
2576+
}
2577+
25672578
self.tunables.configure(&mut tunables);
25682579

25692580
// If no GC heap tunables are explicitly configured, copy the memory

0 commit comments

Comments
 (0)