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

[Digger via Phabricator via llvm-commits]

DiggerLin added inline comments.


================
Comment at: llvm/lib/MC/MCAsmStreamer.cpp:828
+
+  switch (Visibilitily) {
+  default:
----------------
hubert.reinterpretcast wrote:
> Why no `MCSA_Internal` case?
not sure when the llvm will emit a internal visibility.


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1592
+  default:
+    break;
+  case GlobalValue::HiddenVisibility:
----------------
hubert.reinterpretcast wrote:
> There should be a limited number of VisibilityTypes, make this a covering switch.
yes, there are only three visibility
  /// An enumeration for the kinds of visibility of global values.
  enum VisibilityTypes {
    DefaultVisibility = 0,  ///< The GV is visible
    HiddenVisibility,       ///< The GV is hidden
    ProtectedVisibility     ///< The GV is protected
  };


================
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?
in the patach, we only deal with the GlobalValue::ExternalLinkage
for other linkage , we do not deal with in the patch.
  case GlobalValue::CommonLinkage:
  case GlobalValue::LinkOnceAnyLinkage:
  case GlobalValue::LinkOnceODRLinkage:
  case GlobalValue::WeakAnyLinkage:
  case GlobalValue::WeakODRLinkage:

  we do not deal with this moment, we call the  AsmPrinter::emitLinkage

we maybe need to deal above linkage later in other patch.


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1607
+    // If external, declare as a global symbol: .globl _foo
+    if (GV->isDeclaration())
+      OutStreamer->EmitXCOFFSymbolLinkageWithVisibility(GVSym, MCSA_Extern,
----------------
hubert.reinterpretcast wrote:
> Would it make sense to do
> `GV->isDeclaration() ? MCSA_Extern : MCSA_Global`
> ?
if the GV only is declare. we will emit .extern	Name [ , Visibility ]  here 
if the GV is define . we will emit .globl	Name [, Visibility ]


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75866