This is an archive of the discontinued LLVM Phabricator instance.

[ELF] Support a few more SPARCv9 relocations
ClosedPublic

Authored by LemonBoy on Apr 7 2020, 12:10 PM.

Details

Summary

Implemented a bunch of relocations found in binaries with medium/large code model and the Local-Exec TLS model. The binaries link and run fine in Qemu.
In addition, the emulation elf64_sparc is now recognized.

Diff Detail

Event Timeline

LemonBoy created this revision.Apr 7 2020, 12:10 PM

My immediate reaction is "isn't Sparc an abandoned architecture?" See https://en.wikipedia.org/wiki/SPARC and https://lists.freebsd.org/pipermail/freebsd-sparc64/2020-January/010192.html

Though, no objection. You may still need some basic tests, similar to test/ELF/ppc32-* I tried hard to keep the number of tests small yet complete. Does sparc64 use TLS variant 2?

jrtc27 added a comment.Apr 7 2020, 3:16 PM

Just as a note for things that are still missing:

  • GD/LD/IE TLS (you mentioned IE TLS, but this only does *LE*)
  • The HIX22/LOX10 (and others) family for GOTDATA that replaces R_SPARC_GOT22/10
  • R_SPARC_REGISTER, needed to mark how each of the %g[2367] (could have the exact numbers wrong, they're from deep in my memory) are being used by the object files to as to ensure they don't conflict. It's a very unusual relocation (refers to magic STT_SPARC_REGISTER symbols) that should have designed using something other than a relocation...

There may be others, but as one of the SPARC porters in Debian that's what I can think of off the top of my head.

jrtc27 added a comment.Apr 7 2020, 3:21 PM

My immediate reaction is "isn't Sparc an abandoned architecture?" See https://en.wikipedia.org/wiki/SPARC and https://lists.freebsd.org/pipermail/freebsd-sparc64/2020-January/010192.html

Though, no objection. You may still need some basic tests, similar to test/ELF/ppc32-* I tried hard to keep the number of tests small yet complete. Does sparc64 use TLS variant 2?

For FreeBSD, yes, and mostly within LLVM (certainly nobody really uses LLD). The non-release Debian architecture is still going strong though. SPARC (32-bit and 64-bit) is basically the same as IA-32 (although not with the weird calling convention and triple leading underscores), and one of the architectures described in Drepper's original TLS white paper, so yes, variant II.

My immediate reaction is "isn't Sparc an abandoned architecture?" See https://en.wikipedia.org/wiki/SPARC and https://lists.freebsd.org/pipermail/freebsd-sparc64/2020-January/010192.html

Though, no objection. You may still need some basic tests, similar to test/ELF/ppc32-* I tried hard to keep the number of tests small yet complete. Does sparc64 use TLS variant 2?

For FreeBSD, yes, and mostly within LLVM (certainly nobody really uses LLD). The non-release Debian architecture is still going strong though. SPARC (32-bit and 64-bit) is basically the same as IA-32 (although not with the weird calling convention and triple leading underscores), and one of the architectures described in Drepper's original TLS white paper, so yes, variant II.

If the intention is improve Sparc, the integrated assembler issues (rL254199) should be addressed.

https://lists.llvm.org/pipermail/llvm-dev/2017-June/114437.html mentioned a plan to revive the support but I believe the situation has not been improved... The codegen maintainer has moved on (left) since 2014..

ruiu added a comment.Apr 7 2020, 8:54 PM

Improving SPARC support is fine as long as we don't need to do something special for the ISA. But since no active maintainers have a SPARC machine, it is hard to find bugs in SPARC. Can you add tests for the new relocations?

jrtc27 added a comment.Apr 7 2020, 9:30 PM

Improving SPARC support is fine as long as we don't need to do something special for the ISA. But since no active maintainers have a SPARC machine, it is hard to find bugs in SPARC. Can you add tests for the new relocations?

For what it's worth, I have access to various SPARC machines, so can test things as needed. Also, the GCC compile farm (https://cfarm.tetaneutral.net/machines/list/) has multiple SPARC machines; in particular, gcc202 is managed by us Debian sparc64 porters, and has a beefy 64 cores and 64 GiB on a T5 (which, unlike some of the earlier UltraSparcs, has decent single-threaded performance too), so it should be easy for maintainers to test things if they're interested enough. The compile farm is a great resource in general that is really easy to sign up for and most don't know about.

ruiu added a comment.Apr 7 2020, 10:22 PM

Improving SPARC support is fine as long as we don't need to do something special for the ISA. But since no active maintainers have a SPARC machine, it is hard to find bugs in SPARC. Can you add tests for the new relocations?

For what it's worth, I have access to various SPARC machines, so can test things as needed. Also, the GCC compile farm (https://cfarm.tetaneutral.net/machines/list/) has multiple SPARC machines; in particular, gcc202 is managed by us Debian sparc64 porters, and has a beefy 64 cores and 64 GiB on a T5 (which, unlike some of the earlier UltraSparcs, has decent single-threaded performance too), so it should be easy for maintainers to test things if they're interested enough. The compile farm is a great resource in general that is really easy to sign up for and most don't know about.

If the SPARC community is interested in lld, it shouldn't be too hard to make lld usable for SPARC, so it is up to the community.

LemonBoy updated this revision to Diff 255941.Apr 8 2020, 3:06 AM
LemonBoy edited the summary of this revision. (Show Details)

I've added a few tests that cover the newly-added relocations plus a few more, R_SPARC_LM22 is not yet tested because LLVM as doesn't recognize it but I already have a patch ready for that.

My immediate reaction is "isn't Sparc an abandoned architecture?"

Well somebody asked if Zig could target sparc64/linux so I guess there's at least a small group of users interested in it. Since we exclusively use lld I decided to fix this roadblock, this patch isn't meant to be exhaustive but focuses on the missing relocations I've found in the binaries produced by Zig.

Does sparc64 use TLS variant 2?

AFAIR it's one of the few non-x86 arches to use it

you mentioned IE TLS, but this only does *LE*

Typo, the relocations also have "LE" in the name :)

LemonBoy updated this revision to Diff 255945.Apr 8 2020, 3:09 AM
brad added a subscriber: brad.Apr 9 2020, 1:42 PM

My immediate reaction is "isn't Sparc an abandoned architecture?" See https://en.wikipedia.org/wiki/SPARC and https://lists.freebsd.org/pipermail/freebsd-sparc64/2020-January/010192.html

Though, no objection. You may still need some basic tests, similar to test/ELF/ppc32-* I tried hard to keep the number of tests small yet complete. Does sparc64 use TLS variant 2?

For FreeBSD that is the case. FreeBSD/sparc64 always seemed to be on life support. Unlike FreeBSD / NetBSD, OpenBSD's sparc64 port is fully supported and supports the latest and greatest with regard to sparc64.

We're very close to switching to Clang as the system compiler on sparc64.

MaskRay added inline comments.Apr 9 2020, 11:06 PM
lld/test/ELF/sparcv9-reloc.s
37

Indenting by 2 should be sufficient.

lld/test/ELF/sparcv9-tls-le.s
6

Add a ## comment how the values are computed.

LemonBoy updated this revision to Diff 256535.Apr 10 2020, 2:36 AM

Can you refer me to the sparc psABI so that I can verify the implementation?

lld/test/ELF/sparcv9-reloc.s
5

objdump -s does not need --no-show-raw-insn.

Can you refer me to the sparc psABI so that I can verify the implementation?

I've followed this document by Oracle, the TLS ones are in chapter 8.

MaskRay added inline comments.Apr 14 2020, 10:41 AM
lld/ELF/Driver.cpp
151

Add a test similar to emulation-arm.s

The other emulation-* have verbose llvm-readobj output. Don't copy them.

MaskRay retitled this revision from Support a few more SPARCv9 relocations to [ELF] Support a few more SPARCv9 relocations.Apr 14 2020, 11:58 AM
LemonBoy updated this revision to Diff 257433.Apr 14 2020, 11:58 AM
LemonBoy updated this revision to Diff 257434.Apr 14 2020, 11:59 AM

I really hate phabricator

MaskRay added inline comments.Apr 14 2020, 12:36 PM
lld/test/ELF/emulation-sparc.s
2 ↗(On Diff #257434)

You can drop -unknown-linux. FreeBSD/OpenBSD's triples don't have the OS part. Without the OS part, it usually means the generic ELF behavior.

For an object file, it is usually better to use %t.o or similar.

For an executable, %t usually suffices. When there are more than one, Use %t1 %t2 ...

(I know that some existing tests don't follow the convention.) And the existing emulation-* do not follow the best practice.

8 ↗(On Diff #257434)

Add -T before a linker script (you may name it %t.lds, shorter than %t.script)

GNU ld warns without -T

10 ↗(On Diff #257434)

If there are no other prefixes, just use the FileCheck default CHECK. You can omit --check-prefix as well.

LemonBoy updated this revision to Diff 257606.Apr 14 2020, 11:54 PM
LemonBoy marked an inline comment as done.

Minor changes to the test case.

MaskRay accepted this revision.Apr 15 2020, 9:19 AM

LGTM.

This revision is now accepted and ready to land.Apr 15 2020, 9:19 AM

Can you (or anyone) commit this on my behalf? I don't have commit access.

lld/test/ELF/emulation-sparc.s
10 ↗(On Diff #257434)

I've picked the V9 prefix as LLD may want to support the 32bit variants some day

MaskRay updated this revision to Diff 258330.Apr 17 2020, 8:05 AM
MaskRay edited the summary of this revision. (Show Details)

Adjust the tests a bit
Update the description

This revision was automatically updated to reflect the committed changes.