[PATCH] D18330: [ELF/AArch64] Implemented set of R_AARCH64_TLSDESC_* relocations.

[Adhemerval Zanella via llvm-commits]

zatrazz added inline comments.

================
Comment at: ELF/InputSection.cpp:275
@@ +274,3 @@
+    // compute the value of the two words of the TLS descriptor.
+    // Overall design can be found in
+    // "Thread-Local Storage Descriptors for IA32 and AMD64/EM64T"
----------------
emaste wrote:
> Maybe mention "is based on" or such to make it clear this wasn't accidentally copied and pasted from X86-specific stuff.
What about:

// TLS descriptor uses two words in the .got.plt which describes a structure
// of a function pointer and its arguments. Both fields will be adjusted  at 
// runtime by the dynamic loader with a single relocation (usually
// R_<ARCH>_TLSDESC).

================
Comment at: ELF/OutputSections.cpp:293
@@ +292,3 @@
+  // Tail here is amount of such special relocations.
+  Relocs.insert(Relocs.end() - Tail, Reloc);
+}
----------------
emaste wrote:
> I wonder if there's a nicer way to handle this?
I though about this, but I think to remove the Tail it would require to maintain two or move relocation vectors (one for normal and another for tlsdesc) and this will just add more complexity IMHO.

However we might change in later if this shows to be a performance issue.

================
Comment at: ELF/OutputSections.h:179
@@ -170,2 +178,3 @@
     Off_GotPlt,    // The got.plt entry of Sym.
+    Off_GotPltRev,
     Off_Bss,       // The bss entry of Sym (copy reloc).
----------------
emaste wrote:
> Comment?
I think it would be better to change to Off_GotPltTlsdesc and add a comment such 'the got.plt entry for a tlsdesc Sym'.


Repository:
  rL LLVM

http://reviews.llvm.org/D18330