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.reinterpretcast wrote:
> DiggerLin wrote:
> > jasonliu wrote:
> > > This block of code and D78045 will have conflict. One of us will need to
> > > rebase.
> > the one who land later will rebase.
> My understanding is that this would need a semantic reconciliation. I'd like
> to see the rebase of this patch before approving. Also, this would be another
> place to look into for the follow-up mentioned in
> https://reviews.llvm.org/D78045?id=257331#inline-714634 @jasonliu.
rebase the patch on the D78045
================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1641
- // External global variables are already handled.
- if (GV->isDeclaration())
+ if (GV->isDeclaration()) {
+ emitLinkage(GV, Csect->getQualNameSymbol());
----------------
hubert.reinterpretcast wrote:
> This should probably be `isDeclarationForLinker`. It seems we need a larger
> followup for `AvailableExternallyLinkage` that would involve checking our
> calls to `isDeclaration`:
>
> ```
> define void @_Z1gv() {
> entry:
> call void @_Z1fIiEvv()
> ret void
> }
>
> ; Function Attrs: inlinehint nounwind
> define available_externally void @_Z1fIiEvv() #0 {
> entry:
> ret void
> }
>
> attributes #0 = { inlinehint nounwind }
> ```
bool isDeclarationForLinker() const {
if (hasAvailableExternallyLinkage())
return true;
return isDeclaration();
}
since we do not deal with AvailableExternallyLinkage in
AsmPrinter::emitLinkage()
if change to isDeclarationForLinker here , it will hit a report_fatal_error.
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
