[PATCH] D100956: [AIX][TLS] Add ASM portion changes to support TLSGD relocations to XCOFF objects

[Sean Fertile via Phabricator via llvm-commits]

sfertile added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/MCTargetDesc/PPCMCTargetDesc.cpp:141
+      // represents the region handle/variable offset for the symbol, we add
+      // the relocation specifier @m/@gd.
+      if (Kind == MCSymbolRefExpr::VariantKind::VK_PPC_AIX_TLSGD ||
----------------
Real minot nit: I think it might be a bit clearer to break this comment into 2 separate sentences, one for the region handle and its variant-kind/relocation-specifier and one for the variable offset and its variant-kind/relocation-specifier. 


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:530
   StringRef Name = Subtarget->isAIXABI() ? ".__tls_get_addr" : "__tls_get_addr";
   MCSymbol *TlsGetAddr = OutContext.getOrCreateSymbol(Name);
   MCSymbolRefExpr::VariantKind Kind = MCSymbolRefExpr::VK_None;
----------------
Its kind of odd to create this symbol just to end up creating the proper symbol for AIX later in the AIX specific handling. In the existing code its a little sloppy but the second call to `OutContext.getOrCreateSymbol(Name);` at least looks up the same symbol created here. When we create the csect and get its qualname symbol, is that the same MCSymbol as created on this line?

My suggestion is we move creating this symbol down to just after the AIX specific code returns (and if we end up creating the helper function for creating the qual-name symbol for .__tls_get_addr then the name shouldn't be need here either and can also be moved).


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:561
            "GETtls[ld]ADDR[32] must read GPR4");
-    MCSymbol *TlsGetAddrA = OutContext.getOrCreateSymbol(Name);
+    MCSymbol *TlsGetAddrA =
+        OutContext
----------------
We have this same block of code twice in this file (here and line 2320), maybe is it worthwhile to move it to a descriptively named helper function?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100956