jasonliu added inline comments.
================ Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1486 // Emit visibility info for declarations for (const Function &F : M) { ---------------- Comment should change to something similar to: `Emit linkage(XCOFF) and visibility info for declarations.` ================ Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1510 + continue; + } + ---------------- What if we have ``` void foo(); void (*foo_ptr)() = foo; int bar() { foo(); } ``` We would need both `.extern .foo` and `.extern foo[DS]`. Also, please have a similar case into the lit test. ================ Comment at: llvm/lib/MC/MCXCOFFStreamer.cpp:38 case MCSA_Global: + case llvm::MCSA_Extern: Symbol->setStorageClass(XCOFF::C_EXT); ---------------- Please remove `llvm::` for MCSA_Extern and MCSA_Weak to make the style consistent. ================ Comment at: llvm/lib/MC/MCXCOFFStreamer.cpp:48 + Symbol->setStorageClass(XCOFF::C_WEAKEXT); + Symbol->setExternal(true); + break; ---------------- Maybe we should just move `Symbol->setExternal(true);` outside of the switch, as it is set for every attribute that we are going to emit. ================ Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:351 + if (nameShouldBeInStringTable(ContainingCsect->getSectionName())) + Strings.add(ContainingCsect->getSectionName()); + } ---------------- We should `continue` here if the rest of the logic does not matter. ================ Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:352 + Strings.add(ContainingCsect->getSectionName()); + } // If the symbol is the csect itself, we don't need to put the symbol ---------------- A new line after '}'. ================ Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:353 + } // If the symbol is the csect itself, we don't need to put the symbol // into csect's Syms. ---------------- line 353 to 365 did not align properly. ================ Comment at: llvm/test/CodeGen/PowerPC/aix-extern-weak.ll:17 + +define weak void @foo_weak() #0 { +entry: ---------------- Nit: Please remove #0, #1 from the test case. ================ Comment at: llvm/test/CodeGen/PowerPC/aix-extern-weak.ll:55 + +; COMMON: .weak foo_weak[DS] # -- Begin function foo_weak +; COMMON-NEXT: .weak .foo_weak ---------------- A proper space alignment would make the expected result more readable. 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