-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
compiler_rt: Update Windows ABI for float<->int conversion routines #12071
Conversation
1b50749
to
c8d2f5f
Compare
Great work - really appreciate your help with the LLVM stuff lately 🙏 |
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.
I believe the failure is due to codegen/llvm.zig emitting explicit calls to compiler_rt functions in the cases where LLVM will fail to properly do so - meaning that this function signature must be adjusted in codegen.cpp and codegen/llvm.zig to match.
Are you up for those changes too? Let me know if you want some help.
Happy to lend a hand! These are fun bugs to chase for now 🐞 |
Very good point - should have thought of our explicitly-lowered calls. Let me take a stab and see how far I can get. I'll reach out if it gets hairy. |
c8d2f5f
to
1529092
Compare
Starting with LLVM 14, the Libcalls to these functions are now lowered using a Vec(2, u64) instead of the standard ABI for i128 integers, so our compiler-rt implementation needs to be updated to expose the same ABI on Windows.
This is just a cosmetic change. The goal is to keep the export logic relatively flat and centralized.
This change is the Zig counterpart to https://reviews.llvm.org/D110413 Since we lower some libcalls directly (just like clang does), we need to make sure that the ABI we call with matches the ABI of the compiler-rt we are providing (and also the ABI expected by LLVM). While I was at it, I noticed some flawed vector handling in the binary soft float ops in stage 1, so I shored up the logic a bit and expanded an existing test to cover the missing functionality.
This change is the Zig counterpart to https://reviews.llvm.org/D110413 Same as the prior commit, but for stage2
1529092
to
2b52154
Compare
Resolves #12063
Starting with LLVM 14, the Libcalls to these functions are now lowered using a
Vector(2, u64)
instead of the standard ABI for i128 integers on Windows, so our compiler-rt implementation needs to be updated to expose the same ABIRelevant LLVM revision: https://reviews.llvm.org/D110413