c-variadic: multiple ABIs in the same program for arm #144541
Conversation
rustbot
added
A-compiletest
|
Some changes occurred in src/tools/compiletest cc @jieyouxu |
|
@bors2 try |
…bis-arm, r=<try> c-variadic: multiple ABIs in the same program for arm try-job: armhf-gnu
| @@ -959,6 +959,7 @@ const KNOWN_DIRECTIVE_NAMES: &[&str] = &[ | |||
| "only-aarch64-unknown-linux-gnu", | |||
| "only-apple", | |||
| "only-arm", | |||
| "only-arm-unknown-linux-gnueabihf", | |||
There was a problem hiding this comment.
@jieyouxu is this the right thing to do here?
There was a problem hiding this comment.
To let more targets through (as requested below), we can instead allow only-eabihf. That is not perfect, because there are aebi (no hf) targets that still do support executing floating point instructions, but it at least casts a wider net.
Maybe ignore-softfloat could work. I'm not sure how to check for that actually working, from what I can tell the arm targets generally still support floating point instructions, even if they don't pass arguments via fp registers.
There was a problem hiding this comment.
is this the right thing to do here?
This depends on why you need to have the test run on only-arm-unknown-linux-gnueabihf specifically. I think it's fine if we don't really have a good way to partition the targets as @folkertdev describes.
Also yeah, if this is actually about targets with hard float support, maybe something based on that would be better.
There was a problem hiding this comment.
What I meant is, do we just have to add this here? I am surprised there's no new code implementing the logic for this filter.^^
There was a problem hiding this comment.
What I meant is, do we just have to add this here? I am surprised there's no new code implementing the logic for this filter.^^
Yes, that would be sufficient. This filter is only meant to catch unknown directives (I haven't gotten around to getting rid of it yet), but compiletest actually already have logic to accept //@ only-$target_tuple for all target tuples.
| @@ -0,0 +1,62 @@ | |||
| #![feature(extended_varargs_abi_support)] | |||
| //@ run-pass | |||
| //@ only-arm-unknown-linux-gnueabihf | |||
There was a problem hiding this comment.
Why only that one specific target? (And not all arm targets, or so)
There was a problem hiding this comment.
the assembly needs hardware floats, and we're using floats because that is one case where we know aapcs and C are different. We don't have a nice way of filtering for hardware float. Also this is a target that actually runs on CI.
There was a problem hiding this comment.
we're using floats because that is one case where we know aapcs and C are different
Please always put such choices into comments! The chance that someone else reading the code will realize this without a comment is probably negligible.
There was a problem hiding this comment.
this is a target that actually runs on CI.
That may change though...
Would it work to enable the test on all ARM targets, and then inside the test use a cfg!(target_abi) to neuter it when we don't have floats? Then at least if CI switches to some other arm eabihf target the test will still run.
rustbot
added
the
O-Arm
|
Hey ARM Group! This bug has been identified as a good "ARM candidate". cc @adamgemmell @davidtwco @hug-dev @jacobbramley @JamieCunliffe @joaopaulocarreiro @raw-bin @Stammark |
folkertdev
force-pushed
the
c-variadic-same-program-multiple-abis-arm
branch
from
7c6fa6a to
5440cce
Compare
similar to #144379, but for arm, requested in #144066.
It looks like
aapcsandCare in fact the same for variadic functions on 32-bit arm using hardware floats. For non-variadic functions, there are differences between these ABIs.https://github.com/ARM-software/abi-aa/blob/main/aapcs32/aapcs32.rst#65parameter-passing
https://github.com/ARM-software/abi-aa/blob/main/aapcs32/aapcs32.rst#7the-standard-variants
I also noticed that rustc currently emit more instructions than clang for c-variadic functions on arm, see https://godbolt.org/z/hMce9rnTh. I'll fix that separately.
try-job: armhf-gnu
r? @RalfJung