[PATCH] D75866: [AIX] supporting the visibility attribute for aix assembly

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: llvm/lib/MC/MCAsmStreamer.cpp:821
+    if (Symbol->getName() == QualName->getUnqualifiedName() &&
+        XCOFFSym->getContainingCsect()->getMappingClass() != XCOFF::XMC_PR)
+      QualName->print(OS, MAI);
----------------
I am somewhat surprised to encounter this block of code as being something new in the codebase since the problem it solves should not be a "new problem".

If I understand this correctly, this block of code solves a problem that this patch does need a solution for but is not the subject of this patch. In particular, we are still (without this patch) not emitting the linkage on the assembly path for references to functions that are not defined in the current translation unit. I would suggest splitting this out into a separate patch first. @jasonliu @daltenty, fya.


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1603
+  default:  
+    AsmPrinter::EmitLinkage(GV, GVSym);
+    return;
----------------
hubert.reinterpretcast wrote:
> Why is it okay to ignore visibility when encountering this case?
Also, please use the immediate base class when explicitly invoking a base implementation of a virtual function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75866