DiggerLin marked 16 inline comments as done. DiggerLin added inline comments.
================ Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1500 + if (cast<MCSymbolXCOFF>(Name)->hasContainingCsect()) + emitLinkage(&F, Name); + ---------------- jasonliu wrote: > 1. We need to rebase here, as it is called `hasRepresentedCsectSet()` instead > of `hasContainingCsect()` now. > 2. I'm slightly worried here to rely on `hasRepresentedCsectSet()` to check > if a linkage should be emitted. This is based on the assumption that we will > not ever change our implementation for `.bl foo` to `.bl foo[PR]`. But in > https://reviews.llvm.org/D77080#inline-706207, we discussed about this > possibility. So this assumption might not be true in the future. However, I'm > not sure if there is another way to check if this function have been called > directly. > So if there is another way to check, we should pursue the alternative > instead. If there is not, then we need to add an assert here, like > `assert(Name->getName().equals(cast<MCSymbolXCOFF>(Name)->getUnqualifiedName())` > to make sure we don't get a qualname here. > 3. > Have a comment here and tell people what we are doing here. > For example, > // If there is a direct call to external function, then we need to emit > linkage for its function entry point. when we implement .bl foo to .bl foo[PR] the SymbolName will change from .bl[SMC] and check the .bl[SMC]->hasRepresentedCsectSet() ================ Comment at: llvm/test/CodeGen/PowerPC/aix-WeakODRLinkage.ll:10 + ret void +} + ---------------- jasonliu wrote: > Do we also want to test WeakAnyLinkage? WeakAnyLinkage should be weak It has been tested in aix-weak.ll define weak void @foo_weak(i32* %p) { entry: %p.addr = alloca i32*, align 4 store i32* %p, i32** %p.addr, align 4 %0 = load i32*, i32** %p.addr, align 4 %1 = load i32, i32* %0, align 4 %inc = add nsw i32 %1, 1 store i32 %inc, i32* %0, align 4 ret void } 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
