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

MaskRay added inline comments. Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:403 switch (Linkage) { case GlobalValue::CommonLinkage: case GlobalValue::LinkOnceAnyLinkage: hubert.reinterpretcast wrote: > I have my doubts that `CommonLinkage` shou

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

This revision was automatically updated to reflect the committed changes. Closed by commit rGa2c8cd18128d: [AIX] emit .extern and .weak directive linkage (authored by DiggerLin). Changed prior to commit: https://reviews.llvm.org/D76932?vs=261004&id=261219#toc Repository: rG LLVM Github Monor

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

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. LGTM with minor comment. Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:2143 + case GlobalValue::AvailableExternallyLinkage: +report_fatal_error("Unhandled AvailableExtern

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

DiggerLin updated this revision to Diff 261004. DiggerLin marked an inline comment as done. DiggerLin added a comment. take out the functionality of "remove -u from clang" Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76932/new/ https://reviews.ll

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

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. LGTM with minor comment. Comment at: llvm/test/CodeGen/PowerPC/aix-extern-weak.ll:8 +; RUN: llc -verify-machineinstrs -mtriple powerpc-ibm-aix-xcoff -mcpu=pwr4 \ +; RUN: -mattr=-altivec < %s

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

DiggerLin updated this revision to Diff 259057. DiggerLin marked an inline comment as done. DiggerLin added a comment. address comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76932/new/ https://reviews.llvm.org/D76932 Files: clang/lib/Driv

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

DiggerLin marked 2 inline comments as done. DiggerLin added inline comments. Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:2146 } } hubert.reinterpretcast wrote: > Replicate the > ``` > llvm_unreachable("Unknown linkage type!"); > ``` > here

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

hubert.reinterpretcast added inline comments. Comment at: llvm/test/CodeGen/PowerPC/aix-extern-weak.ll:174 +; CHECKSYM-NEXT: Index: [[#Index+11]] +; CHECKSYM-NEXT: ContainingCsectSymbolIndex: 8 +; CHECKSYM-NEXT: ParameterHashIndex: 0x0 This shou

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

hubert.reinterpretcast added inline comments. Comment at: llvm/test/CodeGen/PowerPC/aix-extern-weak.ll:9 +; RUN: -mattr=-altivec -filetype=obj -o %t.o < %s +; RUN: llvm-readobj --symbols %t.o | FileCheck --check-prefix=CHECKSYM %s + No need for two consecutive s

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

DiggerLin updated this revision to Diff 258732. DiggerLin marked 2 inline comments as done. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76932/new/ https://reviews.llvm.org/D76932 Files: clang/lib/Driver/ToolChains/AIX.cpp clang/test/Driver/aix

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

hubert.reinterpretcast added inline comments. Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1509 + +MCSymbol *Name = getSymbol(&F); +if (TM.getTargetTriple().isOSBinFormatXCOFF() && !F.isIntrinsic()) { Add a comment that this gives us the functio

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

DiggerLin updated this revision to Diff 258442. DiggerLin added a comment. I think I address all the comments. thanks hubert. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76932/new/ https://reviews.llvm.org/D76932 Files: clang/lib/Driver/ToolCh

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

DiggerLin updated this revision to Diff 258432. DiggerLin added a comment. handle getSymbol returning a function descriptor symbol after rebase on the D78045 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76932/new/

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

hubert.reinterpretcast added inline comments. Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:2144 report_fatal_error( -"Unhandled linkage when mapping linkage to StorageClass."); +"AvailableExternallyLinkage is for its first instance of linkage

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

DiggerLin marked 7 inline comments as done. DiggerLin added inline comments. Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1496 + +MCSymbol *Name = getSymbol(&F); +if (TM.getTargetTriple().isOSBinFormatXCOFF() && !F.isIntrinsic()) { hubert.reinte

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

DiggerLin updated this revision to Diff 258409. DiggerLin marked 2 inline comments as done. DiggerLin added a comment. rebase the patch on the D78045 and address comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

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

hubert.reinterpretcast added inline comments. Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1496 + +MCSymbol *Name = getSymbol(&F); +if (TM.getTargetTriple().isOSBinFormatXCOFF() && !F.isIntrinsic()) { DiggerLin wrote: > jasonliu wrote: > > This

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

jasonliu accepted this revision. jasonliu added a comment. This revision is now accepted and ready to land. LGTM. Please wait a day or two to see if @hubert.reinterpretcast have further comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76932/

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

DiggerLin updated this revision to Diff 257827. DiggerLin marked 2 inline comments as done. DiggerLin added a comment. address comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76932/new/ https://reviews.llvm.org/D76932 Files: clang/lib/Driv

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

DiggerLin marked 8 inline comments as done. DiggerLin added inline comments. Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:441 + case GlobalValue::ExternalWeakLinkage: +if (TM.getTargetTriple().isOSBinFormatXCOFF()) { + OutStreamer->emitSymbolAttribute(GVSym, M

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

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 o

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

DiggerLin updated this revision to Diff 257473. DiggerLin added a comment. address comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76932/new/ https://reviews.llvm.org/D76932 Files: clang/lib/Driver/ToolChains/AIX.cpp clang/test/Driver/ai

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

DiggerLin updated this revision to Diff 257319. DiggerLin marked 2 inline comments as done. DiggerLin added a comment. address comment and add a new test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76932/new/ https://reviews.llvm.org/D76932

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

jasonliu added inline comments. Comment at: llvm/test/CodeGen/PowerPC/aix-extern.ll:14 + +; Function Attrs: noinline nounwind optnone +define void @foo() #0 { nit: Function Attrs and `#0` could be removed Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

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

jasonliu added inline comments. Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1497 + OutContext.getOrCreateSymbol("." + Name->getName()); + assert(FnEntryPointSym->getName().equals( + cast(FnEntryPointSym)->getUnqualifiedName()) && -

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

jasonliu added inline comments. Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1500 + if (cast(Name)->hasContainingCsect()) +emitLinkage(&F, Name); + DiggerLin wrote: > jasonliu wrote: > > DiggerLin wrote: > > > jasonliu wrote: > > > > 1. We

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

DiggerLin marked 2 inline comments as done. DiggerLin added inline comments. Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1500 + if (cast(Name)->hasContainingCsect()) +emitLinkage(&F, Name); + jasonliu wrote: > DiggerLin wrote: > > jasonliu

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

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

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

jasonliu added inline comments. Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1500 + if (cast(Name)->hasContainingCsect()) +emitLinkage(&F, Name); + DiggerLin wrote: > jasonliu wrote: > > 1. We need to rebase here, as it is called `hasRepres

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

DiggerLin updated this revision to Diff 256067. DiggerLin marked 2 inline comments as done. DiggerLin added a comment. address comment and rebased the patch on D77080 [NFC][XCOFF][AIX] Refactor get/setContainingCsect Repository: rG LLVM Github Monorepo CHAN

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

DiggerLin marked 16 inline comments as done. DiggerLin added inline comments. Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1500 + if (cast(Name)->hasContainingCsect()) +emitLinkage(&F, Name); + jasonliu wrote: > 1. We need to rebase here, a

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

jasonliu added a comment. Thanks for splitting the test case. I think it helps a lot for reading and maintainability. Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1490 continue; GlobalValue::VisibilityTypes V = F.getVisibility(); + This l

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

DiggerLin added inline comments. Comment at: llvm/lib/MC/MCXCOFFStreamer.cpp:48 +Symbol->setStorageClass(XCOFF::C_WEAKEXT); +Symbol->setExternal(true); +break; jasonliu wrote: > Maybe we should just move `Symbol->setExternal(true);` outside of the swi

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

DiggerLin updated this revision to Diff 254846. DiggerLin marked 7 inline comments as done. DiggerLin added a comment. 1. address comment 2. add two new test cases. 3. split a bit test case into three small test case. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

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

jasonliu added inline comments. Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:407 case GlobalValue::WeakAnyLinkage: case GlobalValue::WeakODRLinkage: if (MAI->hasWeakDefDirective()) { Could we verify if these Linkage should also always emit .w

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

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

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

jasonliu added a comment. 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/m

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

DiggerLin updated this revision to Diff 254252. DiggerLin marked 3 inline comments as done. DiggerLin added a comment. address comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76932/new/ https://reviews.llvm.org/D76932 Files: clang/lib/Driv

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

DiggerLin marked 3 inline comments as done. DiggerLin added inline comments. Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1548 + + if (XCOFFSym->hasContainingCsect()) { +MCSymbolXCOFF *QualName = jasonliu wrote: > I hope we can find a better solutio

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

jasonliu added inline comments. Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1589 : getObjFileLowering().getSectionForFunctionDescriptor(F, TM)); - +if (F->isDeclaration()) { + MCSymbolXCOFF *FSym = cast(getSymbol(F)); If it's possi

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

jasonliu added inline comments. Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1548 + + if (XCOFFSym->hasContainingCsect()) { +MCSymbolXCOFF *QualName = I hope we can find a better solution here. IMO, we don't even want to override this function in t

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

jasonliu added inline comments. Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1492 + +if (MAI->hasDotExternDirective()) { + MCSymbol *Name = getSymbol(&F); This query asked if the target supports .extern. However, .extern is not the only direct

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

jasonliu added inline comments. Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1498 + } + emitLinkage(&F, Name); + continue; jasonliu wrote: > I'm surprised we do not enter here for `foo_ext_weak()`. The result of that > is we need to do s

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

hubert.reinterpretcast added inline comments. Comment at: llvm/include/llvm/MC/MCDirectives.h:47 + MCSA_WeakReference, ///< .weak_reference (MachO) + MCSA_WeakDefAutoPrivate ///< .weak_def_can_be_hidden (MachO) }; DiggerLin wrote: > @hubert.rei

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

DiggerLin marked an inline comment as done. DiggerLin added inline comments. Comment at: llvm/include/llvm/MC/MCDirectives.h:47 + MCSA_WeakReference, ///< .weak_reference (MachO) + MCSA_WeakDefAutoPrivate ///< .weak_def_can_be_hidden (MachO) };

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

DiggerLin updated this revision to Diff 253579. DiggerLin added a comment. delete -u from clang test case aix-as.c Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76932/new/ https://reviews.llvm.org/D76932 Files: clang/lib/Driver/ToolChains/AIX.cp

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

DiggerLin updated this revision to Diff 253281. DiggerLin added a comment. clang reformat Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76932/new/ https://reviews.llvm.org/D76932 Files: clang/lib/Driver/ToolChains/AIX.cpp llvm/include/llvm/Cod

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

DiggerLin created this revision. DiggerLin added reviewers: hubert.reinterpretcast, jasonliu, sfertile, daltenty. Herald added subscribers: cfe-commits, kbarton, hiraditya, nemanjai. Herald added a project: clang. DiggerLin edited the summary of this revision. Herald added a subscriber: wuzish. Dig