Skip to content

Commit 734f71c

Browse files
authored
Only create ir::FuncRefs for direct calls (#11408)
We would previously always create Cranelift `ir::FuncRef`s for the callee of a Wasm `call` instruction, even if that was a call to an imported function that actually gets translated to an indirect call in CLIF. This commit makes it so that we delay `ir::FuncRef` creation until we actually emit a direct call. Also I found that we were not looking for already-created `ir::SigRef`s when creating `ir::FuncRef`s, so I fixed that as well, which removes a couple duplicate signature definitions in our disas tests.
1 parent b500820 commit 734f71c

11 files changed

Lines changed: 136 additions & 81 deletions

crates/cranelift/src/func_environ.rs

Lines changed: 61 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,11 @@ use smallvec::SmallVec;
2222
use std::mem;
2323
use wasmparser::{Operator, WasmFeatures};
2424
use wasmtime_environ::{
25-
BuiltinFunctionIndex, DataIndex, ElemIndex, EngineOrModuleTypeIndex, FuncIndex, GlobalIndex,
26-
IndexType, Memory, MemoryIndex, Module, ModuleInternedTypeIndex, ModuleTranslation,
27-
ModuleTypesBuilder, PtrSize, Table, TableIndex, TripleExt, Tunables, TypeConvert, TypeIndex,
28-
VMOffsets, WasmCompositeInnerType, WasmFuncType, WasmHeapTopType, WasmHeapType, WasmRefType,
29-
WasmResult, WasmValType,
25+
BuiltinFunctionIndex, DataIndex, DefinedFuncIndex, ElemIndex, EngineOrModuleTypeIndex,
26+
FuncIndex, GlobalIndex, IndexType, Memory, MemoryIndex, Module, ModuleInternedTypeIndex,
27+
ModuleTranslation, ModuleTypesBuilder, PtrSize, Table, TableIndex, TripleExt, Tunables,
28+
TypeConvert, TypeIndex, VMOffsets, WasmCompositeInnerType, WasmFuncType, WasmHeapTopType,
29+
WasmHeapType, WasmRefType, WasmResult, WasmValType,
3030
};
3131
use wasmtime_environ::{FUNCREF_INIT_BIT, FUNCREF_MASK};
3232
use wasmtime_math::f64_cvt_to_int_bounds;
@@ -1176,9 +1176,14 @@ pub(crate) struct WasmEntities {
11761176
/// `ir::SigRef` in the Cranelift function we are building.
11771177
pub(crate) sig_refs: SecondaryMap<ModuleInternedTypeIndex, PackedOption<ir::SigRef>>,
11781178

1179-
/// Map from a Wasm function index to its associated function reference in
1180-
/// the Cranelift function we are building.
1181-
pub(crate) func_refs: SecondaryMap<FuncIndex, PackedOption<ir::FuncRef>>,
1179+
/// Map from a defined Wasm function index to its associated function
1180+
/// reference in the Cranelift function we are building.
1181+
pub(crate) defined_func_refs: SecondaryMap<DefinedFuncIndex, PackedOption<ir::FuncRef>>,
1182+
1183+
/// Map from an imported Wasm function index for which we statically know
1184+
/// which function will always be used to satisfy that import to its
1185+
/// associated function reference in the Cranelift function we are building.
1186+
pub(crate) imported_func_refs: SecondaryMap<FuncIndex, PackedOption<ir::FuncRef>>,
11821187

11831188
/// Map from a Wasm table index to its associated implementation in the
11841189
/// Cranelift function we are building.
@@ -1207,7 +1212,8 @@ impl FuncEnvironment<'_> {
12071212
get_or_create_global(globals) : make_global : GlobalIndex => GlobalVariable;
12081213
get_or_create_heap(memories) : make_heap : MemoryIndex => Heap;
12091214
get_or_create_interned_sig_ref(sig_refs) : make_sig_ref : ModuleInternedTypeIndex => ir::SigRef;
1210-
get_or_create_func_ref(func_refs) : make_func_ref : FuncIndex => ir::FuncRef;
1215+
get_or_create_defined_func_ref(defined_func_refs) : make_defined_func_ref : DefinedFuncIndex => ir::FuncRef;
1216+
get_or_create_imported_func_ref(imported_func_refs) : make_imported_func_ref : FuncIndex => ir::FuncRef;
12111217
get_or_create_table(tables) : make_table : TableIndex => TableData;
12121218
}
12131219

@@ -1253,39 +1259,48 @@ impl FuncEnvironment<'_> {
12531259
sig_ref
12541260
}
12551261

1256-
fn make_func_ref(&mut self, func: &mut ir::Function, index: FuncIndex) -> ir::FuncRef {
1257-
let sig = self.module.functions[index]
1262+
fn make_defined_func_ref(
1263+
&mut self,
1264+
func: &mut ir::Function,
1265+
def_func_index: DefinedFuncIndex,
1266+
) -> ir::FuncRef {
1267+
let func_index = self.module.func_index(def_func_index);
1268+
let ty = self.module.functions[func_index]
12581269
.signature
12591270
.unwrap_module_type_index();
1260-
let wasm_func_ty = self.types[sig].unwrap_func();
1261-
let sig = crate::wasm_call_signature(self.isa, wasm_func_ty, &self.tunables);
1262-
let signature = func.import_signature(sig);
1263-
self.sig_ref_to_ty[signature] = Some(wasm_func_ty);
1271+
let signature = self.get_or_create_interned_sig_ref(func, ty);
12641272
let name =
12651273
ir::ExternalName::User(func.declare_imported_user_function(ir::UserExternalName {
12661274
namespace: crate::NS_WASM_FUNC,
1267-
index: index.as_u32(),
1275+
index: func_index.as_u32(),
12681276
}));
12691277
func.import_function(ir::ExtFuncData {
12701278
name,
12711279
signature,
1280+
colocated: true,
1281+
})
1282+
}
12721283

1273-
// the value of this flag determines the codegen for calls to this
1274-
// function. if this flag is `false` then absolute relocations will
1275-
// be generated for references to the function, which requires
1276-
// load-time relocation resolution. if this flag is set to `true`
1277-
// then relative relocations are emitted which can be resolved at
1278-
// object-link-time, just after all functions are compiled.
1279-
//
1280-
// this flag is set to `true` for functions defined in the object
1281-
// we'll be defining in this compilation unit, or everything local
1282-
// to the wasm module. this means that between functions in a wasm
1283-
// module there's relative calls encoded. all calls external to a
1284-
// wasm module (e.g. imports or libcalls) are either encoded through
1285-
// the `vmcontext` as relative jumps (hence no relocations) or
1286-
// they're libcalls with absolute relocations.
1287-
colocated: self.module.defined_func_index(index).is_some()
1288-
|| self.translation.known_imported_functions[index].is_some(),
1284+
fn make_imported_func_ref(
1285+
&mut self,
1286+
func: &mut ir::Function,
1287+
func_index: FuncIndex,
1288+
) -> ir::FuncRef {
1289+
assert!(self.module.is_imported_function(func_index));
1290+
assert!(self.translation.known_imported_functions[func_index].is_some());
1291+
let ty = self.module.functions[func_index]
1292+
.signature
1293+
.unwrap_module_type_index();
1294+
let signature = self.get_or_create_interned_sig_ref(func, ty);
1295+
let name =
1296+
ir::ExternalName::User(func.declare_imported_user_function(ir::UserExternalName {
1297+
namespace: crate::NS_WASM_FUNC,
1298+
index: func_index.as_u32(),
1299+
}));
1300+
func.import_function(ir::ExtFuncData {
1301+
name,
1302+
signature,
1303+
colocated: true,
12891304
})
12901305
}
12911306

@@ -1628,11 +1643,11 @@ impl<'a, 'func, 'module_env> Call<'a, 'func, 'module_env> {
16281643
}
16291644
}
16301645

1631-
/// Do a direct call to the given callee function.
1646+
/// Do a Wasm-level direct call to the given callee function.
16321647
pub fn direct_call(
16331648
mut self,
16341649
callee_index: FuncIndex,
1635-
callee: ir::FuncRef,
1650+
sig_ref: ir::SigRef,
16361651
call_args: &[ir::Value],
16371652
) -> WasmResult<ir::Inst> {
16381653
let mut real_call_args = Vec::with_capacity(call_args.len() + 2);
@@ -1643,7 +1658,7 @@ impl<'a, 'func, 'module_env> Call<'a, 'func, 'module_env> {
16431658
.unwrap();
16441659

16451660
// Handle direct calls to locally-defined functions.
1646-
if !self.env.module.is_imported_function(callee_index) {
1661+
if let Some(def_func_index) = self.env.module.defined_func_index(callee_index) {
16471662
// First append the callee vmctx address, which is the same as the caller vmctx in
16481663
// this case.
16491664
real_call_args.push(caller_vmctx);
@@ -1655,13 +1670,15 @@ impl<'a, 'func, 'module_env> Call<'a, 'func, 'module_env> {
16551670
real_call_args.extend_from_slice(call_args);
16561671

16571672
// Finally, make the direct call!
1673+
let callee = self
1674+
.env
1675+
.get_or_create_defined_func_ref(self.builder.func, def_func_index);
16581676
return Ok(self.direct_call_inst(callee, &real_call_args));
16591677
}
16601678

16611679
// Handle direct calls to imported functions. We use an indirect call
16621680
// so that we don't have to patch the code at runtime.
16631681
let pointer_type = self.env.pointer_type();
1664-
let sig_ref = self.builder.func.dfg.ext_funcs[callee].signature;
16651682
let vmctx = self.env.vmctx(self.builder.func);
16661683
let base = self.builder.ins().global_value(pointer_type, vmctx);
16671684

@@ -1694,6 +1711,9 @@ impl<'a, 'func, 'module_env> Call<'a, 'func, 'module_env> {
16941711
// pass the callee's vmctx that we just loaded, not our own). Otherwise,
16951712
// we really do an indirect call.
16961713
if self.env.translation.known_imported_functions[callee_index].is_some() {
1714+
let callee = self
1715+
.env
1716+
.get_or_create_imported_func_ref(self.builder.func, callee_index);
16971717
Ok(self.direct_call_inst(callee, &real_call_args))
16981718
} else {
16991719
let func_addr = self
@@ -1704,7 +1724,7 @@ impl<'a, 'func, 'module_env> Call<'a, 'func, 'module_env> {
17041724
}
17051725
}
17061726

1707-
/// Do an indirect call through the given funcref table.
1727+
/// Do a Wasm-level indirect call through the given funcref table.
17081728
pub fn indirect_call(
17091729
mut self,
17101730
features: &WasmFeatures,
@@ -2773,10 +2793,10 @@ impl FuncEnvironment<'_> {
27732793
&mut self,
27742794
builder: &mut FunctionBuilder,
27752795
callee_index: FuncIndex,
2776-
callee: ir::FuncRef,
2796+
sig_ref: ir::SigRef,
27772797
call_args: &[ir::Value],
27782798
) -> WasmResult<ir::Inst> {
2779-
Call::new(builder, self).direct_call(callee_index, callee, call_args)
2799+
Call::new(builder, self).direct_call(callee_index, sig_ref, call_args)
27802800
}
27812801

27822802
pub fn translate_call_ref(
@@ -2793,10 +2813,10 @@ impl FuncEnvironment<'_> {
27932813
&mut self,
27942814
builder: &mut FunctionBuilder,
27952815
callee_index: FuncIndex,
2796-
callee: ir::FuncRef,
2816+
sig_ref: ir::SigRef,
27972817
call_args: &[ir::Value],
27982818
) -> WasmResult<()> {
2799-
Call::new_tail(builder, self).direct_call(callee_index, callee, call_args)?;
2819+
Call::new_tail(builder, self).direct_call(callee_index, sig_ref, call_args)?;
28002820
Ok(())
28012821
}
28022822

crates/cranelift/src/translate/code_translator.rs

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -588,25 +588,21 @@ pub fn translate_operator(
588588
************************************************************************************/
589589
Operator::Call { function_index } => {
590590
let function_index = FuncIndex::from_u32(*function_index);
591-
let fref = environ.get_or_create_func_ref(builder.func, function_index);
591+
let ty = environ.module.functions[function_index]
592+
.signature
593+
.unwrap_module_type_index();
594+
let sig_ref = environ.get_or_create_interned_sig_ref(builder.func, ty);
592595
let num_args = environ.num_params_for_func(function_index);
593596

594597
// Bitcast any vector arguments to their default type, I8X16, before calling.
595598
let args = stack.peekn_mut(num_args);
596-
bitcast_wasm_params(
597-
environ,
598-
builder.func.dfg.ext_funcs[fref].signature,
599-
args,
600-
builder,
601-
);
599+
bitcast_wasm_params(environ, sig_ref, args, builder);
602600

603-
let call = environ.translate_call(builder, function_index, fref, args)?;
601+
let call = environ.translate_call(builder, function_index, sig_ref, args)?;
604602
let inst_results = builder.inst_results(call);
605603
debug_assert_eq!(
606604
inst_results.len(),
607-
builder.func.dfg.signatures[builder.func.dfg.ext_funcs[fref].signature]
608-
.returns
609-
.len(),
605+
builder.func.dfg.signatures[sig_ref].returns.len(),
610606
"translate_call results should match the call signature"
611607
);
612608
stack.popn(num_args);
@@ -662,19 +658,17 @@ pub fn translate_operator(
662658
************************************************************************************/
663659
Operator::ReturnCall { function_index } => {
664660
let function_index = FuncIndex::from_u32(*function_index);
665-
let fref = environ.get_or_create_func_ref(builder.func, function_index);
661+
let ty = environ.module.functions[function_index]
662+
.signature
663+
.unwrap_module_type_index();
664+
let sig_ref = environ.get_or_create_interned_sig_ref(builder.func, ty);
666665
let num_args = environ.num_params_for_func(function_index);
667666

668667
// Bitcast any vector arguments to their default type, I8X16, before calling.
669668
let args = stack.peekn_mut(num_args);
670-
bitcast_wasm_params(
671-
environ,
672-
builder.func.dfg.ext_funcs[fref].signature,
673-
args,
674-
builder,
675-
);
669+
bitcast_wasm_params(environ, sig_ref, args, builder);
676670

677-
environ.translate_return_call(builder, function_index, fref, args)?;
671+
environ.translate_return_call(builder, function_index, sig_ref, args)?;
678672

679673
stack.popn(num_args);
680674
stack.reachable = false;

tests/disas/call-indirect.wat

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
;;! target = "x86_64"
2+
3+
(module
4+
(table (export "t") 0 100 funcref)
5+
(func (export "f") (param i32 i32) (result i32)
6+
(call_indirect (param i32) (result i32) (local.get 0) (local.get 1))
7+
)
8+
)
9+
10+
;; function u0:0(i64 vmctx, i64, i32, i32) -> i32 tail {
11+
;; gv0 = vmctx
12+
;; gv1 = load.i64 notrap aligned readonly gv0+8
13+
;; gv2 = load.i64 notrap aligned gv1+16
14+
;; gv3 = vmctx
15+
;; gv4 = load.i64 notrap aligned gv3+48
16+
;; gv5 = load.i64 notrap aligned gv3+56
17+
;; sig0 = (i64 vmctx, i64, i32) -> i32 tail
18+
;; sig1 = (i64 vmctx, i32, i64) -> i64 tail
19+
;; fn0 = colocated u1:9 sig1
20+
;; stack_limit = gv2
21+
;;
22+
;; block0(v0: i64, v1: i64, v2: i32, v3: i32):
23+
;; @0035 v5 = load.i64 notrap aligned v0+56
24+
;; @0035 v6 = ireduce.i32 v5
25+
;; @0035 v7 = icmp uge v3, v6
26+
;; @0035 v8 = uextend.i64 v3
27+
;; @0035 v9 = load.i64 notrap aligned v0+48
28+
;; v30 = iconst.i64 3
29+
;; @0035 v10 = ishl v8, v30 ; v30 = 3
30+
;; @0035 v11 = iadd v9, v10
31+
;; @0035 v12 = iconst.i64 0
32+
;; @0035 v13 = select_spectre_guard v7, v12, v11 ; v12 = 0
33+
;; @0035 v14 = load.i64 user5 aligned table v13
34+
;; v29 = iconst.i64 -2
35+
;; @0035 v15 = band v14, v29 ; v29 = -2
36+
;; @0035 brif v14, block3(v15), block2
37+
;;
38+
;; block2 cold:
39+
;; @0035 v17 = iconst.i32 0
40+
;; @0035 v19 = uextend.i64 v3
41+
;; @0035 v20 = call fn0(v0, v17, v19) ; v17 = 0
42+
;; @0035 jump block3(v20)
43+
;;
44+
;; block3(v16: i64):
45+
;; @0035 v22 = load.i64 notrap aligned readonly can_move v0+40
46+
;; @0035 v23 = load.i32 notrap aligned readonly can_move v22+4
47+
;; @0035 v24 = load.i32 user6 aligned readonly v16+16
48+
;; @0035 v25 = icmp eq v24, v23
49+
;; @0035 trapz v25, user7
50+
;; @0035 v26 = load.i64 notrap aligned readonly v16+8
51+
;; @0035 v27 = load.i64 notrap aligned readonly v16+24
52+
;; @0035 v28 = call_indirect sig0, v26(v27, v0, v2)
53+
;; @0038 jump block1
54+
;;
55+
;; block1:
56+
;; @0038 return v28
57+
;; }

tests/disas/component-model/exported-module-makes-adapters-indirect.wat

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,6 @@
6464
;; gv2 = load.i64 notrap aligned gv1+16
6565
;; gv3 = vmctx
6666
;; sig0 = (i64 vmctx, i64, i32) -> i32 tail
67-
;; fn0 = u0:0 sig0
6867
;; stack_limit = gv2
6968
;;
7069
;; block0(v0: i64, v1: i64):

tests/disas/component-model/inlining-fuzz-bug.wat

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,9 @@
5454
;; gv13 = load.i64 notrap aligned gv12+16
5555
;; sig0 = (i64 vmctx, i64) -> i32 tail
5656
;; sig1 = (i64 vmctx, i64) -> i32 tail
57-
;; sig2 = (i64 vmctx, i64) -> i32 tail
5857
;; fn0 = colocated u0:0 sig0
5958
;; fn1 = colocated u0:0 sig1
60-
;; fn2 = colocated u0:1 sig2
59+
;; fn2 = colocated u0:1 sig1
6160
;; stack_limit = gv2
6261
;;
6362
;; block0(v0: i64, v1: i64):

tests/disas/component-model/multiple-instantiations-makes-adapters-indirect.wat

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@
7070
;; gv2 = load.i64 notrap aligned gv1+16
7171
;; gv3 = vmctx
7272
;; sig0 = (i64 vmctx, i64, i32) -> i32 tail
73-
;; fn0 = u0:0 sig0
7473
;; stack_limit = gv2
7574
;;
7675
;; block0(v0: i64, v1: i64):

tests/disas/gc/drc/br-on-cast-fail.wat

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,7 @@
2727
;; gv6 = load.i64 notrap aligned gv4+32
2828
;; sig0 = (i64 vmctx, i32, i32) -> i32 tail
2929
;; sig1 = (i64 vmctx, i64) tail
30-
;; sig2 = (i64 vmctx, i64) tail
3130
;; fn0 = colocated u1:35 sig0
32-
;; fn1 = u0:0 sig1
33-
;; fn2 = u0:1 sig2
3431
;; stack_limit = gv2
3532
;;
3633
;; block0(v0: i64, v1: i64, v2: i32):
@@ -81,6 +78,6 @@
8178
;; block2:
8279
;; @0038 v30 = load.i64 notrap aligned readonly can_move v0+72
8380
;; @0038 v29 = load.i64 notrap aligned readonly can_move v0+88
84-
;; @0038 call_indirect sig2, v30(v29, v0)
81+
;; @0038 call_indirect sig1, v30(v29, v0)
8582
;; @003a return
8683
;; }

tests/disas/gc/drc/br-on-cast.wat

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,7 @@
2727
;; gv6 = load.i64 notrap aligned gv4+32
2828
;; sig0 = (i64 vmctx, i32, i32) -> i32 tail
2929
;; sig1 = (i64 vmctx, i64) tail
30-
;; sig2 = (i64 vmctx, i64) tail
3130
;; fn0 = colocated u1:35 sig0
32-
;; fn1 = u0:0 sig1
33-
;; fn2 = u0:1 sig2
3431
;; stack_limit = gv2
3532
;;
3633
;; block0(v0: i64, v1: i64, v2: i32):
@@ -81,6 +78,6 @@
8178
;; block2:
8279
;; @0039 v30 = load.i64 notrap aligned readonly can_move v0+72
8380
;; @0039 v29 = load.i64 notrap aligned readonly can_move v0+88
84-
;; @0039 call_indirect sig2, v30(v29, v0)
81+
;; @0039 call_indirect sig1, v30(v29, v0)
8582
;; @003b return
8683
;; }

0 commit comments

Comments
 (0)