From 75006466296ed4b0f845cbbec4bf77c21de43b40 Mon Sep 17 00:00:00 2001 From: Fangrui Song Date: Sat, 22 Jun 2024 00:34:16 -0700 Subject: [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. --- llvm/include/llvm/MC/MCAsmLayout.h | 2 +- llvm/include/llvm/MC/MCObjectStreamer.h | 12 +--- llvm/include/llvm/MC/MCSection.h | 7 --- llvm/lib/MC/MCAssembler.cpp | 9 ++- llvm/lib/MC/MCObjectStreamer.cpp | 102 ++------------------------------ llvm/lib/MC/MCSection.cpp | 27 --------- llvm/test/MC/MachO/pending-labels.s | 20 +++---- llvm/tools/dsymutil/MachOUtils.cpp | 1 - 8 files changed, 23 insertions(+), 157 deletions(-) diff --git a/llvm/include/llvm/MC/MCAsmLayout.h b/llvm/include/llvm/MC/MCAsmLayout.h index 6fbfce7..e3f707f 100644 --- a/llvm/include/llvm/MC/MCAsmLayout.h +++ b/llvm/include/llvm/MC/MCAsmLayout.h @@ -45,7 +45,7 @@ public: /// 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 b78c4dd..f4406b25 100644 --- a/llvm/include/llvm/MC/MCObjectStreamer.h +++ b/llvm/include/llvm/MC/MCObjectStreamer.h @@ -91,7 +91,6 @@ public: MCFragment *getCurrentFragment() const; void insert(MCFragment *F) { - flushPendingLabels(F); MCSection *CurSection = getCurrentSectionOnly(); CurSection->addFragment(*F); F->setParent(CurSection); @@ -106,24 +105,15 @@ public: 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 9b3df81..969f54c 100644 --- a/llvm/include/llvm/MC/MCSection.h +++ b/llvm/include/llvm/MC/MCSection.h @@ -226,13 +226,6 @@ public: /// 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 6927556..671f6f7 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(RequiredBundlePadding)); EF->Offset += RequiredBundlePadding; + if (auto *DF = dyn_cast_or_null(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(this)->layoutBundle(&F); + const_cast(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 d138e69..f724973 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(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> 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(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(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( 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 0bf641c..495826e 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 a464435..b210daa 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 b52ab1c..b08ae4c 100644 --- a/llvm/tools/dsymutil/MachOUtils.cpp +++ b/llvm/tools/dsymutil/MachOUtils.cpp @@ -381,7 +381,6 @@ bool generateDsymCompanion( auto &Writer = static_cast(MCAsm.getWriter()); // Layout but don't emit. - ObjectStreamer.flushPendingLabels(); MCAsmLayout Layout(MCAsm); MCAsm.layout(Layout); -- cgit v1.1