[PATCH] D54925: [NFC] [PowerPC] add an routine in PPCTargetLowering to determine if a global is accessed as toc-indirect or not.

[Sean Fertile via Phabricator via llvm-commits]

sfertile added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:4901
 
-    if (GlobalAddressSDNode *G = dyn_cast<GlobalAddressSDNode>(GA)) {
-      const GlobalValue *GV = G->getGlobal();
-      unsigned char GVFlags = PPCSubTarget->classifyGlobalReference(GV);
-      if (GVFlags & PPCII::MO_NLP_FLAG) {
-        SDNode *MN = CurDAG->getMachineNode(PPC::LDtocL, dl, MVT::i64, GA,
-                                            SDValue(Tmp, 0));
-        transferMemOperands(N, MN);
-        ReplaceNode(N, MN);
-        return;
-      }
-    }
-
+    // The TOC entry keeps the global address.
     ReplaceNode(N, CurDAG->getMachineNode(PPC::ADDItocL, dl, MVT::i64,
----------------
In this case we don't  keep the address of the global in the .toc/.got so the comment is misleading, maybe something more like:

```// Build the address relative to the TOC-pointer.```


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:13730
+
+  // If the global reference is actually to a non-lazy-pointer, we have to
+  // do an extra load to get the address of the global.
----------------
I believe Non-Lazy-Pointer is strictly a mac-os construct (not sure exactly what it is). I think the comment here would be less confusing if it simply mentions that the NLP flag indicates that a global access has to use an extra indirection.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.h:793
+    // Returns true if the address of the global is stored in TOC entry.
+    bool isAccessedAsTOCIndirect(SDValue N) const;
+
----------------
I would prefer to keep consistent with the V2 abi naming and call this 'GotIndirect' instead of TOCIndirect, but don't feel super strongly about it.


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

https://reviews.llvm.org/D54925