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

[Digger via Phabricator via cfe-commits]

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