Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

run-make-fulldeps/c-link-to-rust-va-list-fn fails on s390x #84628

Closed
cuviper opened this issue Apr 27, 2021 · 2 comments · Fixed by #105381
Closed

run-make-fulldeps/c-link-to-rust-va-list-fn fails on s390x #84628

cuviper opened this issue Apr 27, 2021 · 2 comments · Fixed by #105381
Labels
A-codegen Area: Code generation A-FFI Area: Foreign function interface (FFI) A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. F-c_variadic `#![feature(c_variadic)]` O-SystemZ Target: SystemZ processors (s390x) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@cuviper
Copy link
Member

cuviper commented Apr 27, 2021 •
edited
Loading

./x.py test run-make-fulldeps fails this test on native s390x-unknown-linux-gnu:

---- [run-make] run-make-fulldeps/c-link-to-rust-va-list-fn stdout ----
error: make failed
status: exit code: 2
command: "make"
stdout:
------------------------------------------
LD_LIBRARY_PATH="/builddir/build/BUILD/rustc-1.51.0-src/build/s390x-unknown-linux-gnu/test/run-make-fulldeps/c-link-to-rust-va-list-fn/c-link-to-rust-va-list-fn:/builddir/build/BUILD/rustc-1.51.0-src/build/s390x-unknown-linux-gnu/stage2/lib:/builddir/build/BUILD/rustc-1.51.0-src/build/s390x-unknown-linux-gnu/stage0-bootstrap-tools/s390x-unknown-linux-gnu/release/deps:/usr/lib" '/builddir/build/BUILD/rustc-1.51.0-src/build/s390x-unknown-linux-gnu/stage2/bin/rustc' --out-dir /builddir/build/BUILD/rustc-1.51.0-src/build/s390x-unknown-linux-gnu/test/run-make-fulldeps/c-link-to-rust-va-list-fn/c-link-to-rust-va-list-fn -L /builddir/build/BUILD/rustc-1.51.0-src/build/s390x-unknown-linux-gnu/test/run-make-fulldeps/c-link-to-rust-va-list-fn/c-link-to-rust-va-list-fn  checkrust.rs
------------------------------------------
stderr:
------------------------------------------
LLVM ERROR: Cannot select: 0x3ff7c04c2e8: f64,ch = vaarg 0x3ff7c00d788, 0x3ff7c04c3b8, SrcValue:ch<0x3ff907d1a40>, TargetConstant:i32<8>
  0x3ff7c04c3b8: i64,ch = CopyFromReg 0x3ff7c00d788, Register:i64 %1
    0x3ff7c04c0e0: i64 = Register %1
  0x3ff7c04be70: i32 = TargetConstant<8>
In function: _ZN4core3ffi10VaListImpl3arg17h292dc5efd67889b3E
make: *** [Makefile:4: all] Error 101
------------------------------------------

(This is not a new failure, but I am cleaning up issues from downstream Red Hat bugzilla.)
@cuviper cuviper added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-codegen Area: Code generation A-FFI Area: Foreign function interface (FFI) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. O-SystemZ Target: SystemZ processors (s390x) F-c_variadic `#![feature(c_variadic)]` labels Apr 27, 2021
@cuviper cuviper mentioned this issue Apr 27, 2021
Open
3 tasks
@cuviper
Copy link
Member Author

cuviper commented Sep 30, 2021

We started discussing this in #44930, and looking at the clang implementation here:

https://github.com/llvm/llvm-project/blob/9ad17fe0debb0b48798aaba6d07b65136fcf0664/clang/lib/CodeGen/TargetInfo.cpp#L7467-L7468

@dlrobertson

We're emitting a void pointer like va_list but it looks like a complex structure is expected. We'll need to change the intrinsic type VaListImpl for the architecture. It would be interesting to see if just changing the structure for s390x fixes this or if we do indeed need to implement a custom va_arg as well.

I tried that minimal step with master...cuviper:c_variadic-s390x, but it still gives the same "Cannot select" error. I don't see any lowering of ISD::VAARG in llvm/lib/Target/SystemZ like other targets, only stuff implementing the calling convention. So I guess we do need custom support, or perhaps the clang bits could be moved down into the LLVM lowering?

@uweigand uweigand mentioned this issue Dec 6, 2022
Merged
@uweigand
Copy link
Contributor

uweigand commented Dec 6, 2022

Hi @cuviper , I ran into the same problem and noticed you had already opened an issue. I've now submitted a PR to implement va_list and va_arg for s390x to fix this.

@uweigand uweigand mentioned this issue Dec 6, 2022
Closed
8 tasks
bors added a commit to rust-lang-ci/rust that referenced this issue Dec 20, 2022
@bors
Auto merge of rust-lang#105381 - uweigand:s390x-ffi-vaarg, r=nikic
d6da428 
Implement va_list and va_arg for s390x FFI

Following the s390x ELF ABI and based on the clang implementation, provide appropriate definitions of va_list in library/core/src/ffi/mod.rs and va_arg handling in compiler/rustc_codegen_llvm/src/va_arg.rs.

Fixes the following test cases on s390x:
src/test/run-make-fulldeps/c-link-to-rust-va-list-fn src/test/ui/abi/variadic-ffi.rs

Fixes rust-lang#84628.
@bors bors closed this as completed in eb22d70 Dec 20, 2022
thomcc pushed a commit to tcdi/postgrestd that referenced this issue May 31, 2023
@uweigand
Implement va_list and va_arg for s390x FFI
b3d2d98 
Following the s390x ELF ABI and based on the clang implementation,
provide appropriate definitions of va_list in library/core/src/ffi/mod.rs
and va_arg handling in compiler/rustc_codegen_llvm/src/va_arg.rs.

Fixes the following test cases on s390x:
src/test/run-make-fulldeps/c-link-to-rust-va-list-fn
src/test/ui/abi/variadic-ffi.rs

Fixes rust-lang/rust#84628.
thomcc pushed a commit to tcdi/postgrestd that referenced this issue May 31, 2023
@bors
Auto merge of #105381 - uweigand:s390x-ffi-vaarg, r=nikic
e0117fe 
Implement va_list and va_arg for s390x FFI

Following the s390x ELF ABI and based on the clang implementation, provide appropriate definitions of va_list in library/core/src/ffi/mod.rs and va_arg handling in compiler/rustc_codegen_llvm/src/va_arg.rs.

Fixes the following test cases on s390x:
src/test/run-make-fulldeps/c-link-to-rust-va-list-fn src/test/ui/abi/variadic-ffi.rs

Fixes rust-lang/rust#84628.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-codegen Area: Code generation A-FFI Area: Foreign function interface (FFI) A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. F-c_variadic `#![feature(c_variadic)]` O-SystemZ Target: SystemZ processors (s390x) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Implement va_list and va_arg for s390x FFI uweigand/rust
2 participants
@cuviper @uweigand