[ELF] Rearrange relocation codes in natural order. NFC.

[ruiu]

The original order looked random, but the new order also looks somewhat random. Should we instead order them alphabetically? Also, "follow the order in the spec" strategy does not work if some relocations are defined in a separate document (such as TLS relocations.)

ELF/Target.cpp

943 ↗

(On Diff #41306)

Actually I'd remove all these "// no overflow check needed." comments as that's obvious from code.

[ikudrin]

In D15045#297899, @ruiu wrote:

The original order looked random, but the new order also looks somewhat random. Should we instead order them alphabetically? Also, "follow the order in the spec" strategy does not work if some relocations are defined in a separate document (such as TLS relocations.)

OK. Alphabetical order also seems convenient to me.

[ikudrin]

• Rearranged alphabetically.

[ruiu]

Please do this for all architectures.

[emaste]

In D15045#297899, @ruiu wrote:

Should we instead order them alphabetically?

I'd certainly prefer alpha order rather than an arbitrary order that's the result of adding new ones at the end. But I wonder if there's a reasonable grouping that's lost in enforcing alpha order? Even with some relocations in separate documents we could produce a reasonable grouping - follow the order in the main spec, followed by the TLS spec for example.

[ikudrin]

• Rearranged most of relocation codes within Target.cpp.

I have to note that I'm not sure that the new order is better than previous one for all the targets.

[ruiu]

Is this alphabetical order?

ELF/Target.cpp

746–753 ↗

(On Diff #41389)

Keep the original style. (I know this style is a bit controversial, but at least I don't want to change that in this patch.)

[ikudrin]

In D15045#298613, @ruiu wrote:

Is this alphabetical order?

I meant this one: Natural sort order.

ELF/Target.cpp

746–753 ↗

(On Diff #41389)

Oops, clang-formatted unintentionally.

[ruiu]

LGTM

Usually relevant relocations share the same prefix, so they are grouped together in the natural order. I'm not sure which is considered standard, natural order or asciibetical order, but I prefer natural order, since "PC8, PC16, PC32, PC64" seems more natural to me than "PC16, PC32, PC64, PC8".

[davide]

Fine to me as well then.

lld/trunk/ELF/Target.cpp

 
 bool X86TargetInfo::relocNeedsPlt(uint32_t Type, const SymbolBody &S) const {
   return Type == R_386_PLT32 || (Type == R_386_PC32 && S.isShared());
 }
 
 void X86TargetInfo::relocateOne(uint8_t *Loc, uint8_t *BufEnd, uint32_t Type,
                                 uint64_t P, uint64_t SA) const {
   switch (Type) {
+  case R_386_32:
+    add32le(Loc, SA);
+    break;
   case R_386_GOT32:
     add32le(Loc, SA - Out<ELF32LE>::Got->getVA());
     break;
   case R_386_GOTPC:
     add32le(Loc, SA + Out<ELF32LE>::Got->getVA() - P);
     break;
   case R_386_PC32:
     add32le(Loc, SA - P);
     break;
-  case R_386_32:
-    add32le(Loc, SA);
-    break;
   case R_386_TLS_LE:
     write32le(Loc, SA - Out<ELF32LE>::TlsPhdr->p_memsz);
     break;
   case R_386_TLS_LE_32:
     write32le(Loc, Out<ELF32LE>::TlsPhdr->p_memsz - SA);
     break;
   default:
     error("unrecognized reloc " + Twine(Type));
case R_X86_64_PLT32:
     return canBePreempted(&S, true);
   }
 }
 
 bool X86_64TargetInfo::isRelRelative(uint32_t Type) const {
   switch (Type) {
   default:
     return false;
-  case R_X86_64_PC64:
-  case R_X86_64_PC32:
-  case R_X86_64_PC16:
-  case R_X86_64_PC8:
-  case R_X86_64_PLT32:
   case R_X86_64_DTPOFF32:
   case R_X86_64_DTPOFF64:
+  case R_X86_64_PC8:
+  case R_X86_64_PC16:
+  case R_X86_64_PC32:
+  case R_X86_64_PC64:
+  case R_X86_64_PLT32:
     return true;
   }
 }
 
 bool X86_64TargetInfo::isTlsOptimized(unsigned Type,
                                       const SymbolBody *S) const {
   if (Config->Shared || (S && !S->isTLS()))
     return false;

 // in the Ulrich's document. As a result of rewriting instructions at the
 // relocation target, relocations immediately follow the TLS relocation (which
 // would be applied to rewritten instructions) may have to be skipped.
 // This function returns a number of relocations that need to be skipped.
 unsigned X86_64TargetInfo::relocateTlsOptimize(uint8_t *Loc, uint8_t *BufEnd,
                                                uint32_t Type, uint64_t P,
                                                uint64_t SA) const {
   switch (Type) {
+  case R_X86_64_DTPOFF32:
+    relocateOne(Loc, BufEnd, R_X86_64_TPOFF32, P, SA);
+    return 0;
   case R_X86_64_GOTTPOFF:
     relocateTlsIeToLe(Loc, BufEnd, P, SA);
     return 0;
-  case R_X86_64_TLSLD:
-    relocateTlsLdToLe(Loc, BufEnd, P, SA);
-    // The next relocation should be against __tls_get_addr, so skip it
-    return 1;
   case R_X86_64_TLSGD:
     relocateTlsGdToLe(Loc, BufEnd, P, SA);
     // The next relocation should be against __tls_get_addr, so skip it
     return 1;
-  case R_X86_64_DTPOFF32:
-    relocateOne(Loc, BufEnd, R_X86_64_TPOFF32, P, SA);
-    return 0;
+  case R_X86_64_TLSLD:
+    relocateTlsLdToLe(Loc, BufEnd, P, SA);
+    // The next relocation should be against __tls_get_addr, so skip it
+    return 1;
   }
   llvm_unreachable("Unknown TLS optimization");
 }
 
 void X86_64TargetInfo::relocateOne(uint8_t *Loc, uint8_t *BufEnd, uint32_t Type,
                                    uint64_t P, uint64_t SA) const {
   switch (Type) {
-  case R_X86_64_PC32:
-  case R_X86_64_GOTPCREL:
-  case R_X86_64_PLT32:
-  case R_X86_64_TLSLD:
-  case R_X86_64_TLSGD:
-  case R_X86_64_TPOFF64:
-    write32le(Loc, SA - P);
-    break;
-  case R_X86_64_64:
-  case R_X86_64_DTPOFF64:
-    write64le(Loc, SA);
-    break;
   case R_X86_64_32:
     checkUInt<32>(SA, Type);
     write32le(Loc, SA);
     break;
   case R_X86_64_32S:
     checkInt<32>(SA, Type);
     write32le(Loc, SA);
     break;
+  case R_X86_64_64:
+    write64le(Loc, SA);
+    break;
   case R_X86_64_DTPOFF32:
     write32le(Loc, SA);
     break;
+  case R_X86_64_DTPOFF64:
+    write64le(Loc, SA);
+    break;
+  case R_X86_64_GOTPCREL:
+  case R_X86_64_PC32:
+  case R_X86_64_PLT32:
+  case R_X86_64_TLSGD:
+  case R_X86_64_TLSLD:
+    write32le(Loc, SA - P);
+    break;
   case R_X86_64_TPOFF32: {
     uint64_t Val = SA - Out<ELF64LE>::TlsPhdr->p_memsz;
     checkInt<32>(Val, Type);
     write32le(Loc, Val);
     break;
   }
+  case R_X86_64_TPOFF64:
+    write32le(Loc, SA - P);
+    break;
   default:
     error("unrecognized reloc " + Twine(Type));
   }
 }
 
 // Relocation masks following the #lo(value), #hi(value), #ha(value),
 // #higher(value), #highera(value), #highest(value), and #highesta(value)
 // macros defined in section 4.5.1. Relocation Types of the PPC-elf64abi

 
 bool PPC64TargetInfo::relocNeedsGot(uint32_t Type, const SymbolBody &S) const {
   if (relocNeedsPlt(Type, S))
     return true;
 
   switch (Type) {
   default: return false;
   case R_PPC64_GOT16:
-  case R_PPC64_GOT16_LO:
-  case R_PPC64_GOT16_HI:
-  case R_PPC64_GOT16_HA:
   case R_PPC64_GOT16_DS:
+  case R_PPC64_GOT16_HA:
+  case R_PPC64_GOT16_HI:
+  case R_PPC64_GOT16_LO:
   case R_PPC64_GOT16_LO_DS:
     return true;
   }
 }
 
 bool PPC64TargetInfo::relocNeedsPlt(uint32_t Type, const SymbolBody &S) const {
   // These are function calls that need to be redirected through a PLT stub.
   return Type == R_PPC64_REL24 && canBePreempted(&S, false);
 }
 
 bool PPC64TargetInfo::isRelRelative(uint32_t Type) const {
   switch (Type) {
   default:
     return true;
-  case R_PPC64_TOC:
   case R_PPC64_ADDR64:
+  case R_PPC64_TOC:
     return false;
   }
 }
 
 void PPC64TargetInfo::relocateOne(uint8_t *Loc, uint8_t *BufEnd, uint32_t Type,
                                   uint64_t P, uint64_t SA) const {
   uint64_t TB = getPPC64TocBase();
 
   // For a TOC-relative relocation, adjust the addend and proceed in terms of
   // the corresponding ADDR16 relocation type.
   switch (Type) {
   case R_PPC64_TOC16:       Type = R_PPC64_ADDR16;       SA -= TB; break;
   case R_PPC64_TOC16_DS:    Type = R_PPC64_ADDR16_DS;    SA -= TB; break;
+  case R_PPC64_TOC16_HA:    Type = R_PPC64_ADDR16_HA;    SA -= TB; break;
+  case R_PPC64_TOC16_HI:    Type = R_PPC64_ADDR16_HI;    SA -= TB; break;
   case R_PPC64_TOC16_LO:    Type = R_PPC64_ADDR16_LO;    SA -= TB; break;
   case R_PPC64_TOC16_LO_DS: Type = R_PPC64_ADDR16_LO_DS; SA -= TB; break;
-  case R_PPC64_TOC16_HI:    Type = R_PPC64_ADDR16_HI;    SA -= TB; break;
-  case R_PPC64_TOC16_HA:    Type = R_PPC64_ADDR16_HA;    SA -= TB; break;
   default: break;
   }
 
   switch (Type) {
+  case R_PPC64_ADDR14: {
+    checkAlignment<4>(SA, Type);
+    // Preserve the AA/LK bits in the branch instruction
+    uint8_t AALK = Loc[3];
+    write16be(Loc + 2, (AALK & 3) | (SA & 0xfffc));
+    break;
+  }
   case R_PPC64_ADDR16:
     checkInt<16>(SA, Type);
     write16be(Loc, SA);
     break;
   case R_PPC64_ADDR16_DS:
     checkInt<16>(SA, Type);
     write16be(Loc, (read16be(Loc) & 3) | (SA & ~3));
     break;
-  case R_PPC64_ADDR16_LO:
-    write16be(Loc, applyPPCLo(SA));
-    break;
-  case R_PPC64_ADDR16_LO_DS:
-    write16be(Loc, (read16be(Loc) & 3) | (applyPPCLo(SA) & ~3));
+  case R_PPC64_ADDR16_HA:
+    write16be(Loc, applyPPCHa(SA));
     break;
   case R_PPC64_ADDR16_HI:
     write16be(Loc, applyPPCHi(SA));
     break;
-  case R_PPC64_ADDR16_HA:
-    write16be(Loc, applyPPCHa(SA));
-    break;
   case R_PPC64_ADDR16_HIGHER:
     write16be(Loc, applyPPCHigher(SA));
     break;
   case R_PPC64_ADDR16_HIGHERA:
     write16be(Loc, applyPPCHighera(SA));
     break;
   case R_PPC64_ADDR16_HIGHEST:
     write16be(Loc, applyPPCHighest(SA));
     break;
   case R_PPC64_ADDR16_HIGHESTA:
     write16be(Loc, applyPPCHighesta(SA));
     break;
-  case R_PPC64_ADDR14: {
-    checkAlignment<4>(SA, Type);
-    // Preserve the AA/LK bits in the branch instruction
-    uint8_t AALK = Loc[3];
-    write16be(Loc + 2, (AALK & 3) | (SA & 0xfffc));
+  case R_PPC64_ADDR16_LO:
+    write16be(Loc, applyPPCLo(SA));
     break;
-  }
-  case R_PPC64_REL16_LO:
-    write16be(Loc, applyPPCLo(SA - P));
+  case R_PPC64_ADDR16_LO_DS:
+    write16be(Loc, (read16be(Loc) & 3) | (applyPPCLo(SA) & ~3));
     break;
-  case R_PPC64_REL16_HI:
-    write16be(Loc, applyPPCHi(SA - P));
+  case R_PPC64_ADDR32:
+    checkInt<32>(SA, Type);
+    write32be(Loc, SA);
+    break;
+  case R_PPC64_ADDR64:
+    write64be(Loc, SA);
     break;
   case R_PPC64_REL16_HA:
     write16be(Loc, applyPPCHa(SA - P));
     break;
-  case R_PPC64_ADDR32:
-    checkInt<32>(SA, Type);
-    write32be(Loc, SA);
+  case R_PPC64_REL16_HI:
+    write16be(Loc, applyPPCHi(SA - P));
+    break;
+  case R_PPC64_REL16_LO:
+    write16be(Loc, applyPPCLo(SA - P));
     break;
   case R_PPC64_REL24: {
     // If we have an undefined weak symbol, we might get here with a symbol
     // address of zero. That could overflow, but the code must be unreachable,
     // so don't bother doing anything at all.
     if (!SA)
       break;
 
void PPC64TargetInfo::relocateOne(uint8_t *Loc, uint8_t *BufEnd, uint32_t Type,
   }
   case R_PPC64_REL32:
     checkInt<32>(SA - P, Type);
     write32be(Loc, SA - P);
     break;
   case R_PPC64_REL64:
     write64be(Loc, SA - P);
     break;
-  case R_PPC64_ADDR64:
   case R_PPC64_TOC:
     write64be(Loc, SA);
     break;
   default:
     error("unrecognized reloc " + Twine(Type));
   }
 }
 
return Type == R_AARCH64_ADR_GOT_PAGE || Type == R_AARCH64_LD64_GOT_LO12_NC ||
          relocNeedsPlt(Type, S);
 }
 
 bool AArch64TargetInfo::relocNeedsPlt(uint32_t Type,
                                       const SymbolBody &S) const {
   switch (Type) {
   default:
     return false;
-  case R_AARCH64_JUMP26:
   case R_AARCH64_CALL26:
+  case R_AARCH64_JUMP26:
     return canBePreempted(&S, true);
   }
 }
 
 static void updateAArch64Adr(uint8_t *L, uint64_t Imm) {
   uint32_t ImmLo = (Imm & 0x3) << 29;
   uint32_t ImmHi = ((Imm & 0x1FFFFC) >> 2) << 5;
   uint64_t Mask = (0x3 << 29) | (0x7FFFF << 5);
case R_AARCH64_ABS64:
     break;
   case R_AARCH64_ADD_ABS_LO12_NC:
     // This relocation stores 12 bits and there's no instruction
     // to do it. Instead, we do a 32 bits store of the value
     // of r_addend bitwise-or'ed Loc. This assumes that the addend
     // bits in Loc are zero.
     or32le(Loc, (SA & 0xFFF) << 10);
     break;
+  case R_AARCH64_ADR_GOT_PAGE: {
+    uint64_t X = getAArch64Page(SA) - getAArch64Page(P);
+    checkInt<33>(X, Type);
+    updateAArch64Adr(Loc, (X >> 12) & 0x1FFFFF); // X[32:12]
+    break;
+  }
   case R_AARCH64_ADR_PREL_LO21: {
     uint64_t X = SA - P;
     checkInt<21>(X, Type);
     updateAArch64Adr(Loc, X & 0x1FFFFF);
     break;
   }
-  case R_AARCH64_ADR_GOT_PAGE:
   case R_AARCH64_ADR_PREL_PG_HI21: {
     uint64_t X = getAArch64Page(SA) - getAArch64Page(P);
     checkInt<33>(X, Type);
     updateAArch64Adr(Loc, (X >> 12) & 0x1FFFFF); // X[32:12]
     break;
   }
-  case R_AARCH64_JUMP26:
-  case R_AARCH64_CALL26: {
+  case R_AARCH64_CALL26:
+  case R_AARCH64_JUMP26: {
     uint64_t X = SA - P;
     checkInt<28>(X, Type);
     or32le(Loc, (X & 0x0FFFFFFC) >> 2);
     break;
   }
-  case R_AARCH64_LDST32_ABS_LO12_NC:
-    or32le(Loc, (SA & 0xFFC) << 8);
-    break;
   case R_AARCH64_LD64_GOT_LO12_NC:
     checkAlignment<8>(SA, Type);
     or32le(Loc, (SA & 0xFF8) << 7);
     break;
-  case R_AARCH64_LDST64_ABS_LO12_NC:
-    or32le(Loc, (SA & 0xFF8) << 7);
-    break;
   case R_AARCH64_LDST8_ABS_LO12_NC:
     or32le(Loc, (SA & 0xFFF) << 10);
     break;
+  case R_AARCH64_LDST32_ABS_LO12_NC:
+    or32le(Loc, (SA & 0xFFC) << 8);
+    break;
+  case R_AARCH64_LDST64_ABS_LO12_NC:
+    or32le(Loc, (SA & 0xFF8) << 7);
+    break;
   case R_AARCH64_PREL16:
     checkIntUInt<16>(SA - P, Type);
     write16le(Loc, SA - P);
     break;
   case R_AARCH64_PREL32:
     checkIntUInt<32>(SA - P, Type);
     write32le(Loc, SA - P);
     break;