[PATCH] D144941: [m68k] Add TLS Support

[Jessica Clarke via Phabricator via llvm-commits]

jrtc27 added inline comments.


================
Comment at: llvm/lib/Target/M68k/M68kISelLowering.cpp:1399
+                             unsigned TargetFlags) {
+  // In general dynamic and local dynamic access models, to access a variable in
+  // Thread Local Storage (TLS), we typically use the function __tls_get_addr.
----------------
0x59616e wrote:
> 0x59616e wrote:
> > jrtc27 wrote:
> > > None of this is m68k-specific (with the exception of using unsigned instead of unsigned long), these long comments are just a repeat of how ELF TLS works in general. I don't think they belong here.
> > The code generation snippet for TLS is complicated. These comments help clarify the details and may be helpful for me or others who come to maintain it in the future.
> *The details of how TLS works are complex*
And those who want the details can go read Drepper's TLS document. All these comments are just noise; for anyone who knows how TLS works (which, if you're reading/writing this part of the compiler, you should) it serves no purpose and makes the code harder to read, and for anyone who doesn't they should go read the actual document. Or just read the code, it's not all that complicated, it's clearly just doing `__tls_get_addr(GOT + sym at TLSGD)`, `__tls_get_addr(GOT + sym at TLSLDM) + sym at TLSLD`, `__m68k_read_tp() + *(GOT + sym at GOTTPOFF)` and `__m68k_read_tp() + sym at TPOFF`. If every common bit of lowering were documented like these then the backends would be absurdly large, but we don't, we generally only document places where there are unusual or complicated things going on.


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

https://reviews.llvm.org/D144941