Repository: bytecodealliance/wasmtime
Pull request #7770 (2024-01-11 16:13:46)

Fix the ABI of Option<u128> on x86_64
LLVM passes the tag in rax and the u128 val in rdx/rcx. Before this change Cranelift would pass the first half of the value in rdx and the second half using an implicit return value pointer.

Fixes https://github.com/rust-lang/rustc_codegen_cranelift/issues/1449



Patch:
diff --git a/cranelift/codegen/src/isa/x64/abi.rs b/cranelift/codegen/src/isa/x64/abi.rs
index 49c54d529..d7df15c61 100644
--- a/cranelift/codegen/src/isa/x64/abi.rs
+++ b/cranelift/codegen/src/isa/x64/abi.rs
@@ -222,7 +222,7 @@ impl ABIMachineSpec for X64ABIMachineSpec {
                         ArgsOrRets::Args => {
                             get_intreg_for_arg(&call_conv, next_gpr, next_param_idx)
                         }
-                        ArgsOrRets::Rets => get_intreg_for_retval(&call_conv, next_gpr),
+                        ArgsOrRets::Rets => get_intreg_for_retval(&call_conv, flags, next_gpr),
                     }
                 } else {
                     match args_or_rets {
@@ -1035,7 +1035,11 @@ fn get_fltreg_for_arg(call_conv: &CallConv, idx: usize, arg_idx: usize) -> Optio
     }
 }
 
-fn get_intreg_for_retval(call_conv: &CallConv, intreg_idx: usize) -> Option<Reg> {
+fn get_intreg_for_retval(
+    call_conv: &CallConv,
+    flags: &settings::Flags,
+    intreg_idx: usize,
+) -> Option<Reg> {
     match call_conv {
         CallConv::Tail => match intreg_idx {
             0 => Some(regs::rax()),
@@ -1057,6 +1061,7 @@ fn get_intreg_for_retval(call_conv: &CallConv, intreg_idx: usize) -> Option<Reg>
         CallConv::Fast | CallConv::Cold | CallConv::SystemV => match intreg_idx {
             0 => Some(regs::rax()),
             1 => Some(regs::rdx()),
+            2 if flags.enable_llvm_abi_extensions() => Some(regs::rcx()),
             _ => None,
         },
         CallConv::WindowsFastcall => match intreg_idx {


Test patch:
diff --git a/cranelift/filetests/filetests/isa/x64/i128.clif b/cranelift/filetests/filetests/isa/x64/i128.clif
index 0654089de..eb69119cc 100644
--- a/cranelift/filetests/filetests/isa/x64/i128.clif
+++ b/cranelift/filetests/filetests/isa/x64/i128.clif
@@ -1366,17 +1366,17 @@ block0(v0: i128):
 ;   pushq   %rbp
 ;   movq    %rsp, %rbp
 ; block0:
-;   movq    %rdi, 0(%rdx)
-;   movq    %rsi, 8(%rdx)
-;   movq    %rdi, 16(%rdx)
-;   movq    %rsi, 24(%rdx)
+;   movq    %rsi, 0(%rdx)
+;   movq    %rdi, 8(%rdx)
+;   movq    %rsi, 16(%rdx)
+;   movq    %rdi, 24(%rdx)
 ;   movq    %rdi, 32(%rdx)
-;   movq    %rdi, 40(%rdx)
-;   movq    %rsi, 48(%rdx)
-;   movq    %rdi, 56(%rdx)
-;   movq    %rdi, %rax
-;   movq    %rsi, 64(%rdx)
+;   movq    %rsi, 40(%rdx)
+;   movq    %rdi, 48(%rdx)
+;   movq    %rdi, %rcx
+;   movq    %rsi, 56(%rdx)
 ;   movq    %rsi, %rdx
+;   movq    %rcx, %rax
 ;   movq    %rbp, %rsp
 ;   popq    %rbp
 ;   ret
@@ -1386,17 +1386,17 @@ block0(v0: i128):
 ;   pushq %rbp
 ;   movq %rsp, %rbp
 ; block1: ; offset 0x4
-;   movq %rdi, (%rdx)
-;   movq %rsi, 8(%rdx)
-;   movq %rdi, 0x10(%rdx)
-;   movq %rsi, 0x18(%rdx)
+;   movq %rsi, (%rdx)
+;   movq %rdi, 8(%rdx)
+;   movq %rsi, 0x10(%rdx)
+;   movq %rdi, 0x18(%rdx)
 ;   movq %rdi, 0x20(%rdx)
-;   movq %rdi, 0x28(%rdx)
-;   movq %rsi, 0x30(%rdx)
-;   movq %rdi, 0x38(%rdx)
-;   movq %rdi, %rax
-;   movq %rsi, 0x40(%rdx)
+;   movq %rsi, 0x28(%rdx)
+;   movq %rdi, 0x30(%rdx)
+;   movq %rdi, %rcx
+;   movq %rsi, 0x38(%rdx)
 ;   movq %rsi, %rdx
+;   movq %rcx, %rax
 ;   movq %rbp, %rsp
 ;   popq %rbp
 ;   retq
@@ -1421,12 +1421,10 @@ block0(v0: i128, v1: i128):
 ;   load_ext_name %g+0, %r9
 ;   call    *%r9
 ;   movq    0(%rsp), %r8
-;   movq    8(%rsp), %r9
 ;   addq    %rsp, $16, %rsp
 ;   virtual_sp_offset_adjust -16
-;   movq    %r13, %rcx
-;   movq    %r8, 0(%rcx)
-;   movq    %r9, 8(%rcx)
+;   movq    %r13, %r9
+;   movq    %r8, 0(%r9)
 ;   movq    0(%rsp), %r13
 ;   addq    %rsp, $16, %rsp
 ;   movq    %rbp, %rsp
@@ -1446,11 +1444,9 @@ block0(v0: i128, v1: i128):
 ;   movabsq $0, %r9 ; reloc_external Abs8 %g 0
 ;   callq *%r9
 ;   movq (%rsp), %r8
-;   movq 8(%rsp), %r9
 ;   addq $0x10, %rsp
-;   movq %r13, %rcx
-;   movq %r8, (%rcx)
-;   movq %r9, 8(%rcx)
+;   movq %r13, %r9
+;   movq %r8, (%r9)
 ;   movq (%rsp), %r13
 ;   addq $0x10, %rsp
 ;   movq %rbp, %rsp
diff --git a/cranelift/filetests/filetests/isa/x64/llvm-abi-option-u128.clif b/cranelift/filetests/filetests/isa/x64/llvm-abi-option-u128.clif
new file mode 100644
index 000000000..c777f9848
--- /dev/null
+++ b/cranelift/filetests/filetests/isa/x64/llvm-abi-option-u128.clif
@@ -0,0 +1,36 @@
+test compile precise-output
+set enable_llvm_abi_extensions=1
+target x86_64
+
+function u0:0(i64) -> i64, i128 system_v {
+block0(v0: i64):
+    v1 = iconst.i64 0x1000_0000_0000_0000
+    v2 = iconst.i64 0x2000_0000_0000_0000
+    v3 = iconcat v1, v2
+    v6 = iconst.i64 1
+    return v6, v3
+}
+
+; VCode:
+;   pushq   %rbp
+;   movq    %rsp, %rbp
+; block0:
+;   movabsq $1152921504606846976, %rdx
+;   movabsq $2305843009213693952, %rcx
+;   movl    $1, %eax
+;   movq    %rbp, %rsp
+;   popq    %rbp
+;   ret
+;
+; Disassembled:
+; block0: ; offset 0x0
+;   pushq %rbp
+;   movq %rsp, %rbp
+; block1: ; offset 0x4
+;   movabsq $0x1000000000000000, %rdx
+;   movabsq $0x2000000000000000, %rcx
+;   movl $1, %eax
+;   movq %rbp, %rsp
+;   popq %rbp
+;   retq
+
diff --git a/crates/test-programs/src/bin/piped_multiple.rs b/crates/test-programs/src/bin/piped_multiple.rs
new file mode 100644
index 000000000..29c1ff414
--- /dev/null
+++ b/crates/test-programs/src/bin/piped_multiple.rs
@@ -0,0 +1,34 @@
+use test_programs::wasi::cli::{stdin, stdout};
+
+fn main() {
+    match std::env::var("PIPED_SIDE")
+        .expect("piped tests require the PIPED_SIDE env var")
+        .as_str()
+    {
+        "PRODUCER" => producer(),
+        "CONSUMER" => consumer(),
+        side => panic!("unknown piped test side: {side}"),
+    }
+}
+
+const CHUNK: &[u8] = &[b'a'; 50];
+
+fn producer() {
+    let out = stdout::get_stdout();
+    let n = out.check_write().unwrap() as usize;
+    assert!(n > CHUNK.len());
+    out.write(CHUNK).unwrap();
+}
+
+fn consumer() {
+    let stdin = stdin::get_stdin();
+    let stdin_pollable1 = stdin.subscribe();
+    let stdin_pollable2 = stdin.subscribe();
+
+    // The two pollables are subscribed to the same resource, and must report the same readyness
+    stdin_pollable1.block();
+    assert!(stdin_pollable1.ready() && stdin_pollable2.ready());
+
+    let bytes = stdin.read(CHUNK.len() as u64).unwrap();
+    assert_eq!(&bytes, CHUNK);
+}
diff --git a/tests/all/piped_tests.rs b/tests/all/piped_tests.rs
index 95f007d04..b4bda7ce3 100644
--- a/tests/all/piped_tests.rs
+++ b/tests/all/piped_tests.rs
@@ -2,34 +2,26 @@
 
 use super::cli_tests::get_wasmtime_command;
 use anyhow::Result;
-use std::process::{ChildStdout, Command, Stdio};
+use std::process::Stdio;
 
-fn producer_command(component_path: &str) -> Result<Command> {
-    let mut cmd = get_wasmtime_command()?;
-    cmd.arg("run")
+pub fn run_wasmtime_piped(component_path: &str) -> Result<()> {
+    let mut producer = get_wasmtime_command()?
+        .arg("run")
         .arg("-Wcomponent-model")
         .arg("--env")
         .arg("PIPED_SIDE=PRODUCER")
         .arg(component_path)
-        .stdout(Stdio::piped());
-    Ok(cmd)
-}
+        .stdout(Stdio::piped())
+        .spawn()?;
 
-fn consumer_command(component_path: &str, input: ChildStdout) -> Result<Command> {
-    let mut cmd = get_wasmtime_command()?;
-    cmd.arg("run")
+    let mut consumer = get_wasmtime_command()?
+        .arg("run")
         .arg("-Wcomponent-model")
         .arg("--env")
         .arg("PIPED_SIDE=CONSUMER")
         .arg(component_path)
-        .stdin(input);
-    Ok(cmd)
-}
-
-pub fn run_wasmtime_piped(component_path: &str) -> Result<()> {
-    let mut producer = producer_command(component_path)?.spawn()?;
-    let stdout = producer.stdout.take().unwrap();
-    let mut consumer = consumer_command(component_path, stdout)?.spawn()?;
+        .stdin(producer.stdout.take().unwrap())
+        .spawn()?;
 
     let producer = producer.wait()?;
     if !producer.success() {
@@ -65,6 +57,11 @@ mod test_programs {
         run_wasmtime_piped(PIPED_SIMPLE_COMPONENT).unwrap()
     }
 
+    #[test]
+    fn piped_multiple() {
+        run_wasmtime_piped(PIPED_MULTIPLE_COMPONENT).unwrap()
+    }
+
     #[test]
     fn piped_polling() {
         run_wasmtime_piped(PIPED_POLLING_COMPONENT).unwrap()