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

[Amy Kwan via Phabricator via llvm-commits]

amyk added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPC.h:131
     /// MO_TPREL_FLAG - If this bit is set the symbol reference is relative to
-    /// TLS Initial Exec model.
+    /// TLS Initial Exec model, or for the TLS Local Exec or Initial Exec models
+    /// on AIX.
----------------
kamaub wrote:
> Accidentally repeated the initial comment?
> What is the difference between TLS Initial Exec model and Initial Exec models on AIX?
> 
> Follow up, I wondering since it is named TPREL to mean Thread pointer relative, which indicates using r13 instead of the get addr call like with dynamic models should we say something like:
> ```
>     /// MO_TPREL_FLAG - If this bit is set the symbol reference is relative to
>     /// the thread pointer and is used for TLS Initial Exec , or  TLS Local Exec model.
Thanks, Kamau. I've updated this comment. 

For initial-exec on AIX, it appears that the regular access sequence looks the same as the local-exec sequence, with the exception that we have a specific the initial exec relocation specifier (`ie`) for the TLS data in the TOC. 



================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:3336
+
+  if (Model == TLSModel::LocalExec) {
+    assert(Is64Bit &&
----------------
- Add documentation on what we're trying to do/produce for local-exec and the code sequence.
- Add the local exec code sequence in the patch description.
- Update the comment for `ADD_TLS` in `PPCISelLowering.h` as this node is not only used for initial-exec.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:3345
+      return DAG.getNode(PPCISD::ADD_TLS, dl, PtrVT, TLSReg, VariableOffset);
+    }
+    // TODO: Add an 'else' to account for generating the 32-bit access sequence.
----------------
Suggestion:
- Remove `assert()`.
- Add an `else` to handle the 32-bit behaviour for now. We can `report_fatal_error()` for 32-bit mode.
- Remove the TODO for 32-bit. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149722