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

[Digger via Phabricator via llvm-commits]

DiggerLin added inline comments.


================
Comment at: llvm/include/llvm/MC/MCAsmInfo.h:98
+  /// .global, .weak, .extern ,.comm.   Default is true.
+  bool HasVisibilityDirective = true;
+
----------------
hubert.reinterpretcast wrote:
> Suggestion:
> `HasVisibilityOnlyWithLinkage = false`
change as suggestion


================
Comment at: llvm/include/llvm/MC/MCStreamer.h:569
 
+  virtual void EmitXCOFFSymbolAttrWithVisibility(MCSymbol *Symbol,
+                                                 MCSymbolAttr Attribute,
----------------
hubert.reinterpretcast wrote:
> Are all the attributes emitted "with visibility" linkage attributes? If so, maybe `EmitXCOFFSymbolLinkageWithVisibility`?
not all attribute emitted with "with visibility" . changed as suggestion.


================
Comment at: llvm/lib/MC/MCXCOFFStreamer.cpp:51
+  // Fixed me: Not implement Visibility on the object file.
+  // EmitSymbolAttribute(Symbol,Visibilitily);
+}
----------------
hubert.reinterpretcast wrote:
> Silently incorrect output is not desirable. Uncomment this and ensure that we get a sensible failure.
OK. I uncomment the EmitSymbolAttribute(Symbol,Visibility); 
it will hit  report_fatal_error("Not implemented yet.");


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75866