-
Notifications
You must be signed in to change notification settings - Fork 14k
Split LLVM intrinsic abi handling from the rest of the abi handling #148533
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.
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
base: main
Are you sure you want to change the base?
Conversation
Intrinsics only need a fraction of the functionality offered by BuilderMethods::call and in particular don't need the FnAbi to be computed other than (currently) as step towards computing the function value type.
|
Some changes occurred in compiler/rustc_codegen_gcc Some changes occurred in compiler/rustc_codegen_ssa |
|
r? @SparrowLii rustbot has assigned @SparrowLii. Use |
|
this is awesome, thanks ❤️. I tested compiling stdarch with this branch (for x86_64-unknown-linux-gnu, aarch64-unknown-linux-gnu and riscv64gc-unknown-linux-gnu targets), it doesn't generate any errors |
|
Thanks for testing! I hadn't thought about testing this on stdarch. Did you also test compiling the stdarch tests? Otherwise no llvm intrinsic calls are actually getting codegened I think due to all |
|
yes, I did |
|
@bjorn3 could you see if it's possible to split the function declaration of LLVM intrinsics too - after inspecting my PR I noticed that it modifies declaration more than calling. Thanks |
|
I figured I would do that after this PR gets merged, but I can do it somewhere in the next couple of days too. In this PR if it hasn't been reviewed by then and otherwise in a separate PR. |
This moves all LLVM intrinsic handling out of the regular call path for cg_gcc and makes it easier to hook into this code for future cg_llvm changes.
|
Something like this @sayantn? @antoyo @GuillaumeGomez would you mind reviewing the cg_gcc changes? |
|
Thanks, It's nice, but do we need to cache the intrinsic instances? |
This does look good to me, but I would need to run the tests of cg_gcc to confirm. |
|
|
Isn't that only running UI tests? |
|
No, any testsuite with this argument will use the GCC backend. |
I meant that this won't run the stdarch tests or other tests we have in the CI of the cg_gcc repo. |
|
Maybe you could build rustc with cg_gcc as default backend and then manually run the stdarch test suite using it? |
|
Ah sorry, the current design looks nice. I will test it on stdarch locally when I get some time |
I tried to build your branch locally and it seems it broke a stage-2 build with cg_gcc. |
|
Does it work with the last commit reverted? |
LLVM intrinsics have weird requirements like requiring the fake "unadjusted" abi, not being callable through function pointers and for all codegen backends other than cg_llvm requiring special cases to redirect them to the correct backend specific intrinsic (or directly codegen their implementation inline without any intrinsic call). By splitting the LLVM intrinsic handling it becomes easier for backends to special case them and should in the future allow getting rid of the abi calculation for
extern "unadjusted"in favor of computing the correct abi directly in the backend without depending on the exact way cg_ssa lowers types.