[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/Target/PowerPC/PPCAsmPrinter.cpp:1600
+  case GlobalValue::InternalLinkage:
+    OutStreamer->emitSymbolAttribute(GVSym, MCSA_LGlobal);
+    return;
----------------
Should there be an assert for `MAI->hasDotLGloblDirective()`?


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1603
+  case GlobalValue::AppendingLinkage:
+    report_fatal_error("AppendingLinkage not yet implement");
+  case GlobalValue::CommonLinkage:
----------------
```
llvm_unreachable("Should never emit this");
```
is right. This linkage type is an LLVM IR artifact.


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1605
+  case GlobalValue::CommonLinkage:
+    report_fatal_error("CommonLinkage of XCOFF should not come to this path");
+  }
----------------
I believe that `llvm_unreachable` is generally correct for cases where the source of the error condition is not from "user input" but instead from API usage error.


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1606
+    report_fatal_error("CommonLinkage of XCOFF should not come to this path");
+  }
+
----------------
Please assert here that `LinkageAttr` is not still `MCSA_Invalid`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75866