-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
compiler rt function muldi3 / mulsi3 are incorrectly generated with mutual recursion #3275
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
Comments
Reported upstream: https://bugs.llvm.org/show_bug.cgi?id=43388 |
I believe this is related to #2051. Currently when you specify riscv64, this is "baseline" which does not have certain useful features, such as:
even though nearly all riscv boards do have these features. I think zig needs to make it easier to select the more common boards, and more obvious what selections one is making as part of the target. For example if the target indicated that 64 bit multiplication is available - which it almost certainly is - then it would side-step this issue. Still an issue though. |
On Linux, clang defaults riscv64 features to "+a,+c,+d,+f,+m,+relax", which side-steps this issue. |
Until ability to specify target CPU features (#2883) is done, this commit gives riscv target better default features. This side-steps #3275 which is a deficiency in compiler-rt when features do not include 32 bit integer division. With this commit, RISC-V compiler-rt tests pass and Hello World works both pure-zig and with musl libc.
After 53210b2, this issue is not encountered (however, it should remain open until the bug in compiler-rt is fixed). After that commit RISC-V compiler-rt tests pass and Hello World works both pure-zig and with musl libc. The next step for RISC-V is behavior tests, which are failing with TODO C ABI panic. |
Yes, when targeting Linux the "GC" extensions should be the default (shorthand for IMAFDC). When targeting a bare metal RV64 it arguably makes sense to not assume any extensions by default, although the 64-bit processors are indeed very likely to include the standard ones. In any case, this issue should still be fixed on the compiler-rt side. |
LLVM bug has long since been fixed. We'll now want to port this over to Zig: https://github.com/llvm/llvm-project/tree/6c331e50e4bfb4158d16ec3fe17ad7bb5c739e9f/compiler-rt/lib/builtins/riscv |
To reproduce:
output:
Segmentation fault (core dumped)
with gdb
Backtrace shows:
It's unclear whether this is an upstream issue or not. The next step is to try to produce a reduction to demonstrate an LLVM bug.#3275 (comment)Reported by @LemonBoy #3260 (review)
The text was updated successfully, but these errors were encountered: