[PATCH] D70461: [AIX] Emit TOC entries for ASM printing

[Jason Liu via Phabricator via llvm-commits]

jasonliu added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/MCTargetDesc/PPCMCTargetDesc.cpp:111
 
-  void emitTCEntry(const MCSymbol &S) override {
+  void emitTCEntry(const MCSymbol &Entry, const MCSymbol &Target) override {
+    const MCAsmInfo *MAI = Streamer.getContext().getAsmInfo();
----------------
daltenty wrote:
> jasonliu wrote:
> > I'm on the fence of this interface change.
> > I could see this interface change makes emitTCEntry a bit cleaner, and it models the TC entry a bit better: a symbol represent the real TC, and a symbol represent the item that TC is trying to store. 
> > But on the other hand, I don't think we are ever getting ".tc A[TC], B", and we always have ".tc A[TC], A" (at least for now). Using the new interface would kinda make that implicit connection lost. 
> If that relationship is something we want to preserve, we could add an assert of the form:
> 
> ```
> assert(Entry.getName().equals(MAI->getSymbolsHaveSMC() ? Target.getUnqualifiedName()  : Target.getName()) && "TOC entries should normally have the same name as their targets")
> ```
Then do we really need two MCSymbol as parameters? I feel like it just complicates the relationship. And one MCSymbol is enough, even on AIX.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70461