Skip to content

Add divmod functions to RuntimeLibcalls.def #68462

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

boomshroom
Copy link

@boomshroom boomshroom commented Oct 7, 2023

Partially addresses #46350 by adding __{,u}divmod{s,d,t}i4 entries to RuntimeLibcalls.def. These functions exist in compiler-rt and appear to exist in libgcc, so it should be safe to emit calls to them. __{,u}divmod{q,h}i4 are not included due to not having generic implementations in compiler-rt (though they are enabled for specific targets for which they have optimized implementations).

__{,u}divmodti4 do not appear to be built for 32-bit platforms, though neither are __{,u}{div,mod}ti3, but they still have RuntimeLibcalls.def entries despite that.

This does not include optimizations (or de-optimizations) to better prepare the code generator to emit calls to these, but is instead meant to serve as a first step to optimizing these kinds of situations.

@RKSimon
Copy link
Collaborator

RKSimon commented Mar 19, 2025

@boomshroom Are you still looking at this?

@boomshroom
Copy link
Author

If I remember correctly, I think someone once said that they were intentionally left blank globally due to potential ABI incompatibilities with the two-output functions. At the time I made the PR, it also wouldn't have fully resolved #131685 and #46350, since it only adds the symbols, but for integers wider than the target platform natively supports, divmod got broken up into div+mul+sub before emitting these symbols. As a result, they only actually had an affect on RISC-V systems that lacked M, since they were using bit-widths supported by the target, so the prior transformation didn't happen even though they didn't have divide instructions. This PR was intended as a first-step to resolving those issues and would need a follow-up to avoid div+mul+sub. #58212 is a related issue that this issue wouldn't directly help with, but the fix to that would when combined with this PR resolve the other issues.

I haven't been looking at LLVM recently, so I don't know how much has changed since I first posted this, and the fact that the patch is so small made me think it shouldn't have taken very long to merge, but here we are a year and a half later and the same issue this was meant to start addressing has been re-reported.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants