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

"automatic va_arg instruction only works on Darwin" when running AArch64 tests #72579

Closed
pietroalbini opened this issue May 25, 2020 · 3 comments
Closed

"automatic va_arg instruction only works on Darwin" when running AArch64 tests #72579

pietroalbini opened this issue May 25, 2020 · 3 comments
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. O-Arm Target: 32-bit Arm processors (armv6, armv7, thumb...), including 64-bit Arm in AArch32 state T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@pietroalbini
Copy link
Member

When running the rustc test suite on AArch64, the ui/abi/variadic-ffi.rs test fails due to the following LLVM assertion:

rustc: /checkout/src/llvm-project/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:5726: llvm::SDValue llvm::AArch64TargetLowering::LowerVAARG(llvm::SDValue, llvm::SelectionDAG&) const: Assertion `Subtarget->isTargetDarwin() && "automatic va_arg instruction only works on Darwin"' failed.
@pietroalbini pietroalbini added O-Arm Target: 32-bit Arm processors (armv6, armv7, thumb...), including 64-bit Arm in AArch32 state T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. labels May 25, 2020
@jonas-schievink jonas-schievink added 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. labels May 25, 2020
@jhass
Copy link

jhass commented Jun 4, 2020 •
edited
Loading

Here's how Clang implements va_arg on AArch64: https://github.com/llvm/llvm-project/blob/a574edbba2b24fcfb733aa2d82308131f5b7d2d6/clang/lib/CodeGen/TargetInfo.cpp#L5677-L5921

I think it's best to just disable va_arg on AArch64 (and quite possibly more targets) until LLVM decides to properly implement it. Don't get your hopes up on that though, when I asked in IRC why clang would do this rather than fixing va_arg on AArch64, I got comments in the vein of:

<jhass> it almost feels like va_arg is just broken on aarch64 and rather than fixing that somebody went through the effort of reimplemnting it like this for clang
<nbjoerg> no
<nbjoerg> the IR stuff is really only appropiate and meant for the easy cases
<jhass> a bunch of i32's is an easy case, no? I can't get that to work
<nbjoerg> I don't know the aarch64 ABI from memory
<nbjoerg> in general, the IR is only really applicable if you have a nice stack based calling convention
<jhass> well, the call instruction seems to be implemented fine for the aarch64 variadic calling convention?
<jhass> why is it inappropriate for the va_arg instruction to handle it as well?
<jhass> why does it exist then?
<nbjoerg> jhass: legacy
<nbjoerg> jhass: noone bothered enough to remove it

@pietroalbini
Copy link
Member Author

cc @Stammark @JamieCunliffe @vigoux @joaopaulocarreiro @raw-bin, this is one of the tests that are failing on AArch64.

@JamieCunliffe JamieCunliffe mentioned this issue Jun 23, 2020
Merged
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 22, 2020
@Manishearth
Rollup merge of rust-lang#73655 - JamieCunliffe:jamie_va-args-c, r=nikic
233dcac 
va_args implementation for AAPCS.

Implement the va args in codegen for AAPCS, this will be used as the
default va_args implementation for AArch64 rather than the va_args
llvm-ir as it currently is.

This should fix the following issues:
rust-lang#56475
rust-lang#72579
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 22, 2020
@Manishearth
Rollup merge of rust-lang#73655 - JamieCunliffe:jamie_va-args-c, r=nikic
fc5b8c3 
va_args implementation for AAPCS.

Implement the va args in codegen for AAPCS, this will be used as the
default va_args implementation for AArch64 rather than the va_args
llvm-ir as it currently is.

This should fix the following issues:
rust-lang#56475
rust-lang#72579
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 22, 2020
@Manishearth
Rollup merge of rust-lang#73655 - JamieCunliffe:jamie_va-args-c, r=nikic
dade0f1 
va_args implementation for AAPCS.

Implement the va args in codegen for AAPCS, this will be used as the
default va_args implementation for AArch64 rather than the va_args
llvm-ir as it currently is.

This should fix the following issues:
rust-lang#56475
rust-lang#72579
@pietroalbini
Copy link
Member Author

This was fixed!

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. O-Arm Target: 32-bit Arm processors (armv6, armv7, thumb...), including 64-bit Arm in AArch32 state T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@jhass @jonas-schievink @pietroalbini