This is an archive of the discontinued LLVM Phabricator instance.

[ARM] Add big-endian version of pcrel fixups for adr instructions
ClosedPublic

Authored by dim on May 12 2020, 3:34 AM.

Details

Summary

In 2e24219d3cbf, a number of ARM pcrel fixups were resolved at assembly
time, to solve PR44929. This only covered little-endian ARM however, so
add similar fixups for big-endian ARM. Also extend the test case to
cover big-endian ARM.

Diff Detail

Event Timeline

dim created this revision.May 12 2020, 3:34 AM
psmith accepted this revision.May 12 2020, 7:48 AM

Looks good to me. The intention is that the little and big-endian table flags match, only the Offset is expected to change for big-endian, and this change does make sure that the flags match. Will be worth waiting a day or so to see if there are any further comments.

This revision is now accepted and ready to land.May 12 2020, 7:48 AM
dim added a comment.May 12 2020, 9:08 AM

Note I already merged this fix (minus the test case) into FreeBSD here: https://svnweb.freebsd.org/changeset/base/360697, since we needed it ASAP for being able to build big-endian ARM kernel code. So in that sense it's already been exercised for a bit. :)

MaskRay accepted this revision.May 12 2020, 9:29 AM

In the title, s/\[arm]/[ARM]/

In D79774#2031755, @dim wrote:

Note I already merged this fix (minus the test case) into FreeBSD here: https://svnweb.freebsd.org/changeset/base/360697, since we needed it ASAP for being able to build big-endian ARM kernel code. So in that sense it's already been exercised for a bit. :)

MFC after: immediately, since this fix is meant for stable/11

I still think the offending FreeBSD assembly can be fixed to be more portable.

dim retitled this revision from [arm] Add big-endian version of pcrel fixups for adr instructions to [ARM] Add big-endian version of pcrel fixups for adr instructions.May 12 2020, 10:31 AM
dim added a comment.May 12 2020, 10:34 AM
In D79774#2031755, @dim wrote:

Note I already merged this fix (minus the test case) into FreeBSD here: https://svnweb.freebsd.org/changeset/base/360697, since we needed it ASAP for being able to build big-endian ARM kernel code. So in that sense it's already been exercised for a bit. :)

MFC after: immediately, since this fix is meant for stable/11

I still think the offending FreeBSD assembly can be fixed to be more portable.

It's really up to the FreeBSD ARM maintainers, and they seemed to be rather unhappy with the required changes. Then again this particular fix is only meant for the upcoming FreeBSD 11.4 release, which will probably be the last one to support big-endian ARM. So if this whole piece of code is refactored later on, it will most likely not matter anymore.

This revision was automatically updated to reflect the committed changes.