[XCOFF][AIX] Fix getSymbol to return the correct qualname when necessary

[jasonliu]

Summary

AIX symbol have qualname and unqualified name. The stock getSymbol could only return unqualified name, which leads us to patch many caller side(lowerConstant, getMCSymbolForTOCPseudoMO). So we should try to address this problem in the callee side(getSymbol) and clean up the caller side instead.

Note: this is a "mostly" NFC patch, with a fix for the original lowerConstant behavior.

[jasonliu]

This comment was removed by jasonliu.

[jasonliu]

llvm/lib/Target/TargetMachine.cpp

263

To reviewers,
I'm not sure if this is a preferred way to inject target specific behavior. Or we want to override TargetMachine::getSymbol directly instead.
Please let me know if you find the other way is more appealing, or you could think of a better way to inject this target specific behavior.
Thanks.

[jasonliu]

llvm/include/llvm/Target/TargetLoweringObjectFile.h

241

specialied -> specialized.

[hubert.reinterpretcast]

llvm/include/llvm/Target/TargetLoweringObjectFile.h

240

Add "a" before "special".

llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

1975

Add "a" before "qualname". Replace "the" with "a" before "GV".

1993

Add "a" before "GV".

Let's indicate the specific reason why we expect that this would change when -fdata-sections is specified (namely that we could avoid having label symbols if the linkage name is applied to the csect symbol). User-specified section names can either cause such cases or get in the way of them.

llvm/test/CodeGen/PowerPC/aix-xcoff-lower-comm.ll

6

The object-file testing is not enough to check against the existence of additional symbols whose unqualified name is common. It is probably sufficient if we just verify that the relocation names the defined common[RW].

53

This should use a FileCheck variable.

[jasonliu]

Address comments.

[hubert.reinterpretcast]

llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

1994

No comma before "since".

llvm/lib/Target/TargetMachine.cpp

261

Suggestion:
XCOFF symbols have a special naming convention.

263

Can getTargetSymbol return nullptr to indicate that "falling through" to the generic path is okay? This means that the three lines below don't need to be duplicated.

llvm/test/CodeGen/PowerPC/aix-xcoff-lower-comm.ll

3

The two lines above have trailing whitespace.

27

I think {{[0-9]+}} or similar should be used here. The file does not need to have a .text section.

86

If it is important that there is no entry between pointer and common, then we can base everything off INDX. I don't think we are actually checking for that here, so we should use COM_INDX+1 in place of INDX+5.

[daltenty]

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

1748

Does it matter we're losing this early error check? Will we end up creating symbols for unanticipated GV kinds before we hit the corresponding check in emitGlobalVariable (and do we care)?

llvm/lib/Target/TargetMachine.cpp

263

This seems the right way to go to me. Going through the TLOF seems appropriate since this is really an object format property, doing a override in the TargetMachine hierarchy doesn't really seem to capture that.

[DiggerLin]

llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

1997

to be consistent with getSectionForExternalReference and getSectionForFunctionDescriptor , I am prefer to use getNameWithPrefix(NameStr, GV, TM); here

[hubert.reinterpretcast]

llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

1970

I understand the current caller does not have the necessary context, but target symbols for XCOFF are not limited to the csect versus label that this currently handles. It extends to function descriptor versus entry point.

Can we use an expanded version of this function to common up the entry point referencing between AsmPrinter::SetupMachineFunction and the transformCallee that is used by PPCTargetLowering::FinishCall?

[jasonliu]

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

1748

I think it's possible for us to create symbols for unanticpated GV kinds, but it will get caught in emitGlobalVariable if we don't do it in getTargetSymbol. I think it should be Okay if we could eventually catch it.

[jasonliu]

llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

1970

Relay some of the discussion from offline:
I feel adding a boolean to the parameter to determine if we need a function descriptor or entry point would make this function very XCOFF centric and complicate the interface.
I would try to add comments to make sure user of this function is clear that this function will only produce function descriptor. And in a separate patch, I will add a new function called getFunctionEntryPointSymbol to return the function entry point for every caller.

[jasonliu]

Address various comments.

[hubert.reinterpretcast]

I think this is pretty much there. It is important enough that I'd want explicit approval from another one of the reviewers as well though.

llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h

262

Comments about function descriptor versus entry point would go here?

llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

1976

Comment goes here to clarify that a GV representing the address of a function is inherently ambiguous between representing a function descriptor and a function entry point.

[jasonliu]

Updated comments to clarify function descriptor vs function entry point.

[DiggerLin]

llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

1987

add some thing like to test isBSSLocal by the way ?

@comm_bs = internal global i32 0, align 4
@ps = global i32* @comm_bs, align 4

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

1632

since we get the qualname symbol, I do not think we need to set
GVSym->setStorageClass (....) here.

1639

since we have created the MCSectionXCOFF *Csect in the getSymbol.
do we still need this ?

can we change to
if (GVSym->hasRepresentedCsectSet())

Csect = GVSym->getRepresentedCsect()

else {

Csect = getObjFileLowering().SectionForGlobal(
        : getObjFileLowering().SectionForGlobal(
              GV, GVKind = getObjFileLowering().getKindForGlobal(GV, TM),

}
?

[DiggerLin]

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

1643–1644

since we have a qualname symbol, I do not think we need to create and unnecessary UnqualifiedName symbol.
maybe we change change the interface of emitXCOFFLocalCommonSymbol. I do not think we need to pass two Symbols(MCSymbol *LabelSym, and MCSymbol *CsectSym) in the emitXCOFFLocalCommonSymbol.

[DiggerLin]

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

1632

sorry, please discard the comment, maybe label here, it still need to set the StorageClass() here

[hubert.reinterpretcast]

llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

1978

s/represents/represent/;

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

1643–1644

The label does not have to be given linkage, but it is not optional in the assembly syntax.

[jasonliu]

llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

1987

I don't feel a strong need about this, as this code path is going to get exercise by emitGlobalVariable and getMCSymbolForTOCPseudoMO, and so we know we will get a qualname for it and is covered by the old test cases already.

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

1639

Thanks. I don't think we need to get csect for external symbol. For the other symbol, even if it's qualname, we could still get it with SectionForGlobal query.
I would like to avoid the usage of hasRepresentedCsectSet() if possible. Whenever we use that interface means we have a potential timing issue (some are indeed by design) for csect setting.
So I adjust the code accordingly.

1643–1644

.lcomm a[BS],4,a[BS],2 does not work. First expression must be a label.

[hubert.reinterpretcast]

Thanks. I believe all of the comments have been addressed appropriately.

[DiggerLin]

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

1643–1644

we can change

LabelSym->print(OS, MAI);

to

OS << CsectSym->getUnqualifiedName();

in the function void MCAsmStreamer::emitXCOFFLocalCommonSymbol

[jasonliu]

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

1643–1644

I'd prefer to leave the interface as it is. The label and the csect does not necessarily have to be .lcomm a, 4, a[BS],2. It could be `.lcomm a, 4, b[BS],2'.
In the future, if we need to do an assembly paser, we could receive the case like above, and need to have label and csect represented separately.

[DiggerLin]

LGTM

[daltenty]

LGTM

llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h

getSectionForFunctionDescriptor(const Function *F,
                                   const TargetMachine &TM) const override;
   MCSection *getSectionForTOCEntry(const MCSymbol *Sym) const override;
 
   /// For external functions, this will always return a function descriptor
   /// csect.
   MCSection *
   getSectionForExternalReference(const GlobalObject *GO,
                                  const TargetMachine &TM) const override;
+
+  /// For functions, this will always return a function descriptor symbol.
+  MCSymbol *getTargetSymbol(const GlobalValue *GV,
+                            const TargetMachine &TM) const override;
 };
 
 } // end namespace llvm
 
 #endif // LLVM_CODEGEN_TARGETLOWERINGOBJECTFILEIMPL_H

llvm/include/llvm/Target/TargetLoweringObjectFile.h

public:
   /// On targets that associate external references with a section, return such
   /// a section for the given external global.
   virtual MCSection *
   getSectionForExternalReference(const GlobalObject *GO,
                                  const TargetMachine &TM) const {
     return nullptr;
   }
 
+  /// Targets that have a special convention for their symbols could use
+  /// this hook to return a specialized symbol.
+  virtual MCSymbol *getTargetSymbol(const GlobalValue *GV,
+                                    const TargetMachine &TM) const {
+    return nullptr;
+  }
+
 protected:
   virtual MCSection *SelectSectionForGlobal(const GlobalObject *GO,
                                             SectionKind Kind,
                                             const TargetMachine &TM) const = 0;
 };
 
 } // end namespace llvm
 
 #endif // LLVM_CODEGEN_TARGETLOWERINGOBJECTFILE_H

llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

MCSection *TargetLoweringObjectFileWasm::getStaticDtorSection(
     unsigned Priority, const MCSymbol *KeySym) const {
   llvm_unreachable("@llvm.global_dtors should have been lowered already");
   return nullptr;
 }
 
 //===----------------------------------------------------------------------===//
 //                                  XCOFF
 //===----------------------------------------------------------------------===//
+MCSymbol *
+TargetLoweringObjectFileXCOFF::getTargetSymbol(const GlobalValue *GV,
+                                               const TargetMachine &TM) const {
+  if (TM.getDataSections())
+    report_fatal_error("XCOFF unique data sections not yet implemented");
+
+  // We always use a qualname symbol for a GV that represents
+  // a declaration, a function descriptor, or a common symbol.
+  // It is inherently ambiguous when the GO represents the address of a
+  // function, as the GO could either represent a function descriptor or a
+  // function entry point. We choose to always return a function descriptor
+  // here.
+  if (const GlobalObject *GO = dyn_cast<GlobalObject>(GV)) {
+    if (GO->isDeclaration())
+      return cast<MCSectionXCOFF>(getSectionForExternalReference(GO, TM))
+          ->getQualNameSymbol();
+
+    SectionKind GOKind = getKindForGlobal(GO, TM);
+    if (GOKind.isText())
+      return cast<MCSectionXCOFF>(
+                 getSectionForFunctionDescriptor(cast<Function>(GO), TM))
+          ->getQualNameSymbol();
+    if (GOKind.isCommon() || GOKind.isBSSLocal())
+      return cast<MCSectionXCOFF>(SectionForGlobal(GO, GOKind, TM))
+          ->getQualNameSymbol();
+  }
+
+  // For all other cases, fall back to getSymbol to return the unqualified name.
+  // This could change for a GV that is a GlobalVariable when we decide to
+  // support -fdata-sections since we could avoid having label symbols if the
+  // linkage name is applied to the csect symbol.
+  return nullptr;
+}
+
 MCSection *TargetLoweringObjectFileXCOFF::getExplicitSectionGlobal(
     const GlobalObject *GO, SectionKind Kind, const TargetMachine &TM) const {
   report_fatal_error("XCOFF explicit sections not yet implemented.");
 }
 
 MCSection *TargetLoweringObjectFileXCOFF::getSectionForExternalReference(
     const GlobalObject *GO, const TargetMachine &TM) const {
   assert(GO->isDeclaration() &&

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

 namespace {
 
 class PPCAsmPrinter : public AsmPrinter {
 protected:
   MapVector<const MCSymbol *, MCSymbol *> TOC;
   const PPCSubtarget *Subtarget = nullptr;
   StackMaps SM;
 
-  virtual MCSymbol *getMCSymbolForTOCPseudoMO(const MachineOperand &MO);
-
 public:
   explicit PPCAsmPrinter(TargetMachine &TM,
                          std::unique_ptr<MCStreamer> Streamer)
       : AsmPrinter(TM, std::move(Streamer)), SM(*this) {}
 
   StringRef getPassName() const override { return "PowerPC Assembly Printer"; }
 
   MCSymbol *lookUpOrCreateTOCEntry(const MCSymbol *Sym);
public:
   void emitFunctionBodyStart() override;
   void emitFunctionBodyEnd() override;
   void emitInstruction(const MachineInstr *MI) override;
 };
 
 class PPCAIXAsmPrinter : public PPCAsmPrinter {
 private:
   static void ValidateGV(const GlobalVariable *GV);
-protected:
-  MCSymbol *getMCSymbolForTOCPseudoMO(const MachineOperand &MO) override;
 
 public:
   PPCAIXAsmPrinter(TargetMachine &TM, std::unique_ptr<MCStreamer> Streamer)
       : PPCAsmPrinter(TM, std::move(Streamer)) {}
 
   StringRef getPassName() const override { return "AIX PPC Assembly Printer"; }
 
   void SetupMachineFunction(MachineFunction &MF) override;
 
-  const MCExpr *lowerConstant(const Constant *CV) override;
-
   void emitGlobalVariable(const GlobalVariable *GV) override;
 
   void emitFunctionDescriptor() override;
 
   void emitEndOfAsmFile(Module &) override;
 };
 
 } // end anonymous namespace
EmitToStreamer(*OutStreamer,
                  MCInstBuilder(Subtarget->isPPC64() ?
                                PPC::BL8_NOP_TLS : PPC::BL_TLS)
                  .addExpr(TlsRef)
                  .addExpr(SymVar));
 }
 
 /// Map a machine operand for a TOC pseudo-machine instruction to its
 /// corresponding MCSymbol.
-MCSymbol *PPCAsmPrinter::getMCSymbolForTOCPseudoMO(const MachineOperand &MO) {
+static MCSymbol *getMCSymbolForTOCPseudoMO(const MachineOperand &MO,
+                                           AsmPrinter &AP) {
   switch (MO.getType()) {
   case MachineOperand::MO_GlobalAddress:
-    return getSymbol(MO.getGlobal());
+    return AP.getSymbol(MO.getGlobal());
   case MachineOperand::MO_ConstantPoolIndex:
-    return GetCPISymbol(MO.getIndex());
+    return AP.GetCPISymbol(MO.getIndex());
   case MachineOperand::MO_JumpTableIndex:
-    return GetJTISymbol(MO.getIndex());
+    return AP.GetJTISymbol(MO.getIndex());
   case MachineOperand::MO_BlockAddress:
-    return GetBlockAddressSymbol(MO.getBlockAddress());
+    return AP.GetBlockAddressSymbol(MO.getBlockAddress());
   default:
     llvm_unreachable("Unexpected operand type to get symbol.");
   }
 }
 
 /// EmitInstruction -- Print out a single PowerPC MI in Darwin syntax to
 /// the current output stream.
 ///
case PPC::LWZtoc: {
     // Change the opcode to LWZ.
     TmpInst.setOpcode(PPC::LWZ);
 
     const MachineOperand &MO = MI->getOperand(1);
     assert((MO.isGlobal() || MO.isCPI() || MO.isJTI() || MO.isBlockAddress()) &&
            "Invalid operand for LWZtoc.");
 
     // Map the operand to its corresponding MCSymbol.
-    const MCSymbol *const MOSymbol = getMCSymbolForTOCPseudoMO(MO);
+    const MCSymbol *const MOSymbol = getMCSymbolForTOCPseudoMO(MO, *this);
 
     // Create a reference to the GOT entry for the symbol. The GOT entry will be
     // synthesized later.
     if (PL == PICLevel::SmallPIC && !IsAIX) {
       const MCExpr *Exp =
         MCSymbolRefExpr::create(MOSymbol, MCSymbolRefExpr::VK_GOT,
                                 OutContext);
       TmpInst.getOperand(1) = MCOperand::createExpr(Exp);
case PPC::LDtoc: {
     const MachineOperand &MO = MI->getOperand(1);
     assert((MO.isGlobal() || MO.isCPI() || MO.isJTI() || MO.isBlockAddress()) &&
            "Invalid operand!");
 
     // Map the machine operand to its corresponding MCSymbol, then map the
     // global address operand to be a reference to the TOC entry we will
     // synthesize later.
     MCSymbol *TOCEntry =
-        lookUpOrCreateTOCEntry(getMCSymbolForTOCPseudoMO(MO));
+        lookUpOrCreateTOCEntry(getMCSymbolForTOCPseudoMO(MO, *this));
 
     const MCSymbolRefExpr::VariantKind VK =
         IsAIX ? MCSymbolRefExpr::VK_None : MCSymbolRefExpr::VK_PPC_TOC;
     const MCExpr *Exp =
         MCSymbolRefExpr::create(TOCEntry, VK, OutContext);
     TmpInst.getOperand(1) = MCOperand::createExpr(Exp);
     EmitToStreamer(*OutStreamer, TmpInst);
     return;
case PPC::ADDIStocHA: {
     // Change the opcode to ADDIS.
     TmpInst.setOpcode(PPC::ADDIS);
 
     const MachineOperand &MO = MI->getOperand(2);
     assert((MO.isGlobal() || MO.isCPI() || MO.isJTI() || MO.isBlockAddress()) &&
            "Invalid operand for ADDIStocHA.");
 
     // Map the machine operand to its corresponding MCSymbol.
-    MCSymbol *MOSymbol = getMCSymbolForTOCPseudoMO(MO);
+    MCSymbol *MOSymbol = getMCSymbolForTOCPseudoMO(MO, *this);
 
     // Always use TOC on AIX. Map the global address operand to be a reference
     // to the TOC entry we will synthesize later. 'TOCEntry' is a label used to
     // reference the storage allocated in the TOC which contains the address of
     // 'MOSymbol'.
     MCSymbol *TOCEntry = lookUpOrCreateTOCEntry(MOSymbol);
     const MCExpr *Exp = MCSymbolRefExpr::create(TOCEntry,
                                                 MCSymbolRefExpr::VK_PPC_U,
case PPC::LWZtocL: {
     // Change the opcode to lwz.
     TmpInst.setOpcode(PPC::LWZ);
 
     const MachineOperand &MO = MI->getOperand(1);
     assert((MO.isGlobal() || MO.isCPI() || MO.isJTI() || MO.isBlockAddress()) &&
            "Invalid operand for LWZtocL.");
 
     // Map the machine operand to its corresponding MCSymbol.
-    MCSymbol *MOSymbol = getMCSymbolForTOCPseudoMO(MO);
+    MCSymbol *MOSymbol = getMCSymbolForTOCPseudoMO(MO, *this);
 
     // Always use TOC on AIX. Map the global address operand to be a reference
     // to the TOC entry we will synthesize later. 'TOCEntry' is a label used to
     // reference the storage allocated in the TOC which contains the address of
     // 'MOSymbol'.
     MCSymbol *TOCEntry = lookUpOrCreateTOCEntry(MOSymbol);
     const MCExpr *Exp = MCSymbolRefExpr::create(TOCEntry,
                                                 MCSymbolRefExpr::VK_PPC_L,
case PPC::ADDIStocHA8: {
     // constant pool index with large code model enabled, then generate a TOC
     // entry and reference that. Otherwise, reference the symbol directly.
     TmpInst.setOpcode(PPC::ADDIS8);
 
     const MachineOperand &MO = MI->getOperand(2);
     assert((MO.isGlobal() || MO.isCPI() || MO.isJTI() || MO.isBlockAddress()) &&
            "Invalid operand for ADDIStocHA8!");
 
-    const MCSymbol *MOSymbol = getMCSymbolForTOCPseudoMO(MO);
+    const MCSymbol *MOSymbol = getMCSymbolForTOCPseudoMO(MO, *this);
 
     const bool GlobalToc =
         MO.isGlobal() && Subtarget->isGVIndirectSymbol(MO.getGlobal());
     if (GlobalToc || MO.isJTI() || MO.isBlockAddress() ||
         (MO.isCPI() && TM.getCodeModel() == CodeModel::Large))
       MOSymbol = lookUpOrCreateTOCEntry(MOSymbol);
 
     const MCSymbolRefExpr::VariantKind VK =
assert((MO.isGlobal() || MO.isCPI() || MO.isJTI() ||
             MO.isBlockAddress()) &&
            "Invalid operand for LDtocL!");
 
     LLVM_DEBUG(assert(
         (!MO.isGlobal() || Subtarget->isGVIndirectSymbol(MO.getGlobal())) &&
         "LDtocL used on symbol that could be accessed directly is "
         "invalid. Must match ADDIStocHA8."));
 
-    const MCSymbol *MOSymbol = getMCSymbolForTOCPseudoMO(MO);
+    const MCSymbol *MOSymbol = getMCSymbolForTOCPseudoMO(MO, *this);
 
     if (!MO.isCPI() || TM.getCodeModel() == CodeModel::Large)
       MOSymbol = lookUpOrCreateTOCEntry(MOSymbol);
 
     const MCSymbolRefExpr::VariantKind VK =
         IsAIX ? MCSymbolRefExpr::VK_PPC_L : MCSymbolRefExpr::VK_PPC_TOC_LO;
     const MCExpr *Exp =
         MCSymbolRefExpr::create(MOSymbol, VK, OutContext);
case PPC::ADDItocL: {
     const MachineOperand &MO = MI->getOperand(2);
     assert((MO.isGlobal() || MO.isCPI()) && "Invalid operand for ADDItocL.");
 
     LLVM_DEBUG(assert(
         !(MO.isGlobal() && Subtarget->isGVIndirectSymbol(MO.getGlobal())) &&
         "Interposable definitions must use indirect access."));
 
     const MCExpr *Exp =
-        MCSymbolRefExpr::create(getMCSymbolForTOCPseudoMO(MO),
+        MCSymbolRefExpr::create(getMCSymbolForTOCPseudoMO(MO, *this),
                                 MCSymbolRefExpr::VK_PPC_TOC_LO, OutContext);
     TmpInst.getOperand(2) = MCOperand::createExpr(Exp);
     EmitToStreamer(*OutStreamer, TmpInst);
     return;
   }
   case PPC::ADDISgotTprelHA: {
     // Transform: %xd = ADDISgotTprelHA %x2, @sym
     // Into:      %xd = ADDIS8 %x2, sym@got@tlsgd@ha
void PPCAIXAsmPrinter::ValidateGV(const GlobalVariable *GV) {
 
   if (GV->hasSection())
     report_fatal_error("Custom section for Data not yet supported.");
 
   if (GV->hasComdat())
     report_fatal_error("COMDAT not yet supported by AIX.");
 }
 
-const MCExpr *PPCAIXAsmPrinter::lowerConstant(const Constant *CV) {
-  if (const Function *F = dyn_cast<Function>(CV)) {
-    MCSectionXCOFF *Csect = cast<MCSectionXCOFF>(
-        F->isDeclaration()
-            ? getObjFileLowering().getSectionForExternalReference(F, TM)
-            : getObjFileLowering().getSectionForFunctionDescriptor(F, TM));
-
-    return MCSymbolRefExpr::create(Csect->getQualNameSymbol(), OutContext);
-  }
-  return PPCAsmPrinter::lowerConstant(CV);
-}
-
 static bool isSpecialLLVMGlobalArrayForStaticInit(const GlobalVariable *GV) {
   return StringSwitch<bool>(GV->getName())
       .Cases("llvm.global_ctors", "llvm.global_dtors", true)
       .Default(false);
 }
 
 void PPCAIXAsmPrinter::emitGlobalVariable(const GlobalVariable *GV) {
   ValidateGV(GV);
 
   // TODO: Update the handling of global arrays for static init when we support
   // the ".ref" directive.
   // Otherwise, we can skip these arrays, because the AIX linker collects
   // static init functions simply based on their name.
   if (isSpecialLLVMGlobalArrayForStaticInit(GV))
     return;
 
   // Create the symbol, set its storage class.
   MCSymbolXCOFF *GVSym = cast<MCSymbolXCOFF>(getSymbol(GV));
   GVSym->setStorageClass(
       TargetLoweringObjectFileXCOFF::getStorageClassForGlobal(GV));
 
-  SectionKind GVKind;
-
-  // Create the containing csect and set it. We set it for externals as well,
-  // since this may not have been set elsewhere depending on how they are used.
-  MCSectionXCOFF *Csect = cast<MCSectionXCOFF>(
-      GV->isDeclaration()
-          ? getObjFileLowering().getSectionForExternalReference(GV, TM)
-          : getObjFileLowering().SectionForGlobal(
-                GV, GVKind = getObjFileLowering().getKindForGlobal(GV, TM),
-                TM));
-
   // External global variables are already handled.
   if (GV->isDeclaration())
     return;
 
+  SectionKind GVKind = getObjFileLowering().getKindForGlobal(GV, TM);
   if (!GVKind.isGlobalWriteableData() && !GVKind.isReadOnly())
     report_fatal_error("Encountered a global variable kind that is "
                        "not supported yet.");
 
+  MCSectionXCOFF *Csect = cast<MCSectionXCOFF>(
+      getObjFileLowering().SectionForGlobal(GV, GVKind, TM));
+
   // Switch to the containing csect.
   OutStreamer->SwitchSection(Csect);
 
   const DataLayout &DL = GV->getParent()->getDataLayout();
 
   // Handle common symbols.
   if (GVKind.isCommon() || GVKind.isBSSLocal()) {
     unsigned Align =
       GV->getAlignment() ? GV->getAlignment() : DL.getPreferredAlignment(GV);
     uint64_t Size = DL.getTypeAllocSize(GV->getType()->getElementType());
 
     if (GVKind.isBSSLocal())
       OutStreamer->emitXCOFFLocalCommonSymbol(
-          GVSym, Size, Csect->getQualNameSymbol(), Align);
+          OutContext.getOrCreateSymbol(GVSym->getUnqualifiedName()), Size,
+          GVSym, Align);
     else
-      OutStreamer->emitCommonSymbol(Csect->getQualNameSymbol(), Size, Align);
+      OutStreamer->emitCommonSymbol(GVSym, Size, Align);
     return;
   }
 
   MCSymbol *EmittedInitSym = GVSym;
   emitLinkage(GV, EmittedInitSym);
   emitAlignment(getGVAlignment(GV, DL), GV);
   OutStreamer->emitLabel(EmittedInitSym);
   emitGlobalConstant(GV->getParent()->getDataLayout(), GV->getInitializer());
MCSectionXCOFF *TCEntry = cast<MCSectionXCOFF>(
         getObjFileLowering().getSectionForTOCEntry(I.first));
     OutStreamer->SwitchSection(TCEntry);
 
     OutStreamer->emitLabel(I.second);
     TS.emitTCEntry(*I.first);
   }
 }
 
-MCSymbol *
-PPCAIXAsmPrinter::getMCSymbolForTOCPseudoMO(const MachineOperand &MO) {
-  const GlobalObject *GO = nullptr;
-
-  // If the MO is a function or certain kind of globals, we want to make sure to
-  // refer to the csect symbol, otherwise we can just do the default handling.
-  if (MO.getType() != MachineOperand::MO_GlobalAddress ||
-      !(GO = dyn_cast<const GlobalObject>(MO.getGlobal())))
-    return PPCAsmPrinter::getMCSymbolForTOCPseudoMO(MO);
-
-  // Do an early error check for globals we don't support. This will go away
-  // eventually.
-  const auto *GV = dyn_cast<const GlobalVariable>(GO);
-  if (GV) {
-    ValidateGV(GV);
-  }
-
-  // If the global object is a global variable without initializer or is a
-  // declaration of a function, then XSym is an external referenced symbol.
-  // Hence we may need to explictly create a MCSectionXCOFF for it so that we
-  // can return its symbol later.
-  if (GO->isDeclaration()) {
-    return cast<MCSectionXCOFF>(
-               getObjFileLowering().getSectionForExternalReference(GO, TM))
-        ->getQualNameSymbol();
-  }
-
-  // Handle initialized global variables and defined functions.
-  SectionKind GOKind = getObjFileLowering().getKindForGlobal(GO, TM);
-
-  if (GOKind.isText()) {
-    // If the MO is a function, we want to make sure to refer to the function
-    // descriptor csect.
-    return cast<MCSectionXCOFF>(
-               getObjFileLowering().getSectionForFunctionDescriptor(
-                   cast<const Function>(GO), TM))
-        ->getQualNameSymbol();
-  } else if (GOKind.isCommon() || GOKind.isBSSLocal()) {
-    // If the operand is a common then we should refer to the csect symbol.
-    return cast<MCSectionXCOFF>(
-               getObjFileLowering().SectionForGlobal(GO, GOKind, TM))
-        ->getQualNameSymbol();
-  }
-
-  // Other global variables are refered to by labels inside of a single csect,
-  // so refer to the label directly.
-  return getSymbol(GV);
-}
-
 /// createPPCAsmPrinterPass - Returns a pass that prints the PPC assembly code
 /// for a MachineFunction to the given output stream, in a format that the
 /// Darwin assembler can deal with.
 ///
 static AsmPrinter *
 createPPCAsmPrinterPass(TargetMachine &tm,
                         std::unique_ptr<MCStreamer> &&Streamer) {
   if (tm.getTargetTriple().isOSAIX())

llvm/lib/Target/TargetMachine.cpp

if (MayAlwaysUsePrivate || !GV->hasPrivateLinkage()) {
     return;
   }
   const TargetLoweringObjectFile *TLOF = getObjFileLowering();
   TLOF->getNameWithPrefix(Name, GV, *this);
 }
 
 MCSymbol *TargetMachine::getSymbol(const GlobalValue *GV) const {
   const TargetLoweringObjectFile *TLOF = getObjFileLowering();
+  // XCOFF symbols could have special naming convention.
+  if (MCSymbol *TargetSymbol = TLOF->getTargetSymbol(GV, *this))
+    return TargetSymbol;
+
   SmallString<128> NameStr;
   getNameWithPrefix(NameStr, GV, TLOF->getMangler());
   return TLOF->getContext().getOrCreateSymbol(NameStr);
 }
 
 TargetIRAnalysis TargetMachine::getTargetIRAnalysis() {
   // Since Analysis can't depend on Target, use a std::function to invert the
   // dependency.
   return TargetIRAnalysis(
       [this](const Function &F) { return this->getTargetTransformInfo(F); });
 }

llvm/test/CodeGen/PowerPC/aix-xcoff-lower-comm.ll

+; RUN: llc -verify-machineinstrs -mcpu=pwr4 -mtriple powerpc-ibm-aix-xcoff < %s | FileCheck --check-prefixes=CHECK,ASM32 %s
+; RUN: llc -verify-machineinstrs -mcpu=pwr4 -mtriple powerpc64-ibm-aix-xcoff < %s | FileCheck --check-prefixes=CHECK,ASM64 %s
+
+; RUN: llc -verify-machineinstrs -mcpu=pwr4 -mtriple powerpc-ibm-aix-xcoff -filetype=obj -o %t.o < %s
+; RUN: llvm-readobj -r --expand-relocs -t %t.o | FileCheck --check-prefixes=RELOC,SYM %s
+
+; RUN: not --crash llc -verify-machineinstrs -mcpu=pwr4 -mtriple powerpc64-ibm-aix-xcoff -filetype=obj < %s 2>&1 | \
+; RUN: FileCheck --check-prefix=XCOFF64 %s
+; XCOFF64: LLVM ERROR: 64-bit XCOFF object files are not supported yet.
+
+@common = common global i32 0, align 4
+@pointer = global i32* @common, align 4
+
+
+; CHECK:             .comm   common[RW],4,2
+; CHECK-NEXT:        .csect .data[RW]
+; CHECK-NEXT:        .globl  pointer
+; ASM32-NEXT:        .align  2
+; ASM64-NEXT:        .align  3
+; CHECK-NEXT:pointer:
+; ASM32-NEXT:        .long   common[RW]
+; ASM64-NEXT:        .llong   common[RW]
+
+
+; RELOC:      Relocations [
+; RELOC-NEXT:   Section (index: {{[0-9]+}}) .data {
+; RELOC-NEXT:   Relocation {
+; RELOC-NEXT:     Virtual Address: 0x0
+; RELOC-NEXT:     Symbol: common ([[#COM_INDX:]])
+; RELOC-NEXT:     IsSigned: No
+; RELOC-NEXT:     FixupBitValue: 0
+; RELOC-NEXT:     Length: 32
+; RELOC-NEXT:     Type: R_POS (0x0)
+; RELOC-NEXT:   }
+; RELOC-NEXT: }
+; RELOC-NEXT: ]
+
+; SYM:        Symbol {{[{][[:space:]] *}}Index: [[#INDX:]]{{[[:space:]] *}}Name: .data
+; SYM-NEXT:     Value (RelocatableAddress): 0x0
+; SYM-NEXT:     Section: .data
+; SYM-NEXT:     Type: 0x0
+; SYM-NEXT:     StorageClass: C_HIDEXT (0x6B)
+; SYM-NEXT:     NumberOfAuxEntries: 1
+; SYM-NEXT:     CSECT Auxiliary Entry {
+; SYM-NEXT:       Index: [[#INDX+1]]
+; SYM-NEXT:       SectionLen: 4
+; SYM-NEXT:       ParameterHashIndex: 0x0
+; SYM-NEXT:       TypeChkSectNum: 0x0
+; SYM-NEXT:       SymbolAlignmentLog2: 2
+; SYM-NEXT:       SymbolType: XTY_SD (0x1)
+; SYM-NEXT:       StorageMappingClass: XMC_RW (0x5)
+; SYM-NEXT:       StabInfoIndex: 0x0
+; SYM-NEXT:       StabSectNum: 0x0
+; SYM-NEXT:     }
+; SYM-NEXT:   }
+; SYM-NEXT:   Symbol {
+; SYM-NEXT:     Index: [[#INDX+2]]
+; SYM-NEXT:     Name: pointer
+; SYM-NEXT:     Value (RelocatableAddress): 0x0
+; SYM-NEXT:     Section: .data
+; SYM-NEXT:     Type: 0x0
+; SYM-NEXT:     StorageClass: C_EXT (0x2)
+; SYM-NEXT:     NumberOfAuxEntries: 1
+; SYM-NEXT:     CSECT Auxiliary Entry {
+; SYM-NEXT:       Index: [[#INDX+3]]
+; SYM-NEXT:       ContainingCsectSymbolIndex: [[#INDX]]
+; SYM-NEXT:       ParameterHashIndex: 0x0
+; SYM-NEXT:       TypeChkSectNum: 0x0
+; SYM-NEXT:       SymbolAlignmentLog2: 0
+; SYM-NEXT:       SymbolType: XTY_LD (0x2)
+; SYM-NEXT:       StorageMappingClass: XMC_RW (0x5)
+; SYM-NEXT:       StabInfoIndex: 0x0
+; SYM-NEXT:       StabSectNum: 0x0
+; SYM-NEXT:     }
+; SYM-NEXT:   }
+; SYM-NEXT:   Symbol {
+; SYM-NEXT:     Index: [[#COM_INDX]]
+; SYM-NEXT:     Name: common
+; SYM-NEXT:     Value (RelocatableAddress): 0x4
+; SYM-NEXT:     Section: .bss
+; SYM-NEXT:     Type: 0x0
+; SYM-NEXT:     StorageClass: C_EXT (0x2)
+; SYM-NEXT:     NumberOfAuxEntries: 1
+; SYM-NEXT:     CSECT Auxiliary Entry {
+; SYM-NEXT:       Index: [[#COM_INDX+1]]
+; SYM-NEXT:       SectionLen: 4
+; SYM-NEXT:       ParameterHashIndex: 0x0
+; SYM-NEXT:       TypeChkSectNum: 0x0
+; SYM-NEXT:       SymbolAlignmentLog2: 2
+; SYM-NEXT:       SymbolType: XTY_CM (0x3)
+; SYM-NEXT:       StorageMappingClass: XMC_RW (0x5)
+; SYM-NEXT:       StabInfoIndex: 0x0
+; SYM-NEXT:       StabSectNum: 0x0
+; SYM-NEXT:     }
+; SYM-NEXT:   }