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

[Amy Kwan via Phabricator via llvm-commits]

amyk marked an inline comment as done.
amyk added a comment.

Leaving comments from a group code review.



================
Comment at: llvm/lib/Target/PowerPC/MCTargetDesc/PPCXCOFFObjectWriter.cpp:112
+    case MCSymbolRefExpr::VK_PPC_AIX_TLSLE:
+      return {XCOFF::RelocationType::R_TLS_LE, SignAndSizeForFKData};
     case MCSymbolRefExpr::VK_None:
----------------
Verify if this change is appropriate for `SignAndSizeForFKData`.


================
Comment at: llvm/lib/Target/PowerPC/PPC.h:132
+    /// the thread pointer and the symbol can be used for the TLS Initial Exec
+    /// Local Exec models.
     MO_TPREL_FLAG = 64,
----------------
Missing an `and` in this comment.


================
Comment at: llvm/lib/Target/PowerPC/PPCInstr64Bit.td:1911
           (ADDIS8 $in, tblockaddress:$g)>;
 
 // AIX 64-bit small code model TLS access.
----------------
- Clean up comments to combine both this pattern and the one below.
- Expand this comment to say that it's used for global-dynamic, or loading for local-exec (offset for the TLS variable is in the TOC)
- Remove code model comments from the `PPCaddTls`.


================
Comment at: llvm/lib/Target/PowerPC/PPCInstr64Bit.td:1916
 
+// The following pattern is needed to match a local-exec TLS access on AIX
+// (64-bit) for both small and large code models. The typical AIX 64-bit access
----------------
- Remove this comment, as it is not necessary to explain the small/large code model intricacies here.
- Add the comment on why we need ADDTLS (in order to add the offset to r13, and in case we can optimized the user of the address).


================
Comment at: llvm/lib/Target/PowerPC/PPCInstr64Bit.td:1924
+// For the large code model, we match the load (addis+ld) in PPCISelDAGToDAG.
+def : Pat<(PPCaddTls i64:$in, i64:$addr),
+          (ADD8TLS $in, $addr)>;
----------------
kamaub wrote:
> Should this have an IsAIX guard?
We've discussed this - having a guard specifically for AIX should not be necessary as producing the `PPCaddTls` node with two `i64` inputs is only possible on AIX.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149722