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 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.
================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:445
+ }
+ llvm_unreachable("Should never emit this");
case GlobalValue::AppendingLinkage:
----------------
We can fall through using an annotation to suppress the warning:
```
LLVM_FALLTHROUGH;
```
================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1505
+
+ MCSectionXCOFF *Csect = cast<MCSectionXCOFF>(
+ getObjFileLowering().getSectionForExternalReference(&F, TM));
----------------
A comment here to explain that we are emitting linkage for the function
descriptor would help.
================
Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:2014
case GlobalValue::LinkOnceODRLinkage:
+ case GlobalValue::LinkOnceAnyLinkage:
+ case GlobalValue::WeakAnyLinkage:
----------------
Swap the order of the two `LinkOnce*` cases to match their definition order.
================
Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:2021
"There is no mapping that implements AppendingLinkage for XCOFF.");
default:
report_fatal_error(
----------------
Since `AvailableExternallyLinkage` is the only value left, just replace
`default` with that and change the `report_fatal_error` to say
`AvailableExternallyLinkage` for its first instance of "linkage".
================
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());
----------------
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 }
```
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
