[llvm] 7500646 - [MC] Remove pending labels
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Sat Jun 22 00:34:20 PDT 2024
Author: Fangrui Song
Date: 2024-06-22T00:34:16-07:00
New Revision: 75006466296ed4b0f845cbbec4bf77c21de43b40
URL: https://github.com/llvm/llvm-project/commit/75006466296ed4b0f845cbbec4bf77c21de43b40
DIFF: https://github.com/llvm/llvm-project/commit/75006466296ed4b0f845cbbec4bf77c21de43b40.diff
LOG: [MC] Remove pending labels
This commit removes the complexity introduced by pending labels in
https://reviews.llvm.org/D5915 by using a simpler approach. D5915 aimed
to ensure padding placement before `.Ltmp0` for the following code, but
at the cost of expensive per-instruction `flushPendingLabels`.
```
// similar to llvm/test/MC/X86/AlignedBundling/labeloffset.s
.bundle_lock align_to_end
calll .L0$pb
.bundle_unlock
.L0$pb:
popl %eax
.Ltmp0: //// padding should be inserted before this label instead of after
addl $_GLOBAL_OFFSET_TABLE_+(.Ltmp0-.L0$pb), %eax
```
(D5915 was adjusted by https://reviews.llvm.org/D8072 and
https://reviews.llvm.org/D71368)
This patch achieves the same goal by setting the offset of the empty
MCDataFragment (`Prev`) in `layoutBundle`. This eliminates the need for
pending labels and simplifies the code.
llvm/test/MC/MachO/pending-labels.s (D71368): relocation symbols are
changed, but the result is still supported by linkers.
Added:
Modified:
llvm/include/llvm/MC/MCAsmLayout.h
llvm/include/llvm/MC/MCObjectStreamer.h
llvm/include/llvm/MC/MCSection.h
llvm/lib/MC/MCAssembler.cpp
llvm/lib/MC/MCObjectStreamer.cpp
llvm/lib/MC/MCSection.cpp
llvm/test/MC/MachO/pending-labels.s
llvm/tools/dsymutil/MachOUtils.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/MC/MCAsmLayout.h b/llvm/include/llvm/MC/MCAsmLayout.h
index 6fbfce78759e1..e3f707fd21a18 100644
--- a/llvm/include/llvm/MC/MCAsmLayout.h
+++ b/llvm/include/llvm/MC/MCAsmLayout.h
@@ -45,7 +45,7 @@ class MCAsmLayout {
/// its bundle padding will be recomputed.
void invalidateFragmentsFrom(MCFragment *F);
- void layoutBundle(MCFragment *F);
+ void layoutBundle(MCFragment *Prev, MCFragment *F);
/// \name Section Access (in layout order)
/// @{
diff --git a/llvm/include/llvm/MC/MCObjectStreamer.h b/llvm/include/llvm/MC/MCObjectStreamer.h
index b78c4dd85d96e..f4406b255231d 100644
--- a/llvm/include/llvm/MC/MCObjectStreamer.h
+++ b/llvm/include/llvm/MC/MCObjectStreamer.h
@@ -91,7 +91,6 @@ class MCObjectStreamer : public MCStreamer {
MCFragment *getCurrentFragment() const;
void insert(MCFragment *F) {
- flushPendingLabels(F);
MCSection *CurSection = getCurrentSectionOnly();
CurSection->addFragment(*F);
F->setParent(CurSection);
@@ -106,24 +105,15 @@ class MCObjectStreamer : public MCStreamer {
protected:
bool changeSectionImpl(MCSection *Section, const MCExpr *Subsection);
- /// Assign a label to the current Section and Subsection even though a
- /// fragment is not yet present. Use flushPendingLabels(F) to associate
- /// a fragment with this label.
- void addPendingLabel(MCSymbol* label);
-
/// If any labels have been emitted but not assigned fragments in the current
/// Section and Subsection, ensure that they get assigned to fragment F.
/// Optionally, one can provide an offset \p FOffset as a symbol offset within
/// the fragment.
- void flushPendingLabels(MCFragment *F, uint64_t FOffset = 0);
+ void flushPendingLabels(MCFragment *F, uint64_t FOffset = 0) {}
public:
void visitUsedSymbol(const MCSymbol &Sym) override;
- /// Create a data fragment for any pending labels across all Sections
- /// and Subsections.
- void flushPendingLabels();
-
MCAssembler &getAssembler() { return *Assembler; }
MCAssembler *getAssemblerPtr() override;
/// \name MCStreamer Interface
diff --git a/llvm/include/llvm/MC/MCSection.h b/llvm/include/llvm/MC/MCSection.h
index 9b3df81324226..969f54cca2382 100644
--- a/llvm/include/llvm/MC/MCSection.h
+++ b/llvm/include/llvm/MC/MCSection.h
@@ -226,13 +226,6 @@ class MCSection {
/// Add a pending label for the requested subsection. This label will be
/// associated with a fragment in flushPendingLabels()
void addPendingLabel(MCSymbol* label, unsigned Subsection = 0);
-
- /// Associate all pending labels in a subsection with a fragment.
- void flushPendingLabels(MCFragment *F, unsigned Subsection);
-
- /// Associate all pending labels with empty data fragments. One fragment
- /// will be created for each subsection as necessary.
- void flushPendingLabels();
};
} // end namespace llvm
diff --git a/llvm/lib/MC/MCAssembler.cpp b/llvm/lib/MC/MCAssembler.cpp
index 6927556e678b1..671f6f71b3a4f 100644
--- a/llvm/lib/MC/MCAssembler.cpp
+++ b/llvm/lib/MC/MCAssembler.cpp
@@ -403,7 +403,7 @@ uint64_t MCAssembler::computeFragmentSize(const MCAsmLayout &Layout,
llvm_unreachable("invalid fragment kind");
}
-void MCAsmLayout::layoutBundle(MCFragment *F) {
+void MCAsmLayout::layoutBundle(MCFragment *Prev, MCFragment *F) {
// If bundling is enabled and this fragment has instructions in it, it has to
// obey the bundling restrictions. With padding, we'll have:
//
@@ -439,6 +439,9 @@ void MCAsmLayout::layoutBundle(MCFragment *F) {
report_fatal_error("Padding cannot exceed 255 bytes");
EF->setBundlePadding(static_cast<uint8_t>(RequiredBundlePadding));
EF->Offset += RequiredBundlePadding;
+ if (auto *DF = dyn_cast_or_null<MCDataFragment>(Prev))
+ if (DF->getContents().empty())
+ DF->Offset = EF->Offset;
}
uint64_t MCAsmLayout::getFragmentOffset(const MCFragment *F) const {
@@ -451,14 +454,16 @@ void MCAsmLayout::ensureValid(const MCFragment *Frag) const {
if (Sec.hasLayout())
return;
Sec.setHasLayout(true);
+ MCFragment *Prev = nullptr;
uint64_t Offset = 0;
for (MCFragment &F : Sec) {
F.Offset = Offset;
if (Assembler.isBundlingEnabled() && F.hasInstructions()) {
- const_cast<MCAsmLayout *>(this)->layoutBundle(&F);
+ const_cast<MCAsmLayout *>(this)->layoutBundle(Prev, &F);
Offset = F.Offset;
}
Offset += getAssembler().computeFragmentSize(*this, F);
+ Prev = &F;
}
}
diff --git a/llvm/lib/MC/MCObjectStreamer.cpp b/llvm/lib/MC/MCObjectStreamer.cpp
index d138e69a78540..f724973f46449 100644
--- a/llvm/lib/MC/MCObjectStreamer.cpp
+++ b/llvm/lib/MC/MCObjectStreamer.cpp
@@ -46,59 +46,6 @@ MCAssembler *MCObjectStreamer::getAssemblerPtr() {
return nullptr;
}
-void MCObjectStreamer::addPendingLabel(MCSymbol* S) {
- MCSection *CurSection = getCurrentSectionOnly();
- if (CurSection) {
- // Register labels that have not yet been assigned to a Section.
- if (!PendingLabels.empty()) {
- for (MCSymbol* Sym : PendingLabels)
- CurSection->addPendingLabel(Sym);
- PendingLabels.clear();
- }
-
- // Add this label to the current Section / Subsection.
- CurSection->addPendingLabel(S, CurSubsectionIdx);
-
- // Add this Section to the list of PendingLabelSections.
- PendingLabelSections.insert(CurSection);
- } else
- // There is no Section / Subsection for this label yet.
- PendingLabels.push_back(S);
-}
-
-void MCObjectStreamer::flushPendingLabels(MCFragment *F, uint64_t FOffset) {
- assert(F);
- MCSection *CurSection = getCurrentSectionOnly();
- if (!CurSection) {
- assert(PendingLabels.empty());
- return;
- }
- // Register labels that have not yet been assigned to a Section.
- if (!PendingLabels.empty()) {
- for (MCSymbol* Sym : PendingLabels)
- CurSection->addPendingLabel(Sym, CurSubsectionIdx);
- PendingLabels.clear();
- }
-
- // Associate the labels with F.
- CurSection->flushPendingLabels(F, CurSubsectionIdx);
-}
-
-void MCObjectStreamer::flushPendingLabels() {
- // Register labels that have not yet been assigned to a Section.
- if (!PendingLabels.empty()) {
- MCSection *CurSection = getCurrentSectionOnly();
- assert(CurSection);
- for (MCSymbol* Sym : PendingLabels)
- CurSection->addPendingLabel(Sym, CurSubsectionIdx);
- PendingLabels.clear();
- }
-
- // Assign an empty data fragment to all remaining pending labels.
- for (MCSection* Section : PendingLabelSections)
- Section->flushPendingLabels();
-}
-
// When fixup's offset is a forward declared label, e.g.:
//
// .reloc 1f, R_MIPS_JALR, foo
@@ -113,7 +60,6 @@ void MCObjectStreamer::resolvePendingFixups() {
"unresolved relocation offset");
continue;
}
- flushPendingLabels(PendingFixup.DF, PendingFixup.DF->getContents().size());
PendingFixup.Fixup.setOffset(PendingFixup.Sym->getOffset() +
PendingFixup.Fixup.getOffset());
@@ -245,7 +191,6 @@ void MCObjectStreamer::emitValueImpl(const MCExpr *Value, unsigned Size,
SMLoc Loc) {
MCStreamer::emitValueImpl(Value, Size, Loc);
MCDataFragment *DF = getOrCreateDataFragment();
- flushPendingLabels(DF, DF->getContents().size());
MCDwarfLineEntry::make(this, getCurrentSectionOnly());
@@ -291,17 +236,9 @@ void MCObjectStreamer::emitLabel(MCSymbol *Symbol, SMLoc Loc) {
// If there is a current fragment, mark the symbol as pointing into it.
// Otherwise queue the label and set its fragment pointer when we emit the
// next fragment.
- auto *F = dyn_cast_or_null<MCDataFragment>(getCurrentFragment());
- if (F) {
- Symbol->setFragment(F);
- Symbol->setOffset(F->getContents().size());
- } else {
- // Assign all pending labels to offset 0 within the dummy "pending"
- // fragment. (They will all be reassigned to a real fragment in
- // flushPendingLabels())
- Symbol->setOffset(0);
- addPendingLabel(Symbol);
- }
+ MCDataFragment *F = getOrCreateDataFragment();
+ Symbol->setFragment(F);
+ Symbol->setOffset(F->getContents().size());
emitPendingAssignments(Symbol);
}
@@ -598,11 +535,9 @@ void MCObjectStreamer::emitCVInlineLinetableDirective(
void MCObjectStreamer::emitCVDefRangeDirective(
ArrayRef<std::pair<const MCSymbol *, const MCSymbol *>> Ranges,
StringRef FixedSizePortion) {
- MCFragment *Frag =
- getContext().getCVContext().emitDefRange(*this, Ranges, FixedSizePortion);
+ getContext().getCVContext().emitDefRange(*this, Ranges, FixedSizePortion);
// Attach labels that were pending before we created the defrange fragment to
// the beginning of the new fragment.
- flushPendingLabels(Frag, 0);
this->MCStreamer::emitCVDefRangeDirective(Ranges, FixedSizePortion);
}
@@ -620,7 +555,6 @@ void MCObjectStreamer::emitCVFileChecksumOffsetDirective(unsigned FileNo) {
void MCObjectStreamer::emitBytes(StringRef Data) {
MCDwarfLineEntry::make(this, getCurrentSectionOnly());
MCDataFragment *DF = getOrCreateDataFragment();
- flushPendingLabels(DF, DF->getContents().size());
DF->getContents().append(Data.begin(), Data.end());
}
@@ -653,8 +587,6 @@ void MCObjectStreamer::emitValueToOffset(const MCExpr *Offset,
// Associate DTPRel32 fixup with data and resize data area
void MCObjectStreamer::emitDTPRel32Value(const MCExpr *Value) {
MCDataFragment *DF = getOrCreateDataFragment();
- flushPendingLabels(DF, DF->getContents().size());
-
DF->getFixups().push_back(MCFixup::create(DF->getContents().size(),
Value, FK_DTPRel_4));
DF->getContents().resize(DF->getContents().size() + 4, 0);
@@ -663,8 +595,6 @@ void MCObjectStreamer::emitDTPRel32Value(const MCExpr *Value) {
// Associate DTPRel64 fixup with data and resize data area
void MCObjectStreamer::emitDTPRel64Value(const MCExpr *Value) {
MCDataFragment *DF = getOrCreateDataFragment();
- flushPendingLabels(DF, DF->getContents().size());
-
DF->getFixups().push_back(MCFixup::create(DF->getContents().size(),
Value, FK_DTPRel_8));
DF->getContents().resize(DF->getContents().size() + 8, 0);
@@ -673,8 +603,6 @@ void MCObjectStreamer::emitDTPRel64Value(const MCExpr *Value) {
// Associate TPRel32 fixup with data and resize data area
void MCObjectStreamer::emitTPRel32Value(const MCExpr *Value) {
MCDataFragment *DF = getOrCreateDataFragment();
- flushPendingLabels(DF, DF->getContents().size());
-
DF->getFixups().push_back(MCFixup::create(DF->getContents().size(),
Value, FK_TPRel_4));
DF->getContents().resize(DF->getContents().size() + 4, 0);
@@ -683,8 +611,6 @@ void MCObjectStreamer::emitTPRel32Value(const MCExpr *Value) {
// Associate TPRel64 fixup with data and resize data area
void MCObjectStreamer::emitTPRel64Value(const MCExpr *Value) {
MCDataFragment *DF = getOrCreateDataFragment();
- flushPendingLabels(DF, DF->getContents().size());
-
DF->getFixups().push_back(MCFixup::create(DF->getContents().size(),
Value, FK_TPRel_8));
DF->getContents().resize(DF->getContents().size() + 8, 0);
@@ -693,8 +619,6 @@ void MCObjectStreamer::emitTPRel64Value(const MCExpr *Value) {
// Associate GPRel32 fixup with data and resize data area
void MCObjectStreamer::emitGPRel32Value(const MCExpr *Value) {
MCDataFragment *DF = getOrCreateDataFragment();
- flushPendingLabels(DF, DF->getContents().size());
-
DF->getFixups().push_back(
MCFixup::create(DF->getContents().size(), Value, FK_GPRel_4));
DF->getContents().resize(DF->getContents().size() + 4, 0);
@@ -703,8 +627,6 @@ void MCObjectStreamer::emitGPRel32Value(const MCExpr *Value) {
// Associate GPRel64 fixup with data and resize data area
void MCObjectStreamer::emitGPRel64Value(const MCExpr *Value) {
MCDataFragment *DF = getOrCreateDataFragment();
- flushPendingLabels(DF, DF->getContents().size());
-
DF->getFixups().push_back(
MCFixup::create(DF->getContents().size(), Value, FK_GPRel_4));
DF->getContents().resize(DF->getContents().size() + 8, 0);
@@ -789,8 +711,6 @@ MCObjectStreamer::emitRelocDirective(const MCExpr &Offset, StringRef Name,
MCSymbolRefExpr::create(getContext().createTempSymbol(), getContext());
MCDataFragment *DF = getOrCreateDataFragment(&STI);
- flushPendingLabels(DF, DF->getContents().size());
-
MCValue OffsetVal;
if (!Offset.evaluateAsRelocatable(OffsetVal, nullptr, nullptr))
return std::make_pair(false,
@@ -830,9 +750,6 @@ MCObjectStreamer::emitRelocDirective(const MCExpr &Offset, StringRef Name,
void MCObjectStreamer::emitFill(const MCExpr &NumBytes, uint64_t FillValue,
SMLoc Loc) {
- MCDataFragment *DF = getOrCreateDataFragment();
- flushPendingLabels(DF, DF->getContents().size());
-
assert(getCurrentSectionOnly() && "need a section");
insert(
getContext().allocFragment<MCFillFragment>(FillValue, 1, NumBytes, Loc));
@@ -861,9 +778,6 @@ void MCObjectStreamer::emitFill(const MCExpr &NumValues, int64_t Size,
}
// Otherwise emit as fragment.
- MCDataFragment *DF = getOrCreateDataFragment();
- flushPendingLabels(DF, DF->getContents().size());
-
assert(getCurrentSectionOnly() && "need a section");
insert(
getContext().allocFragment<MCFillFragment>(Expr, Size, NumValues, Loc));
@@ -871,12 +785,7 @@ void MCObjectStreamer::emitFill(const MCExpr &NumValues, int64_t Size,
void MCObjectStreamer::emitNops(int64_t NumBytes, int64_t ControlledNopLength,
SMLoc Loc, const MCSubtargetInfo &STI) {
- // Emit an NOP fragment.
- MCDataFragment *DF = getOrCreateDataFragment();
- flushPendingLabels(DF, DF->getContents().size());
-
assert(getCurrentSectionOnly() && "need a section");
-
insert(getContext().allocFragment<MCNopsFragment>(
NumBytes, ControlledNopLength, Loc, STI));
}
@@ -916,9 +825,6 @@ void MCObjectStreamer::finishImpl() {
// Emit pseudo probes for the current module.
MCPseudoProbeTable::emit(this);
- // Update any remaining pending labels with empty data fragments.
- flushPendingLabels();
-
resolvePendingFixups();
getAssembler().Finish();
}
diff --git a/llvm/lib/MC/MCSection.cpp b/llvm/lib/MC/MCSection.cpp
index 0bf641c4427eb..495826efb6a2a 100644
--- a/llvm/lib/MC/MCSection.cpp
+++ b/llvm/lib/MC/MCSection.cpp
@@ -80,33 +80,6 @@ void MCSection::switchSubsection(unsigned Subsection) {
StringRef MCSection::getVirtualSectionKind() const { return "virtual"; }
void MCSection::addPendingLabel(MCSymbol *label, unsigned Subsection) {
- PendingLabels.push_back(PendingLabel(label, Subsection));
-}
-
-void MCSection::flushPendingLabels(MCFragment *F, unsigned Subsection) {
- // Set the fragment and fragment offset for all pending symbols in the
- // specified Subsection, and remove those symbols from the pending list.
- for (auto It = PendingLabels.begin(); It != PendingLabels.end(); ++It) {
- PendingLabel& Label = *It;
- if (Label.Subsection == Subsection) {
- Label.Sym->setFragment(F);
- assert(Label.Sym->getOffset() == 0);
- PendingLabels.erase(It--);
- }
- }
-}
-
-void MCSection::flushPendingLabels() {
- // Make sure all remaining pending labels point to data fragments, by
- // creating new empty data fragments for each Subsection with labels pending.
- while (!PendingLabels.empty()) {
- PendingLabel& Label = PendingLabels[0];
- switchSubsection(Label.Subsection);
- MCFragment *F = new MCDataFragment();
- addFragment(*F);
- F->setParent(this);
- flushPendingLabels(F, Label.Subsection);
- }
}
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
diff --git a/llvm/test/MC/MachO/pending-labels.s b/llvm/test/MC/MachO/pending-labels.s
index a4644357ca4ee..b210daace0e0a 100644
--- a/llvm/test/MC/MachO/pending-labels.s
+++ b/llvm/test/MC/MachO/pending-labels.s
@@ -32,17 +32,17 @@ __data:
.quad L8-.
// CHECK: 0000000000000038 X86_64_RELOC_SUBTRACTOR _function1-__data
// CHECK: 0000000000000038 X86_64_RELOC_UNSIGNED _function1
-// CHECK: 0000000000000030 X86_64_RELOC_SUBTRACTOR _function1-__data
-// CHECK: 0000000000000030 X86_64_RELOC_UNSIGNED _function1
+// CHECK: 0000000000000030 X86_64_RELOC_SUBTRACTOR __text-__data
+// CHECK: 0000000000000030 X86_64_RELOC_UNSIGNED __text
// CHECK: 0000000000000028 X86_64_RELOC_SUBTRACTOR _function2-__data
// CHECK: 0000000000000028 X86_64_RELOC_UNSIGNED _function2
// CHECK: 0000000000000020 X86_64_RELOC_SUBTRACTOR _function2-__data
// CHECK: 0000000000000020 X86_64_RELOC_UNSIGNED _function2
-// CHECK: 0000000000000018 X86_64_RELOC_SUBTRACTOR _function2-__data
-// CHECK: 0000000000000018 X86_64_RELOC_UNSIGNED _function2
-// CHECK: 0000000000000010 X86_64_RELOC_SUBTRACTOR _function1-__data
-// CHECK: 0000000000000010 X86_64_RELOC_UNSIGNED _function1
-// CHECK: 0000000000000008 X86_64_RELOC_SUBTRACTOR _function2-__data
-// CHECK: 0000000000000008 X86_64_RELOC_UNSIGNED _function2
-// CHECK: 0000000000000000 X86_64_RELOC_SUBTRACTOR _function1-__data
-// CHECK: 0000000000000000 X86_64_RELOC_UNSIGNED _function1
+// CHECK: 0000000000000018 X86_64_RELOC_SUBTRACTOR __text_cold-__data
+// CHECK: 0000000000000018 X86_64_RELOC_UNSIGNED __text_cold
+// CHECK: 0000000000000010 X86_64_RELOC_SUBTRACTOR __text-__data
+// CHECK: 0000000000000010 X86_64_RELOC_UNSIGNED __text
+// CHECK: 0000000000000008 X86_64_RELOC_SUBTRACTOR __text_cold-__data
+// CHECK: 0000000000000008 X86_64_RELOC_UNSIGNED __text_cold
+// CHECK: 0000000000000000 X86_64_RELOC_SUBTRACTOR __text-__data
+// CHECK: 0000000000000000 X86_64_RELOC_UNSIGNED __text
diff --git a/llvm/tools/dsymutil/MachOUtils.cpp b/llvm/tools/dsymutil/MachOUtils.cpp
index b52ab1ce6d294..b08ae4cf64ab9 100644
--- a/llvm/tools/dsymutil/MachOUtils.cpp
+++ b/llvm/tools/dsymutil/MachOUtils.cpp
@@ -381,7 +381,6 @@ bool generateDsymCompanion(
auto &Writer = static_cast<MachObjectWriter &>(MCAsm.getWriter());
// Layout but don't emit.
- ObjectStreamer.flushPendingLabels();
MCAsmLayout Layout(MCAsm);
MCAsm.layout(Layout);
More information about the llvm-commits
mailing list