Open AaronKutch opened 3 years ago
@Aaronkutch is this specific to (u128, u128)? Or is it an issue for all tuples of integers?
I don't know because 32-bit SPARC is not a target I can easily download and test from rustup. Arrays and other structures should be tested.
My guess is that the return value ends up getting passed via registers. If that's true, the fix can be as easy as adjusting the ABI code to pass the return value indirectly.
I'm also quite surprised the first thing attempted in working around the issue in compiler-builtins
was not changing to a different calling convention.
lowering that leads to the same IR that clang generates
clang's behaviour is irrelevant for "Rust"
calling convention. There shouldn't be a goal of matching the behaviour of clang
for anything but extern "C"
and other well known calling conventions.
I know the basics around calling conventions in assembly, but I'm not familiar with the implementation details in Rust and LLVM. Shouldn't LLVM be pushing values onto the stack instead of giving up when it realizes that the registers cannot hold all the values? I can define a function
pub fn test(x: u128) -> (u128, u128, u128, u128, u128, u128, u128, u128, u128, u128, u128, u128, u128, u128)
and it will compile just fine on sparc64. I can look at the assembly and see it moving a bunch of values to the stack. There should be no problems except for running out of memory for the stack at runtime or the program not being able to be loaded into memory. There might be some special things surrounding extern
functions that I don't know about, but the failing return allocations are happening on a perfectly normal Rust signature fn f(x: u128, y: u128) -> (u128, u128)
.
What I mean by lowering to the same IR that clang generates is that Rust's IR relies on LLVM dealing with it:
ret { i128, i128 } %1
clang's IR explicitly puts the value on the stack for LLVM:
store i128 2, i128* %3, align 16, !tbaa !7
ret void
So this would be a way of fully supporting sparc-unknown-linux-gnu
until the underlying LLVM issue is fixed on all supported LLVM versions.
I'm also quite surprised the first thing attempted in working around the issue in compiler-builtins was not changing to a different calling convention.
What do you mean by changing calling convention? The only way of forcing custom calling conventions is to use assembly in #[naked]
functions. We don't want to use very weird hacks if we add functions of the form fn f(x: u128, y: u128) -> (u128, u128)
and also want to support sparc-unkown-linux-gnu
.
What do you mean by changing calling convention?
Well, if the functions in question are internal implementation details, you can mark them as extern "C"
extern "C" fn test_128(one: u128, two: u128) -> (u128, u128) { ... }
which should in principle give you the same ABI (and LLVM IR) as what clang uses in the first place.
I suppose that would have also worked, but still an ugly workaround for just one edge case, the root of which should be fixed. I should mention that the choice I made in compiler-builtins
is more of a permanent workaround. SPARC is an awful architecture to optimize several classes of algorithms for because it does not have widening multiplication. Widening multiplication is very important for the performance of the default division algorithm choices made in compiler-builtins
. Originally, I didn't want to waste any time on an architecture I didn't want to care about, but when the 32 bit sparc issue came around I decided to knock out two birds with one stone and make a special codepath for all SPARC architectures.
Filled bugs for the LLVM asserts upstream https://bugs.llvm.org/show_bug.cgi?id=48698 and https://bugs.llvm.org/show_bug.cgi?id=48699 but we should still adjust ABI code in Rust.
Filled bugs for the LLVM asserts upstream https://bugs.llvm.org/show_bug.cgi?id=48698 and https://bugs.llvm.org/show_bug.cgi?id=48699 but we should still adjust ABI code in Rust.
48698 appears to have been migrated to Github as https://github.com/llvm/llvm-project/issues/48042 and has been closed 48699 appears to have been migrated to Github as https://github.com/llvm/llvm-project/issues/48043 and is still open
Some time ago,
compiler-builtins
ran into an issue where thesparc-unknown-linux-gnu
target was unable to handle functions that return(u128, u128)
. I made a temporary workaround incompiler-builtins
and said that I would try to fix the underlying issue over winter break. There are currently two proposals for adding functions that return tuples of integers, https://github.com/rust-lang/rfcs/issues/914 and I remember something about widening multiplication that returns a tuple (but I can't find it for some reason). The workaround I made incompiler-builtins
will not work in those cases, since the function returning a tuple is directly in the public interface. Sincesparc-unknown-linux-gnu
is a tier 2.5 target important to some people, we need to fix the issue in LLVM ahead of time or implement some kind of lowering that leads to the same IR that clang generates. Unfortunately, I got distracted by other things and will not have time to dive into LLVM. I am opening this issue so that someone with more experience with LLVM internals can fix the issue. More details are in https://github.com/rust-lang/compiler-builtins/issues/390 and https://github.com/rust-lang/rfcs/issues/914 .