[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
diff --git a/llvm/lib/MC/MCAsmInfo.cpp b/llvm/lib/MC/MCAsmInfo.cpp
index 71e51e3..420dbaa 100644
--- a/llvm/lib/MC/MCAsmInfo.cpp
+++ b/llvm/lib/MC/MCAsmInfo.cpp
@@ -100,7 +100,7 @@
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 65fe884..c51cdff 100644
--- a/llvm/lib/MC/MCAsmInfoXCOFF.cpp
+++ b/llvm/lib/MC/MCAsmInfoXCOFF.cpp
@@ -26,10 +26,11 @@
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 2d9c2cb..3fe401d 100644
--- a/llvm/lib/MC/MCAsmStreamer.cpp
+++ b/llvm/lib/MC/MCAsmStreamer.cpp
@@ -164,7 +164,8 @@
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 @@
// 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 a69ee19..a641711 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 @@
// 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 d52959f..d00e932 100644
--- a/llvm/lib/MC/MCSectionXCOFF.cpp
+++ b/llvm/lib/MC/MCSectionXCOFF.cpp
@@ -15,18 +15,6 @@
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 @@
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 @@
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 b8278cb..c48ee76 100644
--- a/llvm/lib/MC/MCStreamer.cpp
+++ b/llvm/lib/MC/MCStreamer.cpp
@@ -1063,7 +1063,8 @@
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 50937d6..c40a067 100644
--- a/llvm/lib/MC/MCXCOFFStreamer.cpp
+++ b/llvm/lib/MC/MCXCOFFStreamer.cpp
@@ -94,8 +94,9 @@
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 d8f9e0b..cec74ee7 100644
--- a/llvm/lib/MC/XCOFFObjectWriter.cpp
+++ b/llvm/lib/MC/XCOFFObjectWriter.cpp
@@ -78,15 +78,7 @@
// (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 @@
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 @@
// 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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
}
if (!SectionAddressSet) {
- Section->Address = Group->Csects.front().Address;
+ Section->Address = Group->front().Address;
SectionAddressSet = true;
}
}