[PATCH] D78929: [AIX][XCOFF]emit extern linkage for the llvm intrinsic symbol

[Sean Fertile via Phabricator via llvm-commits]

sfertile added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:5294
+      auto &Context = DAG.getMachineFunction().getMMI().getContext();
+      const MCSymbolXCOFF *Sym = cast<MCSymbolXCOFF>(
+          Context.getOrCreateSymbol(Twine(".") + Twine(SymName)));
----------------
DiggerLin wrote:
> sfertile wrote:
> > DiggerLin wrote:
> > > sfertile wrote:
> > > > Why not leave the name as is, and prepend the '.' when we create/lookup the symbol in the AsmPrinter?
> > > if leave name as is, the branch will be
> > > bl memset  
> > > not 
> > > bl .memset
> > > 
> > > if leave name as is , we want to generate bl .memset . we need to modify the operand(ExternalSymbol) of branch to generate correct bl .memset
> > Yes, which is why I said you would move where you prepend the '.' from here, to where you create the MCSymbol from the MO_ExternalSymbol in the ASM printer. 
> > 
> > I think our handling of when there is deceleration in the IR matching the ExtrernalSymbol name is wrong, and it should either be done for all subtargets, or none (but I'm not 100% sure of that yet and investigating). If they are to be common-ed, then by  moving modifying the symbols name too the ASMprinter, it lets us have identical code for all subtargets for transforming ExternalSymbolSDNode callees. 
> > 
> > I'd say hold off on making this change until we better understand the IR Decl/ExternalSym situation though.
> Discussed with Sean offline, I will keep the implement.
Expanding a bit: I though we could create the MCSymbol in `PPCAIXAsmPrinter::emitInstruction` and then create a new MCSymbol MachineOperand and change the operand to that. Digger explained how I was mistaken and we can not change the operand on the MachineInstr, so we have to stick with this approach.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78929