[PATCH] D76932: [AIX] emit .extern and .weak directive linkage

Hubert Tong via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Apr 8 21:35:01 PDT 2020


hubert.reinterpretcast added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:403
   switch (Linkage) {
   case GlobalValue::CommonLinkage:
   case GlobalValue::LinkOnceAnyLinkage:
----------------
I have my doubts that `CommonLinkage` should produce `.weak` for XCOFF. It is working right because we don't actually get here from `PPCAIXAsmPrinter::emitGlobalVariable` for that case. I'm not sure what the best way would be to add an assertion here though.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:439
+  case GlobalValue::ExternalWeakLinkage:
+    OutStreamer->emitSymbolAttribute(GVSym, MCSA_Weak);
+    return;
----------------
Similarly, this is okay without extra checks because non-XCOFF cases should never get here. A check would be nice but I'm not sure how to formulate it in this context.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-extern-weak.ll:11
+
+ at foo_ext_weak_p = global void (...)* bitcast (void ()* @foo_ext_weak to void (...)*)
+
----------------
I would prefer to test the function call and the take-address-of-function cases separately here.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-extern-weak.ll:13
+
+declare extern_weak void @foo_ext_weak() 
+
----------------
There should be testing for `extern_weak` variables as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76932





More information about the cfe-commits mailing list