[PATCH] D68341: [AIX] TOC pseudo expansion for 64bit large + 64bit small + 32bit large modes

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:755
     const MCExpr *Exp =
-      MCSymbolRefExpr::create(TOCEntry, MCSymbolRefExpr::VK_PPC_TOC,
-                              OutContext);
+      MCSymbolRefExpr::create(TOCEntry, VK, OutContext);
     TmpInst.getOperand(1) = MCOperand::createExpr(Exp);
----------------
Similar cases below have two more spaces of indentation.


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:845
+    const MCSymbolRefExpr::VariantKind VK =
+        !IsAIX ? MCSymbolRefExpr::VK_PPC_TOC_HA : MCSymbolRefExpr::VK_PPC_U;
     const MCExpr *Exp =
----------------
See comment below re: the `!`.


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:887
+    const MCSymbolRefExpr::VariantKind VK =
+        !IsAIX ? MCSymbolRefExpr::VK_PPC_TOC_LO : MCSymbolRefExpr::VK_PPC_L;
     const MCExpr *Exp =
----------------
I believe @sfertile made a comment about avoiding the `!` in the condition in cases like these.


================
Comment at: llvm/test/CodeGen/PowerPC/lower-globaladdr64-aix-asm.ll:26
+; LARGE:       ld [[REG4:[0-9]+]], LC1 at l([[REG2]])
+; LARGE:       lwz [[REG4:[0-9]+]], 0([[REG3]])
+
----------------
This does not follow. `REG3` apparently holds the address of the operand for the load, so `REG4` holds the address of the target of the store. We are loading the value to `REG4` though, so its value will be clobbered before we get to the store.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68341