Skip to content

[MC] Replace fragment ilist with singly-linked lists #95077

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions llvm/include/llvm/MC/MCFragment.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,15 @@

namespace llvm {

class MCAssembler;
class MCSection;
class MCSubtargetInfo;
class MCSymbol;

class MCFragment : public ilist_node_with_parent<MCFragment, MCSection> {
class MCFragment {
friend class MCAsmLayout;
friend class MCAssembler;
friend class MCSection;

public:
enum FragmentType : uint8_t {
Expand All @@ -51,6 +54,9 @@ class MCFragment : public ilist_node_with_parent<MCFragment, MCSection> {
};

private:
// The next fragment within the section.
MCFragment *Next = nullptr;

/// The data for the section this fragment is in.
MCSection *Parent;

Expand All @@ -64,10 +70,6 @@ class MCFragment : public ilist_node_with_parent<MCFragment, MCSection> {
/// The layout order of this fragment.
unsigned LayoutOrder;

/// The subsection this fragment belongs to. This is 0 if the fragment is not
// in any subsection.
unsigned SubsectionNumber = 0;

FragmentType Kind;

protected:
Expand All @@ -88,6 +90,8 @@ class MCFragment : public ilist_node_with_parent<MCFragment, MCSection> {
/// This method will dispatch to the appropriate subclass.
void destroy();

MCFragment *getNext() const { return Next; }

FragmentType getKind() const { return Kind; }

MCSection *getParent() const { return Parent; }
Expand All @@ -104,9 +108,6 @@ class MCFragment : public ilist_node_with_parent<MCFragment, MCSection> {
bool hasInstructions() const { return HasInstructions; }

void dump() const;

void setSubsectionNumber(unsigned Value) { SubsectionNumber = Value; }
unsigned getSubsectionNumber() const { return SubsectionNumber; }
};

class MCDummyFragment : public MCFragment {
Expand Down
3 changes: 1 addition & 2 deletions llvm/include/llvm/MC/MCObjectStreamer.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ class raw_pwrite_stream;
/// implementation.
class MCObjectStreamer : public MCStreamer {
std::unique_ptr<MCAssembler> Assembler;
MCSection::iterator CurInsertionPoint;
bool EmitEHFrame;
bool EmitDebugFrame;
SmallVector<MCSymbol *, 2> PendingLabels;
Expand Down Expand Up @@ -94,7 +93,7 @@ class MCObjectStreamer : public MCStreamer {
void insert(MCFragment *F) {
flushPendingLabels(F);
MCSection *CurSection = getCurrentSectionOnly();
CurSection->getFragmentList().insert(CurInsertionPoint, F);
CurSection->addFragment(*F);
F->setParent(CurSection);
}

Expand Down
75 changes: 43 additions & 32 deletions llvm/include/llvm/MC/MCSection.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
#define LLVM_MC_MCSECTION_H

#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/ilist.h"
#include "llvm/MC/MCFragment.h"
#include "llvm/MC/SectionKind.h"
#include "llvm/Support/Alignment.h"
Expand All @@ -24,20 +23,18 @@
namespace llvm {

class MCAsmInfo;
class MCAssembler;
class MCContext;
class MCExpr;
class MCSymbol;
class raw_ostream;
class Triple;

template <> struct ilist_alloc_traits<MCFragment> {
static void deleteNode(MCFragment *V);
};

/// Instances of this class represent a uniqued identifier for a section in the
/// current translation unit. The MCContext class uniques and creates these.
class MCSection {
public:
friend MCAssembler;
static constexpr unsigned NonUniqueID = ~0U;

enum SectionVariant {
Expand All @@ -58,12 +55,29 @@ class MCSection {
BundleLockedAlignToEnd
};

using FragmentListType = iplist<MCFragment>;
struct iterator {
MCFragment *F = nullptr;
iterator() = default;
explicit iterator(MCFragment *F) : F(F) {}
MCFragment &operator*() const { return *F; }
bool operator==(const iterator &O) const { return F == O.F; }
bool operator!=(const iterator &O) const { return F != O.F; }
iterator &operator++() {
F = F->Next;
return *this;
}
iterator operator++(int) { return iterator(F->Next); }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This postfix increment operator does not change its own value. It does not seem to be used so far, though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for noticing this unused/misleading function. I'll remove it.

};

using const_iterator = FragmentListType::const_iterator;
using iterator = FragmentListType::iterator;
struct FragList {
MCFragment *Head = nullptr;
MCFragment *Tail = nullptr;
};

private:
// At parse time, this holds the fragment list of the current subsection. At
// layout time, this holds the concatenated fragment lists of all subsections.
FragList *CurFragList;
MCSymbol *Begin;
MCSymbol *End = nullptr;
/// The alignment requirement of this section.
Expand Down Expand Up @@ -92,11 +106,10 @@ class MCSection {

MCDummyFragment DummyFragment;

FragmentListType Fragments;

/// Mapping from subsection number to insertion point for subsection numbers
/// below that number.
SmallVector<std::pair<unsigned, MCFragment *>, 1> SubsectionFragmentMap;
// Mapping from subsection number to fragment list. At layout time, the
// subsection 0 list is replaced with concatenated fragments from all
// subsections.
SmallVector<std::pair<unsigned, FragList>, 1> Subsections;

/// State for tracking labels that don't yet have Fragments
struct PendingLabel {
Expand Down Expand Up @@ -171,29 +184,27 @@ class MCSection {
bool isRegistered() const { return IsRegistered; }
void setIsRegistered(bool Value) { IsRegistered = Value; }

MCSection::FragmentListType &getFragmentList() { return Fragments; }
const MCSection::FragmentListType &getFragmentList() const {
return const_cast<MCSection *>(this)->getFragmentList();
}

/// Support for MCFragment::getNextNode().
static FragmentListType MCSection::*getSublistAccess(MCFragment *) {
return &MCSection::Fragments;
}

const MCDummyFragment &getDummyFragment() const { return DummyFragment; }
MCDummyFragment &getDummyFragment() { return DummyFragment; }

iterator begin() { return Fragments.begin(); }
const_iterator begin() const { return Fragments.begin(); }

iterator end() { return Fragments.end(); }
const_iterator end() const { return Fragments.end(); }
bool empty() const { return Fragments.empty(); }

void addFragment(MCFragment &F) { Fragments.push_back(&F); }
FragList *curFragList() const { return CurFragList; }
iterator begin() const { return iterator(CurFragList->Head); }
iterator end() const { return {}; }
bool empty() const { return !CurFragList->Head; }

void addFragment(MCFragment &F) {
// The formal layout order will be finalized in MCAssembler::layout.
if (CurFragList->Tail) {
CurFragList->Tail->Next = &F;
F.setLayoutOrder(CurFragList->Tail->getLayoutOrder() + 1);
} else {
CurFragList->Head = &F;
assert(F.getLayoutOrder() == 0);
}
CurFragList->Tail = &F;
}

MCSection::iterator getSubsectionInsertionPoint(unsigned Subsection);
void switchSubsection(unsigned Subsection);

void dump() const;

Expand Down
15 changes: 14 additions & 1 deletion llvm/lib/MC/MCAssembler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -831,6 +831,19 @@ void MCAssembler::layout(MCAsmLayout &Layout) {
MCSection *Sec = Layout.getSectionOrder()[i];
Sec->setLayoutOrder(i);

// Chain together fragments from all subsections.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe if (Sec->Subsections.size() > 1)? But shouldn't matter much, so feel free to disregard.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is not in a bottleneck, I'll keep simple and avoid the condition.

MCDummyFragment Dummy(Sec);
MCFragment *Tail = &Dummy;
for (auto &[_, List] : Sec->Subsections) {
if (!List.Head)
continue;
Tail->Next = List.Head;
Tail = List.Tail;
}
Sec->Subsections.clear();
Sec->Subsections.push_back({0u, {Dummy.getNext(), Tail}});
Sec->CurFragList = &Sec->Subsections[0].second;

unsigned FragmentIndex = 0;
for (MCFragment &Frag : *Sec)
Frag.setLayoutOrder(FragmentIndex++);
Expand Down Expand Up @@ -1094,7 +1107,7 @@ bool MCAssembler::relaxBoundaryAlign(MCAsmLayout &Layout,

uint64_t AlignedOffset = Layout.getFragmentOffset(&BF);
uint64_t AlignedSize = 0;
for (const MCFragment *F = BF.getNextNode();; F = F->getNextNode()) {
for (const MCFragment *F = BF.getNext();; F = F->getNext()) {
AlignedSize += computeFragmentSize(Layout, *F);
if (F == BF.getLastFragment())
break;
Expand Down
31 changes: 12 additions & 19 deletions llvm/lib/MC/MCExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -661,25 +661,16 @@ static void AttemptToFoldSymbolOffsetDifference(
// this is important when the Subtarget is changed and a new MCDataFragment
// is created in the case of foo: instr; .arch_extension ext; instr .if . -
// foo.
if (SA.isVariable() || SB.isVariable() ||
FA->getSubsectionNumber() != FB->getSubsectionNumber())
if (SA.isVariable() || SB.isVariable())
return;

// Try to find a constant displacement from FA to FB, add the displacement
// between the offset in FA of SA and the offset in FB of SB.
bool Reverse = false;
if (FA == FB) {
if (FA == FB)
Reverse = SA.getOffset() < SB.getOffset();
} else if (!isa<MCDummyFragment>(FA)) {
// Testing FA < FB is slow. Use setLayoutOrder to speed up computation.
// The formal layout order will be finalized in MCAssembler::layout.
if (FA->getLayoutOrder() == 0 || FB->getLayoutOrder()== 0) {
unsigned LayoutOrder = 0;
for (MCFragment &F : *FA->getParent())
F.setLayoutOrder(++LayoutOrder);
}
else if (!isa<MCDummyFragment>(FA))
Reverse = FA->getLayoutOrder() < FB->getLayoutOrder();
}

uint64_t SAOffset = SA.getOffset(), SBOffset = SB.getOffset();
int64_t Displacement = SA.getOffset() - SB.getOffset();
Expand All @@ -695,7 +686,7 @@ static void AttemptToFoldSymbolOffsetDifference(
// instruction, the difference cannot be resolved as it may be changed by
// the linker.
bool BBeforeRelax = false, AAfterRelax = false;
for (auto FI = FB->getIterator(), FE = SecA.end(); FI != FE; ++FI) {
for (auto FI = FB; FI; FI = FI->getNext()) {
auto DF = dyn_cast<MCDataFragment>(FI);
if (DF && DF->isLinkerRelaxable()) {
if (&*FI != FB || SBOffset != DF->getContents().size())
Expand Down Expand Up @@ -726,12 +717,14 @@ static void AttemptToFoldSymbolOffsetDifference(
return;
}
}
// If the previous loop does not find FA, FA must be a dummy fragment not in
// the fragment list (which means SA is a pending label (see
// flushPendingLabels)). In either case, we can resolve the difference.
assert(Found || isa<MCDummyFragment>(FA));
Addend += Reverse ? -Displacement : Displacement;
FinalizeFolding();
// If FA and FB belong to the same subsection, either the previous loop
// found FA, or FA is a dummy fragment not in the fragment list (which means
// SA is a pending label (see flushPendingLabels)) or FA and FB belong to
// different subsections. In either case, we can resolve the difference.
if (Found || isa<MCDummyFragment>(FA)) {
Addend += Reverse ? -Displacement : Displacement;
FinalizeFolding();
}
}
}

Expand Down
4 changes: 1 addition & 3 deletions llvm/lib/MC/MCFragment.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ const MCSymbol *MCAsmLayout::getBaseSymbol(const MCSymbol &Symbol) const {

uint64_t MCAsmLayout::getSectionAddressSize(const MCSection *Sec) const {
// The size is the last fragment's end offset.
const MCFragment &F = Sec->getFragmentList().back();
const MCFragment &F = *Sec->curFragList()->Tail;
return getFragmentOffset(&F) + getAssembler().computeFragmentSize(*this, F);
}

Expand Down Expand Up @@ -197,8 +197,6 @@ uint64_t llvm::computeBundlePadding(const MCAssembler &Assembler,

/* *** */

void ilist_alloc_traits<MCFragment>::deleteNode(MCFragment *V) { V->destroy(); }

MCFragment::MCFragment(FragmentType Kind, bool HasInstructions,
MCSection *Parent)
: Parent(Parent), Atom(nullptr), Offset(~UINT64_C(0)), LayoutOrder(0),
Expand Down
11 changes: 2 additions & 9 deletions llvm/lib/MC/MCObjectStreamer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,6 @@ void MCObjectStreamer::reset() {
if (getContext().getTargetOptions())
Assembler->setRelaxAll(getContext().getTargetOptions()->MCRelaxAll);
}
CurInsertionPoint = MCSection::iterator();
EmitEHFrame = true;
EmitDebugFrame = false;
PendingLabels.clear();
Expand All @@ -200,12 +199,7 @@ void MCObjectStreamer::emitFrames(MCAsmBackend *MAB) {
}

MCFragment *MCObjectStreamer::getCurrentFragment() const {
assert(getCurrentSectionOnly() && "No current section!");

if (CurInsertionPoint != getCurrentSectionOnly()->begin())
return &*std::prev(CurInsertionPoint);

return nullptr;
return getCurrentSectionOnly()->curFragList()->Tail;
}

static bool canReuseDataFragment(const MCDataFragment &F,
Expand Down Expand Up @@ -391,8 +385,7 @@ bool MCObjectStreamer::changeSectionImpl(MCSection *Section,
}

CurSubsectionIdx = unsigned(IntSubsection);
CurInsertionPoint =
Section->getSubsectionInsertionPoint(CurSubsectionIdx);
Section->switchSubsection(CurSubsectionIdx);
return Created;
}

Expand Down
Loading
Loading