[MC] Replace fragment ilist with singly-linked lists #95077
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Created using spr 1.3.5-bogner
llvmbot
added
backend:RISC-V
backend:WebAssembly
backend:X86
mc
|
@llvm/pr-subscribers-mc @llvm/pr-subscribers-backend-webassembly Author: Fangrui Song (MaskRay) ChangesFragments are allocated with As the first step, replace ilist with singly-linked lists.
To eliminate Since fragment lists are disconnected before layout time, we can remove Patch is 20.73 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/95077.diff 12 Files Affected:
diff --git a/llvm/include/llvm/MC/MCFragment.h b/llvm/include/llvm/MC/MCFragment.h
index 67e10c3589495..45599c940659e 100644
--- a/llvm/include/llvm/MC/MCFragment.h
+++ b/llvm/include/llvm/MC/MCFragment.h
@@ -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 {
@@ -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;
@@ -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:
@@ -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; }
@@ -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 {
diff --git a/llvm/include/llvm/MC/MCObjectStreamer.h b/llvm/include/llvm/MC/MCObjectStreamer.h
index e212d54613980..c0a337f5ea45e 100644
--- a/llvm/include/llvm/MC/MCObjectStreamer.h
+++ b/llvm/include/llvm/MC/MCObjectStreamer.h
@@ -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;
@@ -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);
}
diff --git a/llvm/include/llvm/MC/MCSection.h b/llvm/include/llvm/MC/MCSection.h
index 217b9b4b5bc52..d4ca8ecda3fc3 100644
--- a/llvm/include/llvm/MC/MCSection.h
+++ b/llvm/include/llvm/MC/MCSection.h
@@ -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"
@@ -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 {
@@ -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); }
+ };
- 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.
@@ -92,12 +106,6 @@ 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;
-
/// State for tracking labels that don't yet have Fragments
struct PendingLabel {
MCSymbol* Sym;
@@ -107,6 +115,11 @@ class MCSection {
};
SmallVector<PendingLabel, 2> PendingLabels;
+ // 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, std::unique_ptr<FragList>>, 1> Subsections;
+
protected:
// TODO Make Name private when possible.
StringRef Name;
@@ -171,29 +184,23 @@ 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) {
+ if (CurFragList->Tail)
+ CurFragList->Tail->Next = &F;
+ else
+ CurFragList->Head = &F;
+ CurFragList->Tail = &F;
+ }
- MCSection::iterator getSubsectionInsertionPoint(unsigned Subsection);
+ void switchSubsection(unsigned Subsection);
void dump() const;
diff --git a/llvm/lib/MC/MCAssembler.cpp b/llvm/lib/MC/MCAssembler.cpp
index 8490853eda87c..a6e3671ee8ceb 100644
--- a/llvm/lib/MC/MCAssembler.cpp
+++ b/llvm/lib/MC/MCAssembler.cpp
@@ -831,6 +831,20 @@ void MCAssembler::layout(MCAsmLayout &Layout) {
MCSection *Sec = Layout.getSectionOrder()[i];
Sec->setLayoutOrder(i);
+ // Chain together fragments from all subsections.
+ MCDummyFragment Dummy(Sec);
+ MCFragment *Tail = &Dummy;
+ for (auto &[_, Chain] : Sec->Subsections) {
+ if (!Chain->Head)
+ continue;
+ Tail->Next = Chain->Head;
+ Tail = Chain->Tail;
+ }
+ Sec->Subsections.resize(1);
+ Sec->Subsections[0].second->Head = Dummy.getNext();
+ Sec->Subsections[0].second->Tail = Tail;
+ Sec->CurFragList = Sec->Subsections[0].second.get();
+
unsigned FragmentIndex = 0;
for (MCFragment &Frag : *Sec)
Frag.setLayoutOrder(FragmentIndex++);
@@ -1094,7 +1108,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;
diff --git a/llvm/lib/MC/MCExpr.cpp b/llvm/lib/MC/MCExpr.cpp
index b70ac86c18ccf..f4890e77138b6 100644
--- a/llvm/lib/MC/MCExpr.cpp
+++ b/llvm/lib/MC/MCExpr.cpp
@@ -661,8 +661,7 @@ 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
@@ -695,7 +694,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())
@@ -726,12 +725,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();
+ }
}
}
diff --git a/llvm/lib/MC/MCFragment.cpp b/llvm/lib/MC/MCFragment.cpp
index 84a587164c788..6d97e8ce552ba 100644
--- a/llvm/lib/MC/MCFragment.cpp
+++ b/llvm/lib/MC/MCFragment.cpp
@@ -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);
}
@@ -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),
diff --git a/llvm/lib/MC/MCObjectStreamer.cpp b/llvm/lib/MC/MCObjectStreamer.cpp
index ae4e6915fa294..bf1ce76cdc14b 100644
--- a/llvm/lib/MC/MCObjectStreamer.cpp
+++ b/llvm/lib/MC/MCObjectStreamer.cpp
@@ -180,7 +180,6 @@ void MCObjectStreamer::reset() {
if (getContext().getTargetOptions())
Assembler->setRelaxAll(getContext().getTargetOptions()->MCRelaxAll);
}
- CurInsertionPoint = MCSection::iterator();
EmitEHFrame = true;
EmitDebugFrame = false;
PendingLabels.clear();
@@ -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,
@@ -391,8 +385,7 @@ bool MCObjectStreamer::changeSectionImpl(MCSection *Section,
}
CurSubsectionIdx = unsigned(IntSubsection);
- CurInsertionPoint =
- Section->getSubsectionInsertionPoint(CurSubsectionIdx);
+ Section->switchSubsection(CurSubsectionIdx);
return Created;
}
diff --git a/llvm/lib/MC/MCSection.cpp b/llvm/lib/MC/MCSection.cpp
index 9848d7fafe764..81f17a5e1c151 100644
--- a/llvm/lib/MC/MCSection.cpp
+++ b/llvm/lib/MC/MCSection.cpp
@@ -24,7 +24,11 @@ MCSection::MCSection(SectionVariant V, StringRef Name, SectionKind K,
MCSymbol *Begin)
: Begin(Begin), BundleGroupBeforeFirstInst(false), HasInstructions(false),
HasLayout(false), IsRegistered(false), DummyFragment(this), Name(Name),
- Variant(V), Kind(K) {}
+ Variant(V), Kind(K) {
+ // Create a fragment list for subsection 0 and set the active subsection to 0.
+ CurFragList =
+ &*Subsections.emplace_back(0u, std::make_unique<FragList>()).second;
+}
MCSymbol *MCSection::getEndSymbol(MCContext &Ctx) {
if (!End)
@@ -34,7 +38,14 @@ MCSymbol *MCSection::getEndSymbol(MCContext &Ctx) {
bool MCSection::hasEnded() const { return End && End->isInSection(); }
-MCSection::~MCSection() = default;
+MCSection::~MCSection() {
+ for (auto &[_, Chain] : Subsections) {
+ for (MCFragment *X = Chain->Head, *Y; X; X = Y) {
+ Y = X->Next;
+ X->destroy();
+ }
+ }
+}
void MCSection::setBundleLockState(BundleLockStateType NewState) {
if (NewState == NotBundleLocked) {
@@ -55,35 +66,17 @@ void MCSection::setBundleLockState(BundleLockStateType NewState) {
++BundleLockNestingDepth;
}
-MCSection::iterator
-MCSection::getSubsectionInsertionPoint(unsigned Subsection) {
- if (Subsection == 0 && SubsectionFragmentMap.empty())
- return end();
-
- SmallVectorImpl<std::pair<unsigned, MCFragment *>>::iterator MI = lower_bound(
- SubsectionFragmentMap, std::make_pair(Subsection, (MCFragment *)nullptr));
- bool ExactMatch = false;
- if (MI != SubsectionFragmentMap.end()) {
- ExactMatch = MI->first == Subsection;
- if (ExactMatch)
- ++MI;
+void MCSection::switchSubsection(unsigned Subsection) {
+ size_t I = 0, E = Subsections.size();
+ while (I != E && Subsections[I].first < Subsection)
+ ++I;
+ // If the subsection number is not in the sorted Subsections list, create a
+ // new fragment list.
+ if (I == E || Subsections[I].first != Subsection) {
+ Subsections.insert(Subsections.begin() + I,
+ {Subsection, std::make_unique<FragList>()});
}
- iterator IP;
- if (MI == SubsectionFragmentMap.end())
- IP = end();
- else
- IP = MI->second->getIterator();
- if (!ExactMatch && Subsection != 0) {
- // The GNU as documentation claims that subsections have an alignment of 4,
- // although this appears not to be the case.
- MCFragment *F = new MCDataFragment();
- SubsectionFragmentMap.insert(MI, std::make_pair(Subsection, F));
- getFragmentList().insert(IP, F);
- F->setParent(this);
- F->setSubsectionNumber(Subsection);
- }
-
- return IP;
+ CurFragList = Subsections[I].second.get();
}
StringRef MCSection::getVirtualSectionKind() const { return "virtual"; }
@@ -111,13 +104,11 @@ void MCSection::flushPendingLabels() {
// creating new empty data fragments for each Subsection with labels pending.
while (!PendingLabels.empty()) {
PendingLabel& Label = PendingLabels[0];
- iterator CurInsertionPoint =
- this->getSubsectionInsertionPoint(Label.Subsection);
- const MCSymbol *Atom = nullptr;
- if (CurInsertionPoint != begin())
- Atom = std::prev(CurInsertionPoint)->getAtom();
+ switchSubsection(Label.Subsection);
+ const MCSymbol *Atom =
+ CurFragList->Tail ? CurFragList->Tail->getAtom() : nullptr;
MCFragment *F = new MCDataFragment();
- getFragmentList().insert(CurInsertionPoint, F);
+ addFragment(*F);
F->setParent(this);
F->setAtom(Atom);
flushPendingLabels(F, 0, Label.Subsection);
diff --git a/llvm/lib/MC/WasmObjectWriter.cpp b/llvm/lib/MC/WasmObjectWriter.cpp
index 451269608f179..522e268156aa3 100644
--- a/llvm/lib/MC/WasmObjectWriter.cpp
+++ b/llvm/lib/MC/WasmObjectWriter.cpp
@@ -1864,15 +1864,14 @@ uint64_t WasmObjectWriter::writeOneObject(MCAssembler &Asm,
if (EmptyFrag.getKind() != MCFragment::FT_Data)
report_fatal_error(".init_array section should be aligned");
- IT = std::next(IT);
- const MCFragment &AlignFrag = *IT;
+ const MCFragment &AlignFrag = *EmptyFrag.getNext();
if (AlignFrag.getKind() != MCFragment::FT_Align)
report_fatal_error(".init_array section should be aligned");
if (cast<MCAlignFragment>(AlignFrag).getAlignment() !=
Align(is64Bit() ? 8 : 4))
report_fatal_error(".init_array section should be aligned for pointers");
- const MCFragment &Frag = *std::next(IT);
+ const MCFragment &Frag = *AlignFrag.getNext();
if (Frag.hasInstructions() || Frag.getKind() != MCFragment::FT_Data)
report_fatal_error("only data supported in .init_array section");
diff --git a/llvm/lib/Target/Hexagon/MCTargetDesc/HexagonAsmBackend.cpp b/llvm/lib/Target/Hexagon/MCTargetDesc/HexagonAsmBackend.cpp
index 3b6ea81cdf10e..54efe4bc25efe 100644
--- a/llvm/lib/Target/Hexagon/MCTargetDesc/HexagonAsmBackend.cpp
+++ b/llvm/lib/Target/Hexagon/MCTargetDesc/HexagonAsmBackend.cpp
@@ -712,17 +712,20 @@ class HexagonAsmBackend : public MCAsmBackend {
void finishLayout(MCAssembler const &Asm,
MCAsmLayout &Layout) const override {
+ SmallVector<MCFragment *> Frags;
for (auto *I : Layout.getSectionOrder()) {
- for (auto &J : *I) {
- switch (J.getKind()) {
+ Frags.clear();
+ for (MCFragment &F : *I)
+ Frags.push_back(&F);
+ for (size_t J = 0, E = Frags.size(); J != E; ++J) {
+ switch (Frags[J]->getKind()) {
default:
break;
case MCFragment::FT_Align: {
- auto Size = Asm.computeFragmentSize(Layout, J);
- for (auto K = J.getIterator();
- K != I->begin() && Size >= HEXAGON_PACKET_SIZE;) {
+ auto Size = Asm.computeFragmentSize(Layout, *Frags[J]);
+ for (auto K = J; K != 0 && Size >= HEXAGON_PACKET_SIZE;) {
--K;
- switch (K->getKind()) {
+ switch (Frags[K]->getKind()) {
default:
break;
case MCFragment::FT_Align: {
@@ -732,7 +735,7 @@ class HexagonAsmBackend : public MCAsmBackend {
}
case MCFragment::FT_Relaxable: {
MCContext &Context = Asm.getContext();
- auto &RF = cast<MCRelaxableFragment>(*K);
+ auto &RF = cast<MCRelaxableFragment>(*Frags[K]);
auto &Inst = const_cast<MCInst &>(RF.getInst());
while (Size > 0 &&
HexagonMCInstrInfo::bundleSize(Inst) < MaxPacketSize) {
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp
index b8e0f3a867f40..d83dadd301619 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp
@@ -62,7 +62,7 @@ const MCFixup *RISCVMCExpr::getPCRelHiFixup(const MCFragment **DFOut) const {
uint64_t Offset = AUIPCSymbol->getOffset();
if (DF->getContents().size() == Offset) {
- DF = dyn_cast_or_null<MCDataFragment>(DF->getNextNode());
+ DF = dyn_cast_or_null<MCDataFragment>(DF->getNext());
if (!DF)
return nullptr;
Offset = 0;
diff --git a/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp b/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
index 30f22cd322fec..7c55beb22706b 100644
--- a/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
+++ b/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
@@ -521,7 +521,7 @@ void X86AsmBackend::emitInstructionBegin(MCObjectStreamer &OS,
if (!CanPadInst)
return;
- if (PendingBA && PendingBA->getNextNode() == OS.getCurrentFragment()) {
+ ...
[truncated]
|
Created using spr 1.3.5-bogner
aengelke
reviewed
Jun 11, 2024
llvm/include/llvm/MC/MCSection.h
Outdated
| // 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, std::unique_ptr<FragList>>, 1> Subsections; |
There was a problem hiding this comment.
Why a unique_ptr? Whenever Subsections is updated, also CurFragList is updated, so no dangling pointer could occur?
There was a problem hiding this comment.
Thx. Switched to plain FragList.
| @@ -831,6 +831,20 @@ void MCAssembler::layout(MCAsmLayout &Layout) { | |||
| MCSection *Sec = Layout.getSectionOrder()[i]; | |||
| Sec->setLayoutOrder(i); | |||
|
|
|||
| // Chain together fragments from all subsections. | |||
There was a problem hiding this comment.
Maybe if (Sec->Subsections.size() > 1)? But shouldn't matter much, so feel free to disregard.
There was a problem hiding this comment.
Since this is not in a bottleneck, I'll keep simple and avoid the condition.
aengelke
approved these changes
Jun 11, 2024
|
max-rss decrease per https://llvm-compile-time-tracker.com/compare.php?from=48f8130a49aad715ff6d5136dad2447d41e9537b&to=075c26f8b177e976f44c8c4dd6a95e80ee7a9510&stat=max-rss&linkStats=on stage1-ReleaseLTO-g (link only):
|
MaskRay
deleted the
users/MaskRay/spr/mc-replace-fragment-ilist-with-singly-linked-lists
branch
MaskRay
added a commit
that referenced
this pull request
Jun 13, 2024
Mach-O's `.subsections_via_symbols` mechanism associates a fragment with an atom (a non-temporary defined symbol). The current approach (`MCFragment::Atom`) wastes space for other object file formats. After #95077, `MCFragment::LayoutOrder` is only used by `AttemptToFoldSymbolOffsetDifference`. While it could be removed, we might explore future uses for `LayoutOrder`. @aengelke suggests one use case: move `Atom` into MCSection. This works because Mach-O doesn't support `.subsection`, and `LayoutOrder`, as the index into the fragment list, is unchanged. This patch moves MCFragment::Atom to MCSectionMachO::Atoms. `getAtom` may be called at parse time before `Atoms` is initialized, so a bound checking is needed to keep the hack working. Pull Request: #95341
MaskRay
added a commit
that referenced
this pull request
Jun 14, 2024
When both aligned bundling and RelaxAll are enabled, bundle padding is directly written into fragments (https://reviews.llvm.org/D8072). (The original motivation was memory usage, which has been achieved from different angles with recent assembler improvement). The code presents challenges with the work to replace fragment representation (e.g. #94950 #95077). This patch removes the special handling. RelaxAll still works but the behavior seems slightly different as revealed by 2 changed tests. However, most `-mc-relax-all` tests are unchanged. RelaxAll used to be the default for clang -O0. This mode has significant code size drawbacks and newer Clang doesn't use it (#90013). --- flushPendingLabels: The FOffset parameter can be removed: pending labels will be assigned to the incoming fragment at offset 0. Pull Request: #95188
| F = F->Next; | ||
| return *this; | ||
| } | ||
| iterator operator++(int) { return iterator(F->Next); } |
There was a problem hiding this comment.
This postfix increment operator does not change its own value. It does not seem to be used so far, though.
There was a problem hiding this comment.
Thank you for noticing this unused/misleading function. I'll remove it.
EthanLuisMcDonough
pushed a commit
to EthanLuisMcDonough/llvm-project
that referenced
this pull request
Aug 13, 2024
Mach-O's `.subsections_via_symbols` mechanism associates a fragment with an atom (a non-temporary defined symbol). The current approach (`MCFragment::Atom`) wastes space for other object file formats. After llvm#95077, `MCFragment::LayoutOrder` is only used by `AttemptToFoldSymbolOffsetDifference`. While it could be removed, we might explore future uses for `LayoutOrder`. @aengelke suggests one use case: move `Atom` into MCSection. This works because Mach-O doesn't support `.subsection`, and `LayoutOrder`, as the index into the fragment list, is unchanged. This patch moves MCFragment::Atom to MCSectionMachO::Atoms. `getAtom` may be called at parse time before `Atoms` is initialized, so a bound checking is needed to keep the hack working. Pull Request: llvm#95341
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fragments are allocated with
operator newand stored in an ilist withPrev/Next/Parent pointers. A more efficient representation would be an
array of fragments without the overhead of Prev/Next pointers.
As the first step, replace ilist with singly-linked lists.
getPrevNodeuses have been eliminated by previous changes.Prevpointer remains: for each subsection, there is an insertion point andthe current insertion point is stored at
CurInsertionPoint.HexagonAsmBackend::finishLayoutneeds a backward iterator. Save allfragments within
Frags. Hexagon programs are usually small, and theperformance does not matter that much.
To eliminate
Prev, change the subsection representation tosingly-linked lists for subsections and a pointer to the active
singly-linked list. The fragments from all subsections will be chained
together at layout time.
Since fragment lists are disconnected before layout time, we can remove
MCFragment::SubsectionNumber(https://reviews.llvm.org/D69411). Thecurrent implementation of
AttemptToFoldSymbolOffsetDifferencerequiresfuture improvement for robustness.