[PATCH] D68721: [NFC][PowerPC]Clean up PPCAsmPrinter for TOC related pseudo opcode

[Jinsong Ji via Phabricator via llvm-commits]

jsji accepted this revision.
jsji added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: wuzish.

LGTM. Some nit comments.



================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:741
+    // Map the machine operand to its corresponding MCSymbol.
+    const MCSymbol *MOSymbol = getMCSymbolForTOCPseudoMO(MO, *this);
+
----------------
`MOSymbol` used only once here, maybe inline the call directly?


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:761
+    // an external symbol, is a jump table address, or is a block address; or if
+    // the large code model is enabled then generate a TOC entry and reference
+    // that. Otherwise, reference the symbol directly.
----------------
large code model only for CPI?


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:774
     if (GlobalToc || MO.isJTI() || MO.isBlockAddress() ||
-        TM.getCodeModel() == CodeModel::Large)
+	(MO.isCPI() && TM.getCodeModel() == CodeModel::Large))
       MOSymbol = lookUpOrCreateTOCEntry(MOSymbol);
----------------
Are these clang formatted? Formatting looks weird to me.


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:797
+    // an external symbol, is a jump table address, is a block address; or if
+    // large code model is enabled then generate a TOC entry and reference that.
+    // Otherwise reference the symbol directly.
----------------
large code model only for CPI?


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:835
+
+    const MCSymbol *MOSymbol = getMCSymbolForTOCPseudoMO(MO, *this);
+
----------------
MOSymbol used only once here, maybe inline the call directly?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68721