- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 5.7k
Revert "remove hand-coded methods for (U)Int128 on 32 bit systems (#53867)" #59211
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
Conversation
…liaLang#53867)" This reverts commit 13d4f0e. It turns out that LLVM still emits libcalls to `__divti3` in some scenarios (JuliaLang#58911).
| Can we only revert the  | 
| I think we can try that on  Since we were bitten by this over a year after the original PR merged, I'd prefer to do a full revert for  | 
| If llvm is just emitting libcalls here we should just revert the whole thing, it's not like the performance of this matters too much (128 bit on 32bit). The other option is to vendor a C implementation of this and link it in, but LLVM has issues with finding those sometimes | 
| IIUC LLVM is only emitting libcalls here because it is missing the lowering of these operations. The reason I had hoped to leave this to LLVM is I trust them to have better lowerings (e.g. using add with carry functions) which they appear to do for all backends except 32 bit windows. That said, I don't object to a full revert (at least for the backports). | 
| Merged the full revert for now, but please free to re-land a modified version of the change for  | 
| I'm a bit late to the party, but perhaps comments in the code to explain why those methods are still needed would be useful. Commit message has some explanation (great!), but it requires a git blame to find the rationale. | 
We need to revert this for now. Resolves #58911
It seems that the back-end LLVM emits this libcall a lot less often than it used to, but it still happens sometimes and 32-bit Windows has no implementation of
__divti3.