[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`

[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=261219#toc Repository: rG LLVM Github

[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

[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/

[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:

[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!"); > ``` >

[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

[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

[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

[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(); +if (TM.getTargetTriple().isOSBinFormatXCOFF() && !F.isIntrinsic()) { Add a comment that this gives us the function

[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:

[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

[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

[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(); +if (TM.getTargetTriple().isOSBinFormatXCOFF() && !F.isIntrinsic()) {

[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

[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(); +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

[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:

[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,

[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

[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

[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

[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(, 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(, 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

[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(, Name); + DiggerLin wrote: > jasonliu wrote: > > 1. We need to rebase here, as it is called

[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

[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(, Name); + jasonliu wrote: > 1. We need to rebase here, as

[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

[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

[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

[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

[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 : 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

[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:

[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

[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

[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

[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(); This query asked if the target supports .extern. However, .extern is not the only

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

jasonliu added inline comments. Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1498 + } + emitLinkage(, 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

[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: >

[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:

[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

[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.