Attempt to fix PPC/PPC64 ABI issues#1905
Conversation
Issue ldc-developers#1652 seems to be caused by ABI errors in the bootstrap compiler. This PR updates the PPC/PPC64 ABI to conform to clang/gcc output.
|
Kai mentioned that LDC was able to compile itself back then, but that one test still failed. I assume it's the one wrt. reversed sorting, which was also a problem for ARM as long as the arguments order wasn't reversed for |
|
Note that #1680 was merged, i.e., Kai's fixes are in ltsmaster, but not in master. |
| void rewriteFunctionType(TypeFunction *tf, IrFuncTy &fty) override { | ||
| if (!fty.ret->byref) { | ||
| rewriteArgument(fty, *fty.ret); | ||
| } |
There was a problem hiding this comment.
Note that MIPS64TargetABI::rewriteArgument() is empty and needs to be implemented, so this ABI has probably never been properly tested. Almost all ABIs rewrite a non-sret return value just like a regular argument of the same type.
| if (!arg->byref) | ||
| rewriteArgument(fty, *arg); | ||
| else if (passByVal(arg->type)) | ||
| arg->attrs.remove(LLAttribute::ByVal); |
There was a problem hiding this comment.
This should never happen; IIRC, LLVM's byval attribute is never considered for args passed by ref.
There was a problem hiding this comment.
Oh not so sure anymore, I think IrFuncTyArg::byref is set if the arg is to be passed with LLVM's byval attribute (passByVal(arg->type) == true), which means that that decision is overridden here, clearing the byval attribute but leaving IrFuncTyArg::byref intact, so that a D argument passed by value for which passByVal(arg->type) == true is actually passed by reference!
Most importantly, make sure sret is enforced for all non-POD structs as done for all other ABIs, and don't treat extern(D) any different from the C ABI wrt. return values anymore. Also reduce code duplication via common TargetABI patterns.
|
@joakim-noah, @smolt: It would be nice if you could verify everything still works. |
|
Compiling ltsmaster natively on Android/ARM with the ARM portion of this PR, then will try the full PR with master. That'll cover ARM soft-float, somebody with an armhf board will have to test that separately. |
|
I tested the pull request and it fixes the ppc64 issue, #1904 for me. Thanks! I ran it through Fedora's armv7hl, x86_64, ppc64le, ppc64, i686 builders and ldc can successfully build itself on all of these now. |
|
Excellent, thank you very much! |
|
Full stdlib and dmd testsuite passes with this PR on Android/ARM with ltsmaster and master. |
Why would that be, though? |
|
See #865 and dlang/dmd#5232 as to why druntime and the compiler itself currently depend on the reversed order. |
5 participants
Continuation of #1683.