[PATCH] D85994: [LLD][PowerPC] Add check in LLD to produce an error for missing TLSGD/TLSLD

[Stefan Pintilie via Phabricator via llvm-commits]

stefanp added inline comments.


================
Comment at: lld/ELF/Relocations.cpp:1311
   if (config->emachine == EM_PPC64) {
+    // When computing the address of a TLS symbol for the General Dynamic
+    // and Local Dynamic models a call is made to __tls_get_addr. This function
----------------
MaskRay wrote:
> I still think this is a diagnostic of lower value, so I'd hope we can spend less code on this (when it is no longer needed, delete it).
> 
> The comments are too verbose in my opinion. We have explained __tls_get_addr is used by General Dynamic/Local Dynamic TLS models, so there is no point spending more sentences on it. Suggest:
> 
> // For a call to __tls_get_addr, the instruction needs to relocated by two relocations, R_PPC64_TLSGD/R_PPC64_TLSLD and R_PPC64_REL24[_NOTOC]
> 
> I am not sure it is worth checking the relocation at i - 2.
I can certainly reduce the comment.


However, checking `i - 2` is the whole point of this check. The function is passed in `i` that is where `rel` comes from. We then do and `i++` on line 1277 and so in my code I need to use `i - 1` to get back to the original `i`. Once I have checked for R_PPC64_REL24[_NOTOC] and the name of the function I need to go back one step (to `i - 2`) to check and see if we really have a missing R_PPC64_TLSGD/R_PPC64_TLSLD.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85994/new/

https://reviews.llvm.org/D85994