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
+
+@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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
