[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/include/llvm/MC/MCStreamer.h:568
 
+  /// Emit a symbol's linkage and visibilty with linkage directive for XCOFF.
+  ///
----------------
Still missing "a" before linkage.


================
Comment at: llvm/include/llvm/MC/MCStreamer.h:573
+  /// \param Visibility - The visibility of the symbol to emit or MCSA_Invalid
+  /// is the symbol does not have an explicit visibility.
+  virtual void emitXCOFFSymbolLinkageWithVisibility(MCSymbol *Symbol,
----------------
There was a typo in my suggestion:
s/is/if/;


================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1534
 
-    emitVisibility(Name, V, false);
+    // Get the function entry point symbol.
+    MCSymbol *FnEntryPointSym = TLOF.getFunctionEntryPointSymbol(&F, TM);
----------------
Suggestion:
```
    // Handle the XCOFF case.
    // `Name` is the function descriptor symbol (see above). Get the function
    // entry point symbol.
```


================
Comment at: llvm/lib/MC/MCAsmInfoXCOFF.cpp:19
   PrivateLabelPrefix = "L..";
+  HasVisibilityOnlyWithLinkage = true;
   SupportsQuotedNames = false;
----------------
Move this to right after `IsLittleEndian` to maintain matching the base class order.


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1606
+  case GlobalValue::InternalLinkage:
+    assert(MAI->hasDotLGloblDirective() && "Aix has .globl Directive.");
+    OutStreamer->emitSymbolAttribute(GVSym, MCSA_LGlobal);
----------------
The string should be "Expecting .lglobl to be supported for AIX."


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75866