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

[George Rimar via llvm-commits]

grimar added inline comments.

================
Comment at: ELF/InputSection.cpp:236
@@ -222,3 +235,3 @@
       continue;
     } else if (Target->isSizeReloc(Type) && canBePreempted(Body, false)) {
       // A SIZE relocation is supposed to set a symbol size, but if a symbol
----------------
ruiu wrote:
> ... why don't you add
> 
>   } else if (Body->isTls() && Target->isTlsDescReloc(Type, *Body)) {
>     SymVA = Out<ELFT>::GotPlt->getEntryAddr(*Body)
>     continue;
> 
> here? (I'm not saying that this long if-elseif-elseif is easy to read, but consistency matters.
In general descriptor relocations are very specific and if for example we have logic for isTlsGlobalDynamicReloc separated (which is also specific case) then why dont do the same for descreloc ?
Code for Target->isTlsLocalDynamicReloc(Type) and Target->isTlsGlobalDynamicReloc(Type) is above it. So I did it exactly for consistency.
Also I think in the similar situation for R_X86_64_GOTTPOFF optimization (http://reviews.llvm.org/D14713?vs=on&id=40303&whitespace=ignore-most#toc, ELF/InputSection.cpp, line 150) you asked me to move the code above uintX_t SymVA :)



================
Comment at: ELF/Writer.cpp:251
@@ +250,3 @@
+    if (Body && Body->isTls()) {
+      // If module uses TLS descriptor relocations,
+      // then special entries are created for module:
----------------
ruiu wrote:
> Can you move this code to a new function? This function got too large.
Created new function for all TLS code in http://reviews.llvm.org/D16354, moved that code.


http://reviews.llvm.org/D16201