Page MenuHomePhabricator

[Sparc] Use the names .rem and .urem instead of __modsi3 and __umodsi3
ClosedPublic

Authored by dcederman on Jul 3 2018, 11:48 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

dcederman created this revision.Jul 3 2018, 11:48 PM
joerg added a subscriber: joerg.Jul 4 2018, 3:43 PM

That would be the milli code version, wouldn't it?

That would be the milli code version, wouldn't it?

It is an implementation of the modulo operation that does not use the div or mul instructions. That way it can also work on a Sparc V7 system which lacks those instructions. It uses normal instructions, so it is not millicode the way I understand the term.

Currently programs compiled for Sparc V7 does not link when using the modulo operator as it looks for the __modsi3 symbol instead of .rem.

joerg added a comment.Jul 5 2018, 4:21 AM

Not what I mean. Certain platforms like Sparc and SH link a copy of certain routines into every DSO. This is the so-called milli code. They sometimes use special calling conventions as well. That's different from the "normal" helper routines in libgcc, which are shared by all libraries.

Do you have an example of milli code for Sparc? I did not have much luck googling the term. Would __sparc_get_pc_thunk.l7 be an example of milli code? GCC generates it as a way of determining the address of the caller and it uses a non-standard calling convention.

From your description I would say that .rem is a normal helper function. It is also part of glibc and NetBSD libc.

joerg added a comment.Jul 9 2018, 7:33 AM

Looking further, at least on NetBSD libgcc seems to always include both the "normal" and the .rem/.urem routines. While we currently don't replace __umodsi3 and friends, that looks more like an oversight on our part.
I asked the Sparc folks and they can't remember any special reason for why .urem should be used. I.e. it follows the normal Sparc ABI.
As such, I'm mostly ambivalent on this change and the rest of the block.

My aim with the patch is to be able to link Sparc V7 code, ie code with no mul/div instructions, with libgcc and newlib. Since gcc references the .urem symbol instead of __umodsi3, I thought that making the same change for LLVM would be the best fix.

Grepping in the usr/lib folder of NetBSD (sparc32) I see many references to .urem, but none to __umodsi3, so I do not think this change will cause any problems for NetBSD users.

jyknight accepted this revision.Jul 13 2018, 2:19 PM

This looks like it was a simple oversight to me, since LLVM was already emitting .umul, .div, and .udiv -- not matching for rem/urem cannot have been intended.

This revision is now accepted and ready to land.Jul 13 2018, 2:19 PM
This revision was automatically updated to reflect the committed changes.