[XCOFF][AIX] Differentiate usage of label symbol and csect symbol

Summary:
 We are using symbols to represent label and csect interchangeably before, and that could be a problem.
There are cases we would need to add storage mapping class to the symbol if that symbol is actually the name of a csect, but it's hard for us to figure out whether that symbol is a label or csect.

This patch intend to do the following:
    1. Construct a QualName (A name include the storage mapping class)
       MCSymbolXCOFF for every MCSectionXCOFF.
    2. Keep a pointer to that QualName inside of MCSectionXCOFF.
    3. Use that QualName whenever we need a symbol refers to that
       MCSectionXCOFF.
    4. Adapt the snowball effect from the above changes in
       XCOFFObjectWriter.cpp.

Reviewers: xingxue, DiggerLin, sfertile, daltenty, hubert.reinterpretcast

Reviewed By: DiggerLin, daltenty

Subscribers: wuzish, nemanjai, mgorny, hiraditya, kbarton, jsji, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D69633

8 files changed, 47 insertions, 67 deletions

diff --git a/llvm/lib/MC/MCAsmInfo.cpp b/llvm/lib/MC/MCAsmInfo.cpp
index 71e51e320f8..420dbaa80ae 100644
--- a/llvm/lib/MC/MCAsmInfo.cpp
+++ b/llvm/lib/MC/MCAsmInfo.cpp
@@ -100,7 +100,7 @@ MCAsmInfo::getExprForFDESymbol(const MCSymbol *Sym,
   return MCBinaryExpr::createSub(Res, PC, Context);
 }
 
-static bool isAcceptableChar(char C) {
+bool MCAsmInfo::isAcceptableChar(char C) const {
   return (C >= 'a' && C <= 'z') || (C >= 'A' && C <= 'Z') ||
          (C >= '0' && C <= '9') || C == '_' || C == '$' || C == '.' || C == '@';
 }
diff --git a/llvm/lib/MC/MCAsmInfoXCOFF.cpp b/llvm/lib/MC/MCAsmInfoXCOFF.cpp
index 65fe8848e20..c51cdff59fa 100644
--- a/llvm/lib/MC/MCAsmInfoXCOFF.cpp
+++ b/llvm/lib/MC/MCAsmInfoXCOFF.cpp
@@ -26,10 +26,11 @@ MCAsmInfoXCOFF::MCAsmInfoXCOFF() {
   SupportsQuotedNames = false;
 }
 
-bool MCAsmInfoXCOFF::isValidUnquotedName(StringRef Name) const {
-  // FIXME: Remove this function when we stop using "TOC[TC0]" as a symbol name.
-  if (Name.equals("TOC[TC0]"))
+bool MCAsmInfoXCOFF::isAcceptableChar(char C) const {
+  // QualName is allowed for a MCSymbolXCOFF, and
+  // QualName contains '[' and ']'.
+  if (C == '[' || C == ']')
     return true;
 
-  return MCAsmInfo::isValidUnquotedName(Name);
+  return MCAsmInfo::isAcceptableChar(C);
 }
diff --git a/llvm/lib/MC/MCAsmStreamer.cpp b/llvm/lib/MC/MCAsmStreamer.cpp
index 2d9c2cb2125..3fe401de35d 100644
--- a/llvm/lib/MC/MCAsmStreamer.cpp
+++ b/llvm/lib/MC/MCAsmStreamer.cpp
@@ -164,7 +164,8 @@ public:
   void EmitCOFFSectionIndex(MCSymbol const *Symbol) override;
   void EmitCOFFSecRel32(MCSymbol const *Symbol, uint64_t Offset) override;
   void EmitCOFFImgRel32(MCSymbol const *Symbol, int64_t Offset) override;
-  void EmitXCOFFLocalCommonSymbol(MCSymbol *Symbol, uint64_t Size,
+  void EmitXCOFFLocalCommonSymbol(MCSymbol *LabelSym, uint64_t Size,
+                                  MCSymbol *CsectSym,
                                   unsigned ByteAlign) override;
   void emitELFSize(MCSymbol *Symbol, const MCExpr *Value) override;
   void EmitCommonSymbol(MCSymbol *Symbol, uint64_t Size,
@@ -765,16 +766,18 @@ void MCAsmStreamer::EmitCOFFImgRel32(MCSymbol const *Symbol, int64_t Offset) {
 // We need an XCOFF-specific version of this directive as the AIX syntax
 // requires a QualName argument identifying the csect name and storage mapping
 // class to appear before the alignment if we are specifying it.
-void MCAsmStreamer::EmitXCOFFLocalCommonSymbol(MCSymbol *Symbol, uint64_t Size,
+void MCAsmStreamer::EmitXCOFFLocalCommonSymbol(MCSymbol *LabelSym,
+                                               uint64_t Size,
+                                               MCSymbol *CsectSym,
                                                unsigned ByteAlignment) {
   assert(MAI->getLCOMMDirectiveAlignmentType() == LCOMM::Log2Alignment &&
          "We only support writing log base-2 alignment format with XCOFF.");
   assert(isPowerOf2_32(ByteAlignment) && "Alignment must be a power of 2.");
 
   OS << "\t.lcomm\t";
-  Symbol->print(OS, MAI);
-  OS << ',' << Size;
-  OS << ',' << Symbol->getName();
+  LabelSym->print(OS, MAI);
+  OS << ',' << Size << ',';
+  CsectSym->print(OS, MAI);
   OS << ',' << Log2_32(ByteAlignment);
 
   EmitEOL();
diff --git a/llvm/lib/MC/MCContext.cpp b/llvm/lib/MC/MCContext.cpp
index a69ee19e1a1..a6417113fd3 100644
--- a/llvm/lib/MC/MCContext.cpp
+++ b/llvm/lib/MC/MCContext.cpp
@@ -15,6 +15,7 @@
 #include "llvm/ADT/Twine.h"
 #include "llvm/BinaryFormat/COFF.h"
 #include "llvm/BinaryFormat/ELF.h"
+#include "llvm/BinaryFormat/XCOFF.h"
 #include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/MCCodeView.h"
 #include "llvm/MC/MCDwarf.h"
@@ -550,13 +551,15 @@ MCSectionXCOFF *MCContext::getXCOFFSection(StringRef Section,
 
   // Otherwise, return a new section.
   StringRef CachedName = Entry.first.SectionName;
+  MCSymbol *QualName = getOrCreateSymbol(
+      CachedName + "[" + XCOFF::getMappingClassString(SMC) + "]");
 
   MCSymbol *Begin = nullptr;
   if (BeginSymName)
     Begin = createTempSymbol(BeginSymName, false);
 
-  MCSectionXCOFF *Result = new (XCOFFAllocator.Allocate())
-      MCSectionXCOFF(CachedName, SMC, Type, SC, Kind, Begin);
+  MCSectionXCOFF *Result = new (XCOFFAllocator.Allocate()) MCSectionXCOFF(
+      CachedName, SMC, Type, SC, Kind, cast<MCSymbolXCOFF>(QualName), Begin);
   Entry.second = Result;
 
   auto *F = new MCDataFragment();
diff --git a/llvm/lib/MC/MCSectionXCOFF.cpp b/llvm/lib/MC/MCSectionXCOFF.cpp
index d52959f15f9..d00e9322897 100644
--- a/llvm/lib/MC/MCSectionXCOFF.cpp
+++ b/llvm/lib/MC/MCSectionXCOFF.cpp
@@ -15,18 +15,6 @@ using namespace llvm;
 
 MCSectionXCOFF::~MCSectionXCOFF() = default;
 
-static StringRef getMappingClassString(XCOFF::StorageMappingClass SMC) {
-  switch (SMC) {
-  case XCOFF::XMC_DS:
-    return "DS";
-  case XCOFF::XMC_RW:
-    return "RW";
-  case XCOFF::XMC_PR:
-    return "PR";
-  default:
-    report_fatal_error("Unhandled storage-mapping class.");
-  }
-}
 
 void MCSectionXCOFF::PrintSwitchToSection(const MCAsmInfo &MAI, const Triple &T,
                                           raw_ostream &OS,
@@ -35,9 +23,7 @@ void MCSectionXCOFF::PrintSwitchToSection(const MCAsmInfo &MAI, const Triple &T,
     if (getMappingClass() != XCOFF::XMC_PR)
       report_fatal_error("Unhandled storage-mapping class for .text csect");
 
-    OS << "\t.csect " << getSectionName() << "["
-       << getMappingClassString(getMappingClass())
-       << "]" << '\n';
+    OS << "\t.csect " << QualName->getName() << '\n';
     return;
   }
 
@@ -45,8 +31,7 @@ void MCSectionXCOFF::PrintSwitchToSection(const MCAsmInfo &MAI, const Triple &T,
     switch (getMappingClass()) {
     case XCOFF::XMC_RW:
     case XCOFF::XMC_DS:
-      OS << "\t.csect " << getSectionName() << "["
-         << getMappingClassString(getMappingClass()) << "]" << '\n';
+      OS << "\t.csect " << QualName->getName() << '\n';
       break;
     case XCOFF::XMC_TC0:
       OS << "\t.toc\n";
diff --git a/llvm/lib/MC/MCStreamer.cpp b/llvm/lib/MC/MCStreamer.cpp
index b8278cb1107..c48ee76b3fc 100644
--- a/llvm/lib/MC/MCStreamer.cpp
+++ b/llvm/lib/MC/MCStreamer.cpp
@@ -1063,7 +1063,8 @@ void MCStreamer::EmitCOFFSymbolStorageClass(int StorageClass) {
 void MCStreamer::EmitCOFFSymbolType(int Type) {
   llvm_unreachable("this directive only supported on COFF targets");
 }
-void MCStreamer::EmitXCOFFLocalCommonSymbol(MCSymbol *Symbol, uint64_t Size,
+void MCStreamer::EmitXCOFFLocalCommonSymbol(MCSymbol *LabelSym, uint64_t Size,
+                                            MCSymbol *CsectSym,
                                             unsigned ByteAlign) {
   llvm_unreachable("this directive only supported on XCOFF targets");
 }
diff --git a/llvm/lib/MC/MCXCOFFStreamer.cpp b/llvm/lib/MC/MCXCOFFStreamer.cpp
index 50937d6adc0..c40a067e93e 100644
--- a/llvm/lib/MC/MCXCOFFStreamer.cpp
+++ b/llvm/lib/MC/MCXCOFFStreamer.cpp
@@ -94,8 +94,9 @@ MCStreamer *llvm::createXCOFFStreamer(MCContext &Context,
   return S;
 }
 
-void MCXCOFFStreamer::EmitXCOFFLocalCommonSymbol(MCSymbol *Symbol,
+void MCXCOFFStreamer::EmitXCOFFLocalCommonSymbol(MCSymbol *LabelSym,
                                                  uint64_t Size,
+                                                 MCSymbol *CsectSym,
                                                  unsigned ByteAlignment) {
-  EmitCommonSymbol(Symbol, Size, ByteAlignment);
+  EmitCommonSymbol(CsectSym, Size, ByteAlignment);
 }
diff --git a/llvm/lib/MC/XCOFFObjectWriter.cpp b/llvm/lib/MC/XCOFFObjectWriter.cpp
index d8f9e0b593b..cec74ee76c3 100644
--- a/llvm/lib/MC/XCOFFObjectWriter.cpp
+++ b/llvm/lib/MC/XCOFFObjectWriter.cpp
@@ -78,15 +78,7 @@ struct ControlSection {
 // (approximately) the same storage mapping class. For example all the csects
 // with a storage mapping class of `xmc_pr` will get placed into the same
 // container.
-struct CsectGroup {
-  enum LabelDefinitionSupport : bool {
-    LabelDefSupported = true,
-    LabelDefUnsupported = false
-  };
-
-  const LabelDefinitionSupport SupportLabelDef;
-  std::deque<ControlSection> Csects;
-};
+using CsectGroup = std::deque<ControlSection>;
 
 using CsectGroups = std::deque<CsectGroup *>;
 
@@ -132,7 +124,7 @@ struct Section {
     Index = UninitializedIndex;
     // Clear any csects we have stored.
     for (auto *Group : Groups)
-      Group->Csects.clear();
+      Group->clear();
   }
 
   Section(const char *N, XCOFF::SectionTypeFlags Flags, bool IsVirtual,
@@ -157,9 +149,9 @@ class XCOFFObjectWriter : public MCObjectWriter {
   // CsectGroups. These store the csects which make up different parts of
   // the sections. Should have one for each set of csects that get mapped into
   // the same section and get handled in a 'similar' way.
-  CsectGroup ProgramCodeCsects{CsectGroup::LabelDefSupported, {}};
-  CsectGroup DataCsects{CsectGroup::LabelDefSupported, {}};
-  CsectGroup BSSCsects{CsectGroup::LabelDefUnsupported, {}};
+  CsectGroup ProgramCodeCsects;
+  CsectGroup DataCsects;
+  CsectGroup BSSCsects;
 
   // The Predefined sections.
   Section Text;
@@ -294,8 +286,8 @@ void XCOFFObjectWriter::executePostLayoutBinding(MCAssembler &Asm,
       continue;
 
     CsectGroup &Group = getCsectGroup(MCSec);
-    Group.Csects.emplace_back(MCSec);
-    WrapperMap[MCSec] = &Group.Csects.back();
+    Group.emplace_back(MCSec);
+    WrapperMap[MCSec] = &Group.back();
   }
 
   for (const MCSymbol &S : Asm.symbols()) {
@@ -309,6 +301,11 @@ void XCOFFObjectWriter::executePostLayoutBinding(MCAssembler &Asm,
     assert(WrapperMap.find(ContainingCsect) != WrapperMap.end() &&
            "Expected containing csect to exist in map");
 
+    // If the symbol is the Csect itself, we don't need to put the symbol
+    // into Csect's Syms.
+    if (XSym == ContainingCsect->getQualNameSymbol())
+      continue;
+
     // Lookup the containing csect and add the symbol to it.
     WrapperMap[ContainingCsect]->Syms.emplace_back(XSym);
 
@@ -339,7 +336,7 @@ void XCOFFObjectWriter::writeSections(const MCAssembler &Asm,
     assert(CurrentAddressLocation == Section->Address &&
            "We should have no padding between sections.");
     for (const auto *Group : Section->Groups) {
-      for (const auto &Csect : Group->Csects) {
+      for (const auto &Csect : *Group) {
         if (uint32_t PaddingSize = Csect.Address - CurrentAddressLocation)
           W.OS.write_zeros(PaddingSize);
         Asm.writeSectionData(W.OS, Csect.MCCsect, Layout);
@@ -527,20 +524,14 @@ void XCOFFObjectWriter::writeSymbolTable(const MCAsmLayout &Layout) {
       continue;
 
     for (const auto *Group : Section->Groups) {
-      if (Group->Csects.empty())
+      if (Group->empty())
         continue;
 
-      const bool SupportLabelDef = Group->SupportLabelDef;
       const int16_t SectionIndex = Section->Index;
-      for (const auto &Csect : Group->Csects) {
+      for (const auto &Csect : *Group) {
         // Write out the control section first and then each symbol in it.
         writeSymbolTableEntryForControlSection(
             Csect, SectionIndex, Csect.MCCsect->getStorageClass());
-        if (!SupportLabelDef) {
-          assert(Csect.Syms.size() == 1 && "Csect should only contain 1 symbol "
-                                           "which is its label definition.");
-          continue;
-        }
 
         for (const auto Sym : Csect.Syms)
           writeSymbolTableEntryForCsectMemberLabel(
@@ -563,9 +554,8 @@ void XCOFFObjectWriter::assignAddressesAndIndices(const MCAsmLayout &Layout) {
 
   for (auto *Section : Sections) {
     const bool IsEmpty =
-        llvm::all_of(Section->Groups, [](const CsectGroup *Group) {
-          return Group->Csects.empty();
-        });
+        llvm::all_of(Section->Groups,
+                     [](const CsectGroup *Group) { return Group->empty(); });
     if (IsEmpty)
       continue;
 
@@ -576,11 +566,10 @@ void XCOFFObjectWriter::assignAddressesAndIndices(const MCAsmLayout &Layout) {
 
     bool SectionAddressSet = false;
     for (auto *Group : Section->Groups) {
-      if (Group->Csects.empty())
+      if (Group->empty())
         continue;
 
-      const bool SupportLabelDef = Group->SupportLabelDef;
-      for (auto &Csect : Group->Csects) {
+      for (auto &Csect : *Group) {
         const MCSectionXCOFF *MCSec = Csect.MCCsect;
         Csect.Address = alignTo(Address, MCSec->getAlignment());
         Csect.Size = Layout.getSectionAddressSize(MCSec);
@@ -588,10 +577,7 @@ void XCOFFObjectWriter::assignAddressesAndIndices(const MCAsmLayout &Layout) {
         Csect.SymbolTableIndex = SymbolTableIndex;
         // 1 main and 1 auxiliary symbol table entry for the csect.
         SymbolTableIndex += 2;
-
-        if (!SupportLabelDef)
-          continue;
-
+        
         for (auto &Sym : Csect.Syms) {
           Sym.SymbolTableIndex = SymbolTableIndex;
           // 1 main and 1 auxiliary symbol table entry for each contained
@@ -601,7 +587,7 @@ void XCOFFObjectWriter::assignAddressesAndIndices(const MCAsmLayout &Layout) {
       }
 
       if (!SectionAddressSet) {
-        Section->Address = Group->Csects.front().Address;
+        Section->Address = Group->front().Address;
         SectionAddressSet = true;
       }
     }