[PATCH] D82481: [XCOFF][AIX] Give symbol an internal name when desired symbol name contains invalid character(s)

[Digger via Phabricator via llvm-commits]

DiggerLin added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1610
+           "InternalLinkage should not have other visibility setting.");
+    LinkageAttr = MCSA_LGlobal;
+    break;
----------------
jasonliu wrote:
> DiggerLin wrote:
> > I think it maybe better to call emitSymbolAttribute(GVSym, MCSA_LGlobal)  and then return directly,
> > 
> > Reason: 
> > 1. for the .lglobl do not have have visibility, it do not need to go to emitXCOFFSymbolLinkageWithVisibility() ,
> > 2. we can move the code  
> >    if (cast<MCSymbolXCOFF>(Symbol)->hasRename())
> >     OutStreamer->emitXCOFFRenameDirective(Symbol,
> >                              cast<MCSymbolXCOFF>(Symbol)->getSymbolTableName());
> > 
> > into this function.
> > 
> > 3.  if you call  emitSymbolAttribute directly, we do not need to modify the function  emitXCOFFSymbolLinkageWithVisibility() in this patch.
> We only want to emitXCOFFRenameDirective when it's emitting assembly though.
> And emitLinkage is called on both assembly generating path and object generation path, so we can't move that call directly to here.
> It's still possible to make it happen by wrapping emitXCOFFRenameDirective with another new function(something like handleXCOFFRename?), and have that function do emitXCOFFRenameDirective on the assembly generation side, but do nothing on the object file generation side.
> But I don't find adding that new function to be particular appealing though (we need to add so many extra boilerplate to tell other target that this function is not meant for them, we already did it for emitXCOFFRenameDirective but that's necessary evil because that directive is indeed not supported by other target).
If we change the function 
```
void MCStreamer::emitXCOFFRenameDirective(const MCSymbol *Name,
                                          StringRef Rename) {
  llvm_unreachable("emitXCOFFRenameDirecitve is only supported on "
                   "XCOFF targets");
}
```
to 
```
void MCStreamer::emitXCOFFRenameDirective(const MCSymbol *Name,
                                          StringRef Rename) {

}
```
and object generation  path will do nothing for the emitXCOFFRenameDirective(), it just some overhead for object generation  path but the logic will be  more clear.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82481/new/

https://reviews.llvm.org/D82481