This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] ILP32 Backend Relocation Support
ClosedPublic

Authored by joelkevinjones on Apr 13 2017, 9:01 PM.

Details

Summary

Remove "_NC" suffix and semantics from TLSDESC_LD{64,32}_LO12 and

TLSDESC_ADD_LO12 relocations

Rearrange ordering in AArch64.def to follow relocation encoding
Fix name:

R_AARCH64_P32_LD64_GOT_LO12_NC => R_AARCH64_P32_LD32_GOT_LO12_NC

Add support for several "TLS", "TLSGD", and "TLSLD" relocations for

ILP32

Fix return values from isNonILP32reloc
Add implementations for

R_AARCH64_ADR_PREL_PG_HI21_NC, R_AARCH64_P32_LD32_GOT_LO12_NC,
R_AARCH64_P32_TLSIE_LD32_GOTTPREL_LO12_NC,
R_AARCH64_P32_TLSDESC_LD32_LO12, R_AARCH64_LD64_GOT_LO12_NC,
*TLSLD_LDST128_DTPREL_LO12, *TLSLD_LDST128_DTPREL_LO12_NC,
*TLSLE_LDST128_TPREL_LO12, *TLSLE_LDST128_TPREL_LO12_NC

Modify error messages to give name of equivalent relocation in the

ABI not being used, along with better checking for non-existent
requested relocations.

Added assembler support for "pg_hi21_nc"
Relocation definitions added without implementations:

R_AARCH64_P32_TLSDESC_ADR_PREL21, R_AARCH64_P32_TLSGD_ADR_PREL21,
R_AARCH64_P32_TLSGD_ADD_LO12_NC, R_AARCH64_P32_TLSLD_ADR_PREL21, 
R_AARCH64_P32_TLSLD_ADR_PAGE21, R_AARCH64_P32_TLSLD_ADD_LO12_NC,
R_AARCH64_P32_TLSLD_LD_PREL19, R_AARCH64_P32_TLSDESC_LD_PREL19,
R_AARCH64_P32_TLSGD_ADR_PAGE21, R_AARCH64_P32_TLS_DTPREL,
R_AARCH64_P32_TLS_DTPMOD, R_AARCH64_P32_TLS_TPREL,
R_AARCH64_P32_TLSDESC

Fix encoding:

R_AARCH64_P32_TLSDESC_ADR_PAGE21

Diff Detail

Repository
rL LLVM

Event Timeline

joelkevinjones created this revision.Apr 13 2017, 9:01 PM
peter.smith edited edge metadata.Apr 21 2017, 3:24 AM

I've spotted what I think is one mistake, R_AARCH64_P32_TLSDESC_ADR_PREL19 should be R_AARCH64_P32_TLSDESC_ADR_PREL21. I've put some suggestions for some missing test cases as well. Other than that I can't spot any obvious problems.

include/llvm/Support/ELFRelocs/AArch64.def
205 ↗(On Diff #95265)

I think this should be R_AARCH64_P32_TLSDESC_ADR_PREL21
http://infocenter.arm.com/help/topic/com.arm.doc.ihi0056c/IHI0056C_beta_aaelf64.pdf has this as
R_<CLS>_TLSDESC_ADR_PREL21 (search for 123)

lib/Target/AArch64/MCTargetDesc/AArch64ELFObjectWriter.cpp
22 ↗(On Diff #95265)

It isn't obvious from the changes why this is needed, just checking if it has been left in by mistake?

test/MC/AArch64/arm32-tls-relocs.s
127 ↗(On Diff #95265)

I think that there are also the R_AARCH64_P32_TLSLE_LD128_TPREL_LO12 and LO12_NC, I see that there is a test case in arm32-elf-relocs.s, but it would be nice to do the additional checks here.

235 ↗(On Diff #95265)

Is it worth a test case for R_AARCH64_P32_TLSLD_LDST128_DTPREL_LO12 and R_AARCH64_P32_TLSLD_LDST128_LO12_NC forms?

245 ↗(On Diff #95265)

I think that there is a missing test needed to generate the R_AARCH64_TLSDESC_ADR_PREL21 relocation, this is the one I pointed out earlier had been given the code R_AARCH64_TLSDESC_ADR_PREL19

270 ↗(On Diff #95265)

The general dynamic forms are missing tests:
R_AARCH64_P32_TLSGD_ADR_PREL21
R_AARCH64_P32_TLSGD_ADR_PAGE21
R_AARCH64_P32_TLSGD_ADD_LO12_NC

As noted in the inline comments, there are several relocations with no implementations in LP64 or ILP32. My intent with this patch was towards the goal of ILP32 parity with LP64. I still need to file a defect for the missing functionality.

include/llvm/Support/ELFRelocs/AArch64.def
205 ↗(On Diff #95265)

Concur

lib/Target/AArch64/MCTargetDesc/AArch64ELFObjectWriter.cpp
22 ↗(On Diff #95265)

Concur

test/MC/AArch64/arm32-tls-relocs.s
127 ↗(On Diff #95265)

Concur

245 ↗(On Diff #95265)

Currently, there is no implementation for either LP64 or ILP32 for TLSDESC_ADR_PREL21.

270 ↗(On Diff #95265)

There are currently no LP64 or ILP32 implementations for these relocations.

Address comments about missing tests (for those relocations intended to implemented) and about misnamed relocation.

New changes look ok, just from looking at the diff between the two patches I can see:
That there aren't any new tests for R_AARCH64_P32_TLSLD_LDST128_DTPREL_LO12 and R_AARCH64_P32_TLSLD_LDST128_LO12_NC forms (there is a comment) and I think that these are implemented as there is an earlier case in a different file that uses them. Have I got that one wrong?

That aside, and I'm sure you can remove the superfluous "debug.h" header prior to commit, unless there are any objections from anyone else I'd be happy to approve.

joelkevinjones edited the summary of this revision. (Show Details)
  • 0x07b is now named R_AARCH64_P32_TLSDESC_ADR_PREL21
  • "#include... Debug " removed
  • Tests for R_AARCH64_P32_TLSLE_LD128_TPREL_LO12 and ...ST..._NC added
  • For R_AARCH64_TLSDESC_ADR_PREL19, documented in summary as definition added, but not implemented.
  • Documented as definitions added, but not implemented: R_AARCH64_P32_TLSGD_ADR_PREL21, R_AARCH64_P32_TLSGD_ADR_PAGE21, R_AARCH64_P32_TLSGD_ADD_LO12_NC
  • I've added tests to arm{64,32}-{elf,tls}-relocs.s. I've added an encoding test for LDST128_ABS_LO12_NC to arm{64,32}-elf-relocs.s instead of where the other encoding tests are, arm{64,32}-tls-relocs.s, as this relocation isn't for TLS.
peter.smith accepted this revision.May 2 2017, 3:09 AM

I'm happy with the changes, and have no further comments.

This revision is now accepted and ready to land.May 2 2017, 3:09 AM
This revision was automatically updated to reflect the committed changes.