[ELF] - Detemplate Thunk creation.

[grimar]

Summary

Nothing special here, just detemplates code that became possible
to detemplate after recent commits in a straghtforward way.

[peter.smith]

As it stands no objections from me. I've got some changes that I'd like to make in this area for range extension thunks, I'm expecting to be able to rewrite them to take out the template parameter but I'd be grateful if you could take a quick look at https://reviews.llvm.org/D31662, to see if there is anything obvious there that would need the template parameter. The range thunks patches are currently out of date and will need rebasing but I think the general approach will remain the same.

Having said that I'm not making fast progress with the range thunk patches so it may be some time before I get to the patch that needs to put the template parameter back in and it will simplify the implementation in the mean time.

[grimar]

In D33234#755950, @peter.smith wrote:

As it stands no objections from me. I've got some changes that I'd like to make in this area for range extension thunks, I'm expecting to be able to rewrite them to take out the template parameter but I'd be grateful if you could take a quick look at https://reviews.llvm.org/D31662, to see if there is anything obvious there that would need the template parameter. The range thunks patches are currently out of date and will need rebasing but I think the general approach will remain the same.

Having said that I'm not making fast progress with the range thunk patches so it may be some time before I get to the patch that needs to put the template parameter back in and it will simplify the implementation in the mean time.

D31662 does not seem to need templates for me.
Thunks creating code is area under development so I do not have strong position about if we should land this patch or not if there are any concerns about we might need the templates in future.

So I would be happy to hear other reviewers opinions here.

[peter.smith]

Thanks for checking.

Rereading the sentence below implies that I have a patch that definitely needs the ELFT template parameter. I don't have such a patch, just might do. Apologies for any confusion.

Having said that I'm not making fast progress with the range thunk patches so it may be some time before I get to the patch that needs to put the template parameter back in and it will simplify the implementation in the mean time.

I'm happy for you to go ahead, I'm also happy to remove the template parameters when they aren't needed as the range thunks implementation progresses.

[ruiu]

LGTM

lld/trunk/ELF/Relocations.h

struct Relocation {
   SymbolBody *Sym;
 };
 
 template <class ELFT> void scanRelocations(InputSectionBase &);
 
 class ThunkSection;
 class Thunk;
 
-template <class ELFT> class ThunkCreator {
+class ThunkCreator {
 public:
   // Return true if Thunks have been added to OutputSections
   bool createThunks(ArrayRef<OutputSection *> OutputSections);
 
 private:
   void mergeThunks(OutputSection *OS, std::vector<ThunkSection *> &Thunks);
   ThunkSection *getOSThunkSec(ThunkSection *&TS, OutputSection *OS);
   ThunkSection *getISThunkSec(InputSection *IS, OutputSection *OS);

lld/trunk/ELF/Relocations.cpp

template <class ELFT> void elf::scanRelocations(InputSectionBase &S) {
   else
     scanRelocs<ELFT>(S, S.rels<ELFT>());
 }
 
 // Insert the Thunks for OutputSection OS into their designated place
 // in the Sections vector, and recalculate the InputSection output section
 // offsets.
 // This may invalidate any output section offsets stored outside of InputSection
-template <class ELFT>
-void ThunkCreator<ELFT>::mergeThunks(OutputSection *OS,
-                                     std::vector<ThunkSection *> &Thunks) {
+void ThunkCreator::mergeThunks(OutputSection *OS,
   // Order Thunks in ascending OutSecOff
   auto ThunkCmp = [](const ThunkSection *A, const ThunkSection *B) {
     return A->OutSecOff < B->OutSecOff;
   };
   std::stable_sort(Thunks.begin(), Thunks.end(), ThunkCmp);
 
   // Merge sorted vectors of Thunks and InputSections by OutSecOff
   std::vector<InputSection *> Tmp;
auto MergeCmp = [](const InputSection *A, const InputSection *B) {
     return false;
   };
   std::merge(OS->Sections.begin(), OS->Sections.end(), Thunks.begin(),
              Thunks.end(), std::back_inserter(Tmp), MergeCmp);
   OS->Sections = std::move(Tmp);
   OS->assignOffsets();
 }
 
-template <class ELFT>
-ThunkSection *ThunkCreator<ELFT>::getOSThunkSec(ThunkSection *&TS,
-                                                OutputSection *OS) {
+ThunkSection *ThunkCreator::getOSThunkSec(ThunkSection *&TS,
   if (TS == nullptr) {
     uint32_t Off = 0;
     for (auto *IS : OS->Sections) {
       Off = IS->OutSecOff + IS->getSize();
       if ((IS->Flags & SHF_EXECINSTR) == 0)
         break;
     }
     TS = make<ThunkSection>(OS, Off);
     ThunkSections[OS].push_back(TS);
   }
   return TS;
 }
 
-template <class ELFT>
-ThunkSection *ThunkCreator<ELFT>::getISThunkSec(InputSection *IS,
-                                                OutputSection *OS) {
+ThunkSection *ThunkCreator::getISThunkSec(InputSection *IS, OutputSection *OS) {
   ThunkSection *TS = ThunkedSections.lookup(IS);
   if (TS)
     return TS;
   auto *TOS = cast<OutputSection>(IS->OutSec);
   TS = make<ThunkSection>(TOS, IS->OutSecOff);
   ThunkSections[TOS].push_back(TS);
   ThunkedSections[IS] = TS;
   return TS;
 }
 
-template <class ELFT>
-std::pair<Thunk *, bool> ThunkCreator<ELFT>::getThunk(SymbolBody &Body,
-                                                      uint32_t Type) {
+std::pair<Thunk *, bool> ThunkCreator::getThunk(SymbolBody &Body,
   auto res = ThunkedSymbols.insert({&Body, nullptr});
   if (res.second)
-    res.first->second = addThunk<ELFT>(Type, Body);
+    res.first->second = addThunk(Type, Body);
   return std::make_pair(res.first->second, res.second);
 }
 
 // Process all relocations from the InputSections that have been assigned
 // to OutputSections and redirect through Thunks if needed.
 //
 // createThunks must be called after scanRelocs has created the Relocations for
 // each InputSection. It must be called before the static symbol table is
 // finalized. If any Thunks are added to an OutputSection the output section
 // offsets of the InputSections will change.
 //
 // FIXME: All Thunks are assumed to be in range of the relocation. Range
 // extension Thunks are not yet supported.
-template <class ELFT>
-bool ThunkCreator<ELFT>::createThunks(
-    ArrayRef<OutputSection *> OutputSections) {
+bool ThunkCreator::createThunks(ArrayRef<OutputSection *> OutputSections) {
   // Create all the Thunks and insert them into synthetic ThunkSections. The
   // ThunkSections are later inserted back into the OutputSection.
 
   // We separate the creation of ThunkSections from the insertion of the
   // ThunkSections back into the OutputSection as ThunkSections are not always
   // inserted into the same OutputSection as the caller.
   for (OutputSection *OS : OutputSections) {
     ThunkSection *OSTS = nullptr;
for (auto &KV : ThunkSections)
     mergeThunks(KV.first, KV.second);
   return !ThunkSections.empty();
 }
 
 template void elf::scanRelocations<ELF32LE>(InputSectionBase &);
 template void elf::scanRelocations<ELF32BE>(InputSectionBase &);
 template void elf::scanRelocations<ELF64LE>(InputSectionBase &);
 template void elf::scanRelocations<ELF64BE>(InputSectionBase &);
-
-template class elf::ThunkCreator<ELF32LE>;
-template class elf::ThunkCreator<ELF32BE>;
-template class elf::ThunkCreator<ELF64LE>;
-template class elf::ThunkCreator<ELF64BE>;

lld/trunk/ELF/Thunks.h

public:
   const SymbolBody &Destination;
   SymbolBody *ThunkSym;
   uint64_t Offset;
   uint32_t alignment = 4;
 };
 
 // For a Relocation to symbol S create a Thunk to be added to a synthetic
 // ThunkSection. At present there are implementations for ARM and Mips Thunks.
-template <class ELFT> Thunk *addThunk(uint32_t RelocType, SymbolBody &S);
+Thunk *addThunk(uint32_t RelocType, SymbolBody &S);
 
 } // namespace elf
 } // namespace lld
 
 #endif

lld/trunk/ELF/Thunks.cpp

 
 namespace lld {
 namespace elf {
 
 namespace {
 
 // Specific ARM Thunk implementations. The naming convention is:
 // Source State, TargetState, Target Requirement, ABS or PI, Range
-template <class ELFT> class ARMV7ABSLongThunk final : public Thunk {
+class ARMV7ABSLongThunk final : public Thunk {
 public:
   ARMV7ABSLongThunk(const SymbolBody &Dest) : Thunk(Dest) {}
 
   uint32_t size() const override { return 12; }
   void writeTo(uint8_t *Buf, ThunkSection &IS) const override;
   void addSymbols(ThunkSection &IS) override;
 };
 
-template <class ELFT> class ARMV7PILongThunk final : public Thunk {
+class ARMV7PILongThunk final : public Thunk {
 public:
   ARMV7PILongThunk(const SymbolBody &Dest) : Thunk(Dest) {}
 
   uint32_t size() const override { return 16; }
   void writeTo(uint8_t *Buf, ThunkSection &IS) const override;
   void addSymbols(ThunkSection &IS) override;
 };
 
-template <class ELFT> class ThumbV7ABSLongThunk final : public Thunk {
+class ThumbV7ABSLongThunk final : public Thunk {
 public:
   ThumbV7ABSLongThunk(const SymbolBody &Dest) : Thunk(Dest) {
     this->alignment = 2;
   }
 
   uint32_t size() const override { return 10; }
   void writeTo(uint8_t *Buf, ThunkSection &IS) const override;
   void addSymbols(ThunkSection &IS) override;
 };
 
-template <class ELFT> class ThumbV7PILongThunk final : public Thunk {
+class ThumbV7PILongThunk final : public Thunk {
 public:
   ThumbV7PILongThunk(const SymbolBody &Dest) : Thunk(Dest) {
     this->alignment = 2;
   }
 
   uint32_t size() const override { return 12; }
   void writeTo(uint8_t *Buf, ThunkSection &IS) const override;
   void addSymbols(ThunkSection &IS) override;
 };
 
 // MIPS LA25 thunk
-template <class ELFT> class MipsThunk final : public Thunk {
+class MipsThunk final : public Thunk {
 public:
   MipsThunk(const SymbolBody &Dest) : Thunk(Dest) {}
 
   uint32_t size() const override { return 16; }
   void writeTo(uint8_t *Buf, ThunkSection &IS) const override;
   void addSymbols(ThunkSection &IS) override;
   InputSection *getTargetInputSection() const override;
 };
 
 } // end anonymous namespace
 
 // ARM Target Thunks
 static uint64_t getARMThunkDestVA(const SymbolBody &S) {
   uint64_t V = S.isInPlt() ? S.getPltVA() : S.getVA();
   return SignExtend64<32>(V);
 }
 
-template <class ELFT>
-void ARMV7ABSLongThunk<ELFT>::writeTo(uint8_t *Buf, ThunkSection &IS) const {
+void ARMV7ABSLongThunk::writeTo(uint8_t *Buf, ThunkSection &IS) const {
   const uint8_t Data[] = {
       0x00, 0xc0, 0x00, 0xe3, // movw         ip,:lower16:S
       0x00, 0xc0, 0x40, 0xe3, // movt         ip,:upper16:S
       0x1c, 0xff, 0x2f, 0xe1, // bx   ip
   };
-  uint64_t S = getARMThunkDestVA(this->Destination);
+  uint64_t S = getARMThunkDestVA(Destination);
   memcpy(Buf, Data, sizeof(Data));
   Target->relocateOne(Buf, R_ARM_MOVW_ABS_NC, S);
   Target->relocateOne(Buf + 4, R_ARM_MOVT_ABS, S);
 }
 
-template <class ELFT>
-void ARMV7ABSLongThunk<ELFT>::addSymbols(ThunkSection &IS) {
-  this->ThunkSym = addSyntheticLocal(
-      Saver.save("__ARMv7ABSLongThunk_" + this->Destination.getName()),
-      STT_FUNC, this->Offset, size(), &IS);
-  addSyntheticLocal("$a", STT_NOTYPE, this->Offset, 0, &IS);
+void ARMV7ABSLongThunk::addSymbols(ThunkSection &IS) {
+  ThunkSym = addSyntheticLocal(
+      Saver.save("__ARMv7ABSLongThunk_" + Destination.getName()), STT_FUNC,
+      Offset, size(), &IS);
+  addSyntheticLocal("$a", STT_NOTYPE, Offset, 0, &IS);
 }
 
-template <class ELFT>
-void ThumbV7ABSLongThunk<ELFT>::writeTo(uint8_t *Buf, ThunkSection &IS) const {
+void ThumbV7ABSLongThunk::writeTo(uint8_t *Buf, ThunkSection &IS) const {
   const uint8_t Data[] = {
       0x40, 0xf2, 0x00, 0x0c, // movw         ip, :lower16:S
       0xc0, 0xf2, 0x00, 0x0c, // movt         ip, :upper16:S
       0x60, 0x47,             // bx   ip
   };
-  uint64_t S = getARMThunkDestVA(this->Destination);
+  uint64_t S = getARMThunkDestVA(Destination);
   memcpy(Buf, Data, sizeof(Data));
   Target->relocateOne(Buf, R_ARM_THM_MOVW_ABS_NC, S);
   Target->relocateOne(Buf + 4, R_ARM_THM_MOVT_ABS, S);
 }
 
-template <class ELFT>
-void ThumbV7ABSLongThunk<ELFT>::addSymbols(ThunkSection &IS) {
-  this->ThunkSym = addSyntheticLocal(
-      Saver.save("__Thumbv7ABSLongThunk_" + this->Destination.getName()),
-      STT_FUNC, this->Offset, size(), &IS);
-  addSyntheticLocal("$t", STT_NOTYPE, this->Offset, 0, &IS);
+void ThumbV7ABSLongThunk::addSymbols(ThunkSection &IS) {
+  ThunkSym = addSyntheticLocal(
+      Saver.save("__Thumbv7ABSLongThunk_" + Destination.getName()), STT_FUNC,
+      Offset, size(), &IS);
+  addSyntheticLocal("$t", STT_NOTYPE, Offset, 0, &IS);
 }
 
-template <class ELFT>
-void ARMV7PILongThunk<ELFT>::writeTo(uint8_t *Buf, ThunkSection &IS) const {
+void ARMV7PILongThunk::writeTo(uint8_t *Buf, ThunkSection &IS) const {
   const uint8_t Data[] = {
       0xf0, 0xcf, 0x0f, 0xe3, // P:  movw ip,:lower16:S - (P + (L1-P) +8)
       0x00, 0xc0, 0x40, 0xe3, //     movt ip,:upper16:S - (P + (L1-P+4) +8)
       0x0f, 0xc0, 0x8c, 0xe0, // L1: add ip, ip, pc
       0x1c, 0xff, 0x2f, 0xe1, //     bx r12
   };
-  uint64_t S = getARMThunkDestVA(this->Destination);
-  uint64_t P = this->ThunkSym->getVA();
+  uint64_t S = getARMThunkDestVA(Destination);
+  uint64_t P = ThunkSym->getVA();
   memcpy(Buf, Data, sizeof(Data));
   Target->relocateOne(Buf, R_ARM_MOVW_PREL_NC, S - P - 16);
   Target->relocateOne(Buf + 4, R_ARM_MOVT_PREL, S - P - 12);
 }
 
-template <class ELFT>
-void ARMV7PILongThunk<ELFT>::addSymbols(ThunkSection &IS) {
-  this->ThunkSym = addSyntheticLocal(
-      Saver.save("__ARMV7PILongThunk_" + this->Destination.getName()), STT_FUNC,
-      this->Offset, size(), &IS);
-  addSyntheticLocal("$a", STT_NOTYPE, this->Offset, 0, &IS);
+void ARMV7PILongThunk::addSymbols(ThunkSection &IS) {
+  ThunkSym = addSyntheticLocal(
+      Saver.save("__ARMV7PILongThunk_" + Destination.getName()), STT_FUNC,
+      Offset, size(), &IS);
+  addSyntheticLocal("$a", STT_NOTYPE, Offset, 0, &IS);
 }
 
-template <class ELFT>
-void ThumbV7PILongThunk<ELFT>::writeTo(uint8_t *Buf, ThunkSection &IS) const {
+void ThumbV7PILongThunk::writeTo(uint8_t *Buf, ThunkSection &IS) const {
   const uint8_t Data[] = {
       0x4f, 0xf6, 0xf4, 0x7c, // P:  movw ip,:lower16:S - (P + (L1-P) + 4)
       0xc0, 0xf2, 0x00, 0x0c, //     movt ip,:upper16:S - (P + (L1-P+4) + 4)
       0xfc, 0x44,             // L1: add  r12, pc
       0x60, 0x47,             //     bx   r12
   };
-  uint64_t S = getARMThunkDestVA(this->Destination);
-  uint64_t P = this->ThunkSym->getVA();
+  uint64_t S = getARMThunkDestVA(Destination);
+  uint64_t P = ThunkSym->getVA();
   memcpy(Buf, Data, sizeof(Data));
   Target->relocateOne(Buf, R_ARM_THM_MOVW_PREL_NC, S - P - 12);
   Target->relocateOne(Buf + 4, R_ARM_THM_MOVT_PREL, S - P - 8);
 }
 
-template <class ELFT>
-void ThumbV7PILongThunk<ELFT>::addSymbols(ThunkSection &IS) {
-  this->ThunkSym = addSyntheticLocal(
-      Saver.save("__ThumbV7PILongThunk_" + this->Destination.getName()),
-      STT_FUNC, this->Offset, size(), &IS);
-  addSyntheticLocal("$t", STT_NOTYPE, this->Offset, 0, &IS);
+void ThumbV7PILongThunk::addSymbols(ThunkSection &IS) {
+  ThunkSym = addSyntheticLocal(
+      Saver.save("__ThumbV7PILongThunk_" + Destination.getName()), STT_FUNC,
+      Offset, size(), &IS);
+  addSyntheticLocal("$t", STT_NOTYPE, Offset, 0, &IS);
 }
 
 // Write MIPS LA25 thunk code to call PIC function from the non-PIC one.
-template <class ELFT>
-void MipsThunk<ELFT>::writeTo(uint8_t *Buf, ThunkSection &) const {
-  const endianness E = ELFT::TargetEndianness;
-
+void MipsThunk::writeTo(uint8_t *Buf, ThunkSection &) const {
   uint64_t S = this->Destination.getVA();
-  write32<E>(Buf, 0x3c190000);                // lui   $25, %hi(func)
-  write32<E>(Buf + 4, 0x08000000 | (S >> 2)); // j     func
-  write32<E>(Buf + 8, 0x27390000);            // addiu $25, $25, %lo(func)
-  write32<E>(Buf + 12, 0x00000000);           // nop
+  write32(Buf, 0x3c190000, Config->Endianness);                // lui   $25, %hi(func)
+  write32(Buf + 4, 0x08000000 | (S >> 2), Config->Endianness); // j     func
+  write32(Buf + 8, 0x27390000, Config->Endianness);            // addiu $25, $25, %lo(func)
+  write32(Buf + 12, 0x00000000, Config->Endianness);           // nop
   Target->relocateOne(Buf, R_MIPS_HI16, S);
   Target->relocateOne(Buf + 8, R_MIPS_LO16, S);
 }
 
-template <class ELFT> void MipsThunk<ELFT>::addSymbols(ThunkSection &IS) {
-  this->ThunkSym = addSyntheticLocal(
-      Saver.save("__LA25Thunk_" + this->Destination.getName()), STT_FUNC,
-      this->Offset, size(), &IS);
+void MipsThunk::addSymbols(ThunkSection &IS) {
+  ThunkSym =
+      addSyntheticLocal(Saver.save("__LA25Thunk_" + Destination.getName()),
+                        STT_FUNC, Offset, size(), &IS);
 }
 
-template <class ELFT>
-InputSection *MipsThunk<ELFT>::getTargetInputSection() const {
-  auto *DR = dyn_cast<DefinedRegular>(&this->Destination);
+InputSection *MipsThunk::getTargetInputSection() const {
+  auto *DR = dyn_cast<DefinedRegular>(&Destination);
   return dyn_cast<InputSection>(DR->Section);
 }
 
 Thunk::Thunk(const SymbolBody &D) : Destination(D), Offset(0) {}
 
 Thunk::~Thunk() = default;
 
 // Creates a thunk for Thumb-ARM interworking.
-template <class ELFT> static Thunk *addThunkArm(uint32_t Reloc, SymbolBody &S) {
+static Thunk *addThunkArm(uint32_t Reloc, SymbolBody &S) {
   // ARM relocations need ARM to Thumb interworking Thunks.
   // Thumb relocations need Thumb to ARM relocations.
   // Use position independent Thunks if we require position independent code.
   switch (Reloc) {
   case R_ARM_PC24:
   case R_ARM_PLT32:
   case R_ARM_JUMP24:
     if (Config->Pic)
-      return make<ARMV7PILongThunk<ELFT>>(S);
-    return make<ARMV7ABSLongThunk<ELFT>>(S);
+      return make<ARMV7PILongThunk>(S);
+    return make<ARMV7ABSLongThunk>(S);
   case R_ARM_THM_JUMP19:
   case R_ARM_THM_JUMP24:
     if (Config->Pic)
-      return make<ThumbV7PILongThunk<ELFT>>(S);
-    return make<ThumbV7ABSLongThunk<ELFT>>(S);
+      return make<ThumbV7PILongThunk>(S);
+    return make<ThumbV7ABSLongThunk>(S);
   }
   fatal("unrecognized relocation type");
 }
 
-template <class ELFT> static Thunk *addThunkMips(SymbolBody &S) {
-  return make<MipsThunk<ELFT>>(S);
+static Thunk *addThunkMips(SymbolBody &S) {
+  return make<MipsThunk>(S);
 }
 
-template <class ELFT> Thunk *addThunk(uint32_t RelocType, SymbolBody &S) {
+Thunk *addThunk(uint32_t RelocType, SymbolBody &S) {
   if (Config->EMachine == EM_ARM)
-    return addThunkArm<ELFT>(RelocType, S);
+    return addThunkArm(RelocType, S);
   else if (Config->EMachine == EM_MIPS)
-    return addThunkMips<ELFT>(S);
+    return addThunkMips(S);
   llvm_unreachable("add Thunk only supported for ARM and Mips");
   return nullptr;
 }
 
-template Thunk *addThunk<ELF32LE>(uint32_t, SymbolBody &);
-template Thunk *addThunk<ELF32BE>(uint32_t, SymbolBody &);
-template Thunk *addThunk<ELF64LE>(uint32_t, SymbolBody &);
-template Thunk *addThunk<ELF64BE>(uint32_t, SymbolBody &);
 } // end namespace elf
 } // end namespace lld

lld/trunk/ELF/Writer.cpp

template <class ELFT> void Writer<ELFT>::finalizeSections() {
   // jump instructions if jump targets are too far. Create thunks.
   if (Target->NeedsThunks) {
     // FIXME: only ARM Interworking and Mips LA25 Thunks are implemented,
     // these
     // do not require address information. To support range extension Thunks
     // we need to assign addresses so that we can tell if jump instructions
     // are out of range. This will need to turn into a loop that converges
     // when no more Thunks are added
-    ThunkCreator<ELFT> TC;
+    ThunkCreator TC;
     if (TC.createThunks(OutputSections))
       applySynthetic({InX::MipsGot},
                      [](SyntheticSection *SS) { SS->updateAllocSize(); });
   }
   // Fill other section headers. The dynamic table is finalized
   // at the end because some tags like RELSZ depend on result
   // of finalizing other sections.
   for (OutputSection *Sec : OutputSections)