[PATCH] D152669: [AIX][TLS] Generate 32-bit local-exec access code sequence

[Nemanja Ivanovic via Phabricator via llvm-commits]

nemanjai accepted this revision.
nemanjai added a comment.

LGTM aside from some minor nits.



================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:616-626
+  if (MIOpc == PPC::GETtlsTpointer32AIX)
+    return Ctx
+        .getXCOFFSection(".__get_tpointer", SectionKind::getText(),
+                         XCOFF::CsectProperties(XCOFF::XMC_PR, XCOFF::XTY_ER))
+        ->getQualNameSymbol();
+  else
+    return Ctx
----------------
Can this not be something like:
```
StringRef SymName = MIOpc == PPC::GETtlsTpointer32AIX ?
  ".__get_tpointer" : ".__tls_get_addr";

return Ctx.getXCOFFSection(SymName, SectionKind::getText(),
          XCOFF::CsectProperties(XCOFF::XMC_PR, XCOFF::XTY_ER))
  ->getQualNameSymbol();
```


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:628
+
+void PPCAsmPrinter::EmitAIXTlsCallHelper(const MachineInstr *MI) {
+  assert(Subtarget->isAIXABI() &&
----------------
Can this not be a `static` file-scope function?


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:1742
+  case PPCISD::GET_TPOINTER:
+                                return "PPCISD::GET_TPOINTER";
   case PPCISD::ADDI_TLSGD_L_ADDR: return "PPCISD::ADDI_TLSGD_L_ADDR";
----------------
Not sure if it was `clang-format` that suggested this, but please put these on the same line so that it matches the surrounding formatting.


================
Comment at: llvm/lib/Target/PowerPC/PPCTLSDynamicCall.cpp:59
         IsPCREL = isPCREL(MI);
+        bool IsTLSTPRelMI = MI.getOpcode() == PPC::GETtlsTpointer32AIX;
 
----------------
Please add a comment since I don't think the name of the variable can adequately represent what it is for. Something like:
```
// There are a number of slight differences in code generation
// when we call .__get_tpointer (32-bit AIX TLS).
```


================
Comment at: llvm/lib/Target/PowerPC/PPCTLSDynamicCall.cpp:158
+            // which will return the thread pointer into r3.
+            BuildMI(MBB, I, DL, TII->get(Opc2), GPR3);
+          }
----------------
Nit: please refrain from putting single statements in braces.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152669