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

[Jason Liu via Phabricator via cfe-commits]

jasonliu added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:441
+  case GlobalValue::ExternalWeakLinkage:
+    if (TM.getTargetTriple().isOSBinFormatXCOFF()) {
+      OutStreamer->emitSymbolAttribute(GVSym, MCSA_Weak);
----------------
Maybe an assert on isOSBinFormatXCOFF is better?
If not, could we remove the else and llvm_unreachable to let it fall through? Will it have warnings?
Or we could just only remove `else` here.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1496
+
+    MCSymbol *Name = getSymbol(&F);
+    if (TM.getTargetTriple().isOSBinFormatXCOFF() && !F.isIntrinsic()) {
----------------
This block of code and D78045 will have conflict. One of us will need to rebase.


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1641
 
   // External global variables are already handled.
+  if (GV->isDeclaration()) {
----------------
This comment does not apply any more.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-extern-weak.ll:10
+; RUN: llvm-readobj  --symbols %t.o | FileCheck --check-prefix=CHECKSYM %s
+
+ at foo_ext_weak_p = global void (...)* bitcast (void ()* @foo_ext_weak_ref to void (...)*)
----------------
Do we care to check the 64 bit object generation error here and for other test cases that are applicable?


================
Comment at: llvm/test/CodeGen/PowerPC/aix-extern-weak.ll:29
+; COMMON-NEXT:    .globl	.main
+; COMMON-NEXT: 	  .align	4
+; COMMON-NEXT: 	  .csect main[DS]
----------------
Could we align the assembly output in aix-extern-weak.ll, aix-extern.ll, aix-weak.ll?


================
Comment at: llvm/test/CodeGen/PowerPC/aix-extern-weak.ll:48
+; COMMON-NEXT:    .weak   b_w[UA]
+; COMMON-NEXT:    .weak   foo_ext_weak_ref[DS]
+; COMMON-NEXT:    .weak   .foo_ext_weak
----------------
If the plan is not emit function entry point if it's not necessary, let's CHECK-NOT that. 


================
Comment at: llvm/test/CodeGen/PowerPC/aix-extern.ll:14
+
+; Function Attrs: noinline nounwind optnone
+define void @foo() #0 {
----------------
jasonliu wrote:
> nit: Function Attrs and `#0` could be removed
This comment is not addressed. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76932