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

[Lei Huang via Phabricator via llvm-commits]

lei added inline comments.


================
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 ||
----------------
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].
```


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:824
+      TLSModel::Model Model = TM.getTLSModel(MO.getGlobal());
+      if (Model == TLSModel::LocalExec)
+        return MCSymbolRefExpr::VariantKind::VK_PPC_AIX_TLSLE;
----------------
I dont' think we need the tmp variable `Model`, can just update the if stmt thus:
```
if (TM.getTLSModel(MO.getGlobal() == TLSModel::LocalExec)
```


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:3332
 
-  // The general-dynamic model is the only access model supported for now, so
-  // all the GlobalTLSAddress nodes are lowered with this model.
+  if (Model == TLSModel::LocalExec) {
+    if (Is64Bit) {
----------------
I don't think `Model` is used anywhere else... can probably inline that as well.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:3333
+  if (Model == TLSModel::LocalExec) {
+    if (Is64Bit) {
+      // For local-exec on AIX (64-bit), the sequence that is generated involves
----------------
`Is64Bit` is only used once.  Please inline and remove this variable def.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149722