-
-
Notifications
You must be signed in to change notification settings - Fork 267
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
Default to rv64gc on riscv64 #4390
Conversation
I have no opinion/expertise on this, but we shouldn't be switching back and forth between defaults every release. @zyedidia @andreas-schwab Please add to this PR a changelog/release notes entry about this. Possibly related: llvm/llvm-project#61991 |
No strong opinion from my side either, but I think the default should account for all features the D language mandates, e.g., double-precision floating point support etc. |
Fedora's riscv64 appears to be built using --with-arch=rv64gc --with-abi=lp64d (taken from gcc configure invocation) so from my point of view, this PR seems like a step in the right direction. |
As long as we have options to change the enabled extensions and abi (which we do as far as I know) then I think this default is reasonable and consistent with other toolchains. Ideally I think we would have the |
I tried to amend this as |
Made a silly typo, retesting. |
@@ -120,6 +127,10 @@ const char *getABI(const llvm::Triple &triple) { | |||
case llvm::Triple::ppc64le: | |||
return "elfv2"; | |||
case llvm::Triple::riscv64: | |||
if (hasFeature("d")) | |||
return "lp64d"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So a -mattr=-d
would get me a lp64d
ABI; is that intended? Or should it be narrowed to a +d
/+f
only?
An entry in CHANGELOG.md
would be nice too. :)
Also select the correct ABI by default matching the enabled features (double, float or no floating point). Fixes #4375
Thx Andreas! |
Fixes #4375