Skip to content

Commit da8aa9c

Browse files
alexcrichtonalexcrichton
authored
Apply argument extensions to component intrinsics too (bytecodealliance#10540)
* Apply argument extensions to component intrinsics too This commit fixes the definition of host signatures for component builtins to work in the same way that core builtins do which is to use platform-specific sign-extension-rules by default. This fixes a regression found in the wasip3-prototyping repository where new component intrinsics didn't have this set and subsequently were failing in riscv64 CI. * Add some tests
1 parent a6f4185 commit da8aa9c

File tree

5 files changed

+152
-19
lines changed

5 files changed

+152
-19
lines changed

‎crates/cranelift/src/compiler/component.rs‎

Lines changed: 32 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1716,19 +1716,14 @@ mod host {
17161716
$(
17171717
pub(super) fn $name(isa: &dyn TargetIsa, func: &mut ir::Function) -> (ir::SigRef, ComponentBuiltinFunctionIndex) {
17181718
let pointer_type = isa.pointer_type();
1719-
let params = vec![
1720-
$( AbiParam::new(define!(@ty pointer_type $param)) ),*
1721-
];
1722-
let returns = vec![
1723-
$( AbiParam::new(define!(@ty pointer_type $result)) )?
1724-
];
1725-
let sig = func.import_signature(ir::Signature {
1726-
params,
1727-
returns,
1728-
call_conv: CallConv::triple_default(isa.triple()),
1729-
});
1730-
1731-
(sig, ComponentBuiltinFunctionIndex::$name())
1719+
let sig = build_sig(
1720+
isa,
1721+
func,
1722+
&[$( define!(@ty pointer_type $param) ),*],
1723+
&[$( define!(@ty pointer_type $result) ),*],
1724+
);
1725+
1726+
return (sig, ComponentBuiltinFunctionIndex::$name())
17321727
}
17331728
)*
17341729
};
@@ -1745,4 +1740,28 @@ mod host {
17451740
}
17461741

17471742
wasmtime_environ::foreach_builtin_component_function!(define);
1743+
1744+
fn build_sig(
1745+
isa: &dyn TargetIsa,
1746+
func: &mut ir::Function,
1747+
params: &[ir::Type],
1748+
returns: &[ir::Type],
1749+
) -> ir::SigRef {
1750+
let mut sig = ir::Signature {
1751+
params: params.iter().map(|ty| AbiParam::new(*ty)).collect(),
1752+
returns: returns.iter().map(|ty| AbiParam::new(*ty)).collect(),
1753+
call_conv: CallConv::triple_default(isa.triple()),
1754+
};
1755+
1756+
// Once we're declaring the signature of a host function we must respect
1757+
// the default ABI of the platform which is where argument extension of
1758+
// params/results may come into play.
1759+
let extension = isa.default_argument_extension();
1760+
for arg in sig.params.iter_mut().chain(sig.returns.iter_mut()) {
1761+
if arg.value_type.is_int() {
1762+
arg.extension = extension;
1763+
}
1764+
}
1765+
func.import_signature(sig)
1766+
}
17481767
}

‎src/commands/objdump.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ impl ObjdumpCommand {
154154
Func::Wasm
155155
} else if name.contains("trampoline") {
156156
Func::Trampoline
157-
} else if name.contains("libcall") {
157+
} else if name.contains("libcall") || name.starts_with("component") {
158158
Func::Libcall
159159
} else {
160160
panic!("unknown symbol: {name}")

‎tests/disas.rs‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ struct TestConfig {
112112
test: TestKind,
113113
flags: Option<TestConfigFlags>,
114114
objdump: Option<TestConfigFlags>,
115+
filter: Option<String>,
115116
}
116117

117118
#[derive(Debug, Deserialize)]
@@ -195,10 +196,16 @@ impl Test {
195196
}
196197
}
197198
let engine = Engine::new(&config).context("failed to create engine")?;
198-
let module = wat::parse_file(&self.path)?;
199-
let elf = engine
200-
.precompile_module(&module)
201-
.context("failed to compile module")?;
199+
let wasm = wat::parse_file(&self.path)?;
200+
let elf = if wasmparser::Parser::is_component(&wasm) {
201+
engine
202+
.precompile_component(&wasm)
203+
.context("failed to compile component")?
204+
} else {
205+
engine
206+
.precompile_module(&wasm)
207+
.context("failed to compile module")?
208+
};
202209

203210
match self.config.test {
204211
TestKind::Clif | TestKind::Optimize => {
@@ -213,7 +220,8 @@ impl Test {
213220
let entry = entry.context("failed to iterate over tempdir")?;
214221
let path = entry.path();
215222
if let Some(name) = path.file_name().and_then(|s| s.to_str()) {
216-
if !name.starts_with("wasm_func_") {
223+
let filter = self.config.filter.as_deref().unwrap_or("wasm_func_");
224+
if !name.contains(filter) {
217225
continue;
218226
}
219227
}
@@ -276,6 +284,9 @@ fn assert_output(test: &Test, output: CompileOutput) -> Result<()> {
276284
cmd.arg("--traps=false");
277285
}
278286
}
287+
if let Some(filter) = &test.config.filter {
288+
cmd.arg("--filter").arg(filter);
289+
}
279290

280291
let mut child = cmd.spawn().context("failed to run wasmtime")?;
281292
child

‎tests/disas/riscv64-component-builtins-asm.wat‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
;;! target = "riscv64"
2+
;;! test = 'compile'
3+
;;! filter = '_wasm_call'
4+
;;! objdump = '--funcs all'
5+
6+
(component
7+
(type $a (resource (rep i32)))
8+
(core func $f (canon resource.drop $a))
9+
10+
(core module $m (import "" "" (func (param i32))))
11+
(core instance (instantiate $m (with "" (instance (export "" (func $f))))))
12+
)
13+
14+
;; component-resource-drop[0]_wasm_call:
15+
;; addi sp, sp, -0x10
16+
;; sd ra, 8(sp)
17+
;; sd s0, 0(sp)
18+
;; mv s0, sp
19+
;; addi sp, sp, -0x10
20+
;; sd s1, 8(sp)
21+
;; mv s1, a1
22+
;; lw a1, 0(a0)
23+
;; lui a5, 0x706d7
24+
;; addi a3, a5, -0x9d
25+
;; beq a1, a3, 8
26+
;; .byte 0x00, 0x00, 0x00, 0x00
27+
;; ld a1, 0x10(a0)
28+
;; ld a3, 0(s0)
29+
;; sd a3, 0x18(a1)
30+
;; ld a3, 8(s0)
31+
;; sd a3, 0x20(a1)
32+
;; ld a3, 8(a0)
33+
;; ld a3, 0x10(a3)
34+
;; mv a4, zero
35+
;; slli a1, a4, 0x20
36+
;; srai a1, a1, 0x20
37+
;; slli a2, a2, 0x20
38+
;; srai a2, a2, 0x20
39+
;; jalr a3
40+
;; addi a3, zero, -1
41+
;; beq a0, a3, 0x1c
42+
;; ld s1, 8(sp)
43+
;; addi sp, sp, 0x10
44+
;; ld ra, 8(sp)
45+
;; ld s0, 0(sp)
46+
;; addi sp, sp, 0x10
47+
;; ret
48+
;; mv a1, s1
49+
;; ld a4, 0x10(a1)
50+
;; ld a4, 0x138(a4)
51+
;; mv a0, a1
52+
;; jalr a4
53+
;; .byte 0x00, 0x00, 0x00, 0x00

‎tests/disas/riscv64-component-builtins.wat‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
;;! target = "riscv64"
2+
;;! test = 'optimize'
3+
;;! filter = 'component_trampoline_0_Wasm'
4+
5+
(component
6+
(type $a (resource (rep i32)))
7+
(core func $f (canon resource.drop $a))
8+
9+
(core module $m (import "" "" (func (param i32))))
10+
(core instance (instantiate $m (with "" (instance (export "" (func $f))))))
11+
)
12+
13+
;; function u0:0(i64 vmctx, i64, i32) tail {
14+
;; sig0 = (i64 sext, i32 sext, i32 sext) -> i64 sext system_v
15+
;; sig1 = (i64 sext vmctx) system_v
16+
;;
17+
;; block0(v0: i64, v1: i64, v2: i32):
18+
;; v3 = load.i32 notrap aligned little v0
19+
;; v17 = iconst.i32 0x706d_6f63
20+
;; v4 = icmp eq v3, v17 ; v17 = 0x706d_6f63
21+
;; trapz v4, user1
22+
;; v5 = load.i64 notrap aligned v0+16
23+
;; v6 = get_frame_pointer.i64
24+
;; v7 = load.i64 notrap aligned v6
25+
;; store notrap aligned v7, v5+24
26+
;; v8 = get_return_address.i64
27+
;; store notrap aligned v8, v5+32
28+
;; v10 = load.i64 notrap aligned readonly v0+8
29+
;; v11 = load.i64 notrap aligned readonly v10+16
30+
;; v9 = iconst.i32 0
31+
;; v12 = call_indirect sig0, v11(v0, v9, v2) ; v9 = 0
32+
;; v13 = iconst.i64 -1
33+
;; v14 = icmp ne v12, v13 ; v13 = -1
34+
;; brif v14, block2, block1
35+
;;
36+
;; block1 cold:
37+
;; v15 = load.i64 notrap aligned readonly v1+16
38+
;; v16 = load.i64 notrap aligned readonly v15+312
39+
;; call_indirect sig1, v16(v1)
40+
;; trap user1
41+
;;
42+
;; block2:
43+
;; brif.i64 v12, block3, block4
44+
;;
45+
;; block3:
46+
;; jump block4
47+
;;
48+
;; block4:
49+
;; return
50+
;; }

0 commit comments

Comments
 (0)