[PATCH] D149722: [AIX][TLS] Generate 64-bit local-exec access code sequence

Nemanja Ivanovic via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 16 08:50:42 PDT 2023


nemanjai accepted this revision.
nemanjai added a comment.
This revision is now accepted and ready to land.

LGTM.



================
Comment at: llvm/include/llvm/MC/MCExpr.h:302
     VK_PPC_AIX_TLSGDM,      // symbol at m
+    VK_PPC_AIX_TLSLE,       // symbol at le
     VK_PPC_GOT_TLSLD,       // symbol at got@tlsld
----------------
lei wrote:
> I realize this was done similar to `VK_PPC_AIX_TLS*`. but the symbol outputed seems to be too general and feels like we should fix this.  Shouldn't this be something more along the lines of `symbol at aix@tlsle`?
These are actual strings printed in ASM (i.e. the textual representation of the relocation) so we can't freely add `aix` in there since the assembler would not know what to do with that.


================
Comment at: llvm/lib/Target/PowerPC/MCTargetDesc/PPCMCTargetDesc.cpp:155
+      // If the variant kind is VK_PPC_AIX_TLSLE the entry represents the
+      // variable offset for the symbol, we add the relocation specifier @le.
       if (Kind == MCSymbolRefExpr::VariantKind::VK_PPC_AIX_TLSGD ||
----------------
lei wrote:
> These comments seem to be something appeneded each time a new condtion was added.  Can we clean up lines 150-155 to be more general and representative? eg
> ```
> // If the variant kind is a type of VK_PPC_AIX_TLS*, the entry represents the region handle for the symbol and
> // we add the corresponding relocation specifier @[m|gd|le].
> ```
I think it makes sense to unify the `gd/le`, but `m` is the region handle while the other two are variable offsets. Or we can simply state something like:
```
// On AIX, we have a region handle (symbol at m) and the variable offset
// (symbol@{gd|le}) for TLS variables, depending on the TLS model.
```
The details for which is which should be obvious from the code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149722



More information about the llvm-commits mailing list