Skip to content

Commit 9d0754a

Browse files
committed
[MC] Relax fragments eagerly
Lazy relaxation caused hash table lookups (`getFragmentOffset`) and complex use/compute interdependencies. Some expressions involding forward declared symbols (e.g. `subsection-if.s`) cannot be computed. Recursion detection requires complex `IsBeingLaidOut` (https://reviews.llvm.org/D79570). D76114's `invalidateFragmentsFrom` makes lazy relaxation even less useful. Switch to eager relaxation to greatly simplify code and resolve these issues. This change also removes a `getPrevNode` use, which makes it more feasible to replace the fragment representation, which might yield a large peak RSS win. Minor downsides: The number of section relaxations may increase (offset by avoiding the hash table lookup). For relax-recompute-align.s, the computed layout is not optimal.
1 parent cb1a727 commit 9d0754a

11 files changed

+63
-174
lines changed

llvm/include/llvm/MC/MCAsmLayout.h

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -31,37 +31,21 @@ class MCAsmLayout {
3131
/// List of sections in layout order.
3232
llvm::SmallVector<MCSection *, 16> SectionOrder;
3333

34-
/// The last fragment which was laid out, or 0 if nothing has been laid
35-
/// out. Fragments are always laid out in order, so all fragments with a
36-
/// lower ordinal will be valid.
37-
mutable DenseMap<const MCSection *, MCFragment *> LastValidFragment;
38-
39-
/// Make sure that the layout for the given fragment is valid, lazily
40-
/// computing it if necessary.
34+
/// Compute the layout for the section if necessary.
4135
void ensureValid(const MCFragment *F) const;
4236

43-
/// Is the layout for this fragment valid?
44-
bool isFragmentValid(const MCFragment *F) const;
45-
4637
public:
4738
MCAsmLayout(MCAssembler &Assembler);
4839

4940
/// Get the assembler object this is a layout for.
5041
MCAssembler &getAssembler() const { return Assembler; }
5142

52-
/// \returns whether the offset of fragment \p F can be obtained via
53-
/// getFragmentOffset.
54-
bool canGetFragmentOffset(const MCFragment *F) const;
55-
5643
/// Invalidate the fragments starting with F because it has been
5744
/// resized. The fragment's size should have already been updated, but
5845
/// its bundle padding will be recomputed.
5946
void invalidateFragmentsFrom(MCFragment *F);
6047

61-
/// Perform layout for a single fragment, assuming that the previous
62-
/// fragment has already been laid out correctly, and the parent section has
63-
/// been initialized.
64-
void layoutFragment(MCFragment *Fragment);
48+
void layoutBundle(MCFragment *F);
6549

6650
/// \name Section Access (in layout order)
6751
/// @{

llvm/include/llvm/MC/MCAssembler.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -203,10 +203,6 @@ class MCAssembler {
203203
/// were adjusted.
204204
bool layoutOnce(MCAsmLayout &Layout);
205205

206-
/// Perform one layout iteration of the given section and return true
207-
/// if any offsets were adjusted.
208-
bool layoutSectionOnce(MCAsmLayout &Layout, MCSection &Sec);
209-
210206
/// Perform relaxation on a single fragment - returns true if the fragment
211207
/// changes as a result of relaxation.
212208
bool relaxFragment(MCAsmLayout &Layout, MCFragment &F);

llvm/include/llvm/MC/MCFragment.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,6 @@ class MCFragment : public ilist_node_with_parent<MCFragment, MCSection> {
7070

7171
FragmentType Kind;
7272

73-
/// Whether fragment is being laid out.
74-
bool IsBeingLaidOut;
75-
7673
protected:
7774
bool HasInstructions;
7875
bool LinkerRelaxable = false;

llvm/include/llvm/MC/MCSection.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,8 @@ class MCSection {
8989
/// Whether this section has had instructions emitted into it.
9090
bool HasInstructions : 1;
9191

92+
bool HasLayout : 1;
93+
9294
bool IsRegistered : 1;
9395

9496
MCDummyFragment DummyFragment;
@@ -166,6 +168,9 @@ class MCSection {
166168
bool hasInstructions() const { return HasInstructions; }
167169
void setHasInstructions(bool Value) { HasInstructions = Value; }
168170

171+
bool hasLayout() const { return HasLayout; }
172+
void setHasLayout(bool Value) { HasLayout = Value; }
173+
169174
bool isRegistered() const { return IsRegistered; }
170175
void setIsRegistered(bool Value) { IsRegistered = Value; }
171176

llvm/lib/MC/MCAssembler.cpp

Lines changed: 42 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,6 @@ STATISTIC(EmittedFillFragments,
6666
STATISTIC(EmittedNopsFragments, "Number of emitted assembler fragments - nops");
6767
STATISTIC(EmittedOrgFragments, "Number of emitted assembler fragments - org");
6868
STATISTIC(evaluateFixup, "Number of evaluated fixups");
69-
STATISTIC(FragmentLayouts, "Number of fragment layouts");
7069
STATISTIC(ObjectBytes, "Number of emitted object file bytes");
7170
STATISTIC(RelaxationSteps, "Number of assembler layout and relaxation steps");
7271
STATISTIC(RelaxedInstructions, "Number of relaxed instructions");
@@ -404,29 +403,7 @@ uint64_t MCAssembler::computeFragmentSize(const MCAsmLayout &Layout,
404403
llvm_unreachable("invalid fragment kind");
405404
}
406405

407-
void MCAsmLayout::layoutFragment(MCFragment *F) {
408-
MCFragment *Prev = F->getPrevNode();
409-
410-
// We should never try to recompute something which is valid.
411-
assert(!isFragmentValid(F) && "Attempt to recompute a valid fragment!");
412-
// We should never try to compute the fragment layout if its predecessor
413-
// isn't valid.
414-
assert((!Prev || isFragmentValid(Prev)) &&
415-
"Attempt to compute fragment before its predecessor!");
416-
417-
assert(!F->IsBeingLaidOut && "Already being laid out!");
418-
F->IsBeingLaidOut = true;
419-
420-
++stats::FragmentLayouts;
421-
422-
// Compute fragment offset and size.
423-
if (Prev)
424-
F->Offset = Prev->Offset + getAssembler().computeFragmentSize(*this, *Prev);
425-
else
426-
F->Offset = 0;
427-
F->IsBeingLaidOut = false;
428-
LastValidFragment[F->getParent()] = F;
429-
406+
void MCAsmLayout::layoutBundle(MCFragment *F) {
430407
// If bundling is enabled and this fragment has instructions in it, it has to
431408
// obey the bundling restrictions. With padding, we'll have:
432409
//
@@ -454,21 +431,40 @@ void MCAsmLayout::layoutFragment(MCFragment *F) {
454431
// within-fragment padding (which would produce less padding when N is less
455432
// than the bundle size), but for now we don't.
456433
//
457-
if (Assembler.isBundlingEnabled() && F->hasInstructions()) {
458-
assert(isa<MCEncodedFragment>(F) &&
459-
"Only MCEncodedFragment implementations have instructions");
460-
MCEncodedFragment *EF = cast<MCEncodedFragment>(F);
461-
uint64_t FSize = Assembler.computeFragmentSize(*this, *EF);
462-
463-
if (!Assembler.getRelaxAll() && FSize > Assembler.getBundleAlignSize())
464-
report_fatal_error("Fragment can't be larger than a bundle size");
465-
466-
uint64_t RequiredBundlePadding =
467-
computeBundlePadding(Assembler, EF, EF->Offset, FSize);
468-
if (RequiredBundlePadding > UINT8_MAX)
469-
report_fatal_error("Padding cannot exceed 255 bytes");
470-
EF->setBundlePadding(static_cast<uint8_t>(RequiredBundlePadding));
471-
EF->Offset += RequiredBundlePadding;
434+
assert(isa<MCEncodedFragment>(F) &&
435+
"Only MCEncodedFragment implementations have instructions");
436+
MCEncodedFragment *EF = cast<MCEncodedFragment>(F);
437+
uint64_t FSize = Assembler.computeFragmentSize(*this, *EF);
438+
439+
if (!Assembler.getRelaxAll() && FSize > Assembler.getBundleAlignSize())
440+
report_fatal_error("Fragment can't be larger than a bundle size");
441+
442+
uint64_t RequiredBundlePadding =
443+
computeBundlePadding(Assembler, EF, EF->Offset, FSize);
444+
if (RequiredBundlePadding > UINT8_MAX)
445+
report_fatal_error("Padding cannot exceed 255 bytes");
446+
EF->setBundlePadding(static_cast<uint8_t>(RequiredBundlePadding));
447+
EF->Offset += RequiredBundlePadding;
448+
}
449+
450+
uint64_t MCAsmLayout::getFragmentOffset(const MCFragment *F) const {
451+
ensureValid(F);
452+
return F->Offset;
453+
}
454+
455+
void MCAsmLayout::ensureValid(const MCFragment *Frag) const {
456+
MCSection &Sec = *Frag->getParent();
457+
if (Sec.hasLayout())
458+
return;
459+
Sec.setHasLayout(true);
460+
uint64_t Offset = 0;
461+
for (MCFragment &F : Sec) {
462+
F.Offset = Offset;
463+
if (Assembler.isBundlingEnabled() && F.hasInstructions()) {
464+
const_cast<MCAsmLayout *>(this)->layoutBundle(&F);
465+
Offset = F.Offset;
466+
}
467+
Offset += getAssembler().computeFragmentSize(*this, F);
472468
}
473469
}
474470

@@ -848,7 +844,7 @@ void MCAssembler::layout(MCAsmLayout &Layout) {
848844
// another. If any fragment has changed size, we have to re-layout (and
849845
// as a result possibly further relax) all.
850846
for (MCSection &Sec : *this)
851-
Layout.invalidateFragmentsFrom(&*Sec.begin());
847+
Sec.setHasLayout(false);
852848
}
853849

854850
DEBUG_WITH_TYPE("mc-dump", {
@@ -1109,7 +1105,6 @@ bool MCAssembler::relaxBoundaryAlign(MCAsmLayout &Layout,
11091105
if (NewSize == BF.getSize())
11101106
return false;
11111107
BF.setSize(NewSize);
1112-
Layout.invalidateFragmentsFrom(&BF);
11131108
return true;
11141109
}
11151110

@@ -1219,47 +1214,19 @@ bool MCAssembler::relaxFragment(MCAsmLayout &Layout, MCFragment &F) {
12191214
}
12201215
}
12211216

1222-
bool MCAssembler::layoutSectionOnce(MCAsmLayout &Layout, MCSection &Sec) {
1223-
// Holds the first fragment which needed relaxing during this layout. It will
1224-
// remain NULL if none were relaxed.
1225-
// When a fragment is relaxed, all the fragments following it should get
1226-
// invalidated because their offset is going to change.
1227-
MCFragment *FirstRelaxedFragment = nullptr;
1228-
1229-
// Attempt to relax all the fragments in the section.
1230-
for (MCFragment &Frag : Sec) {
1231-
// Check if this is a fragment that needs relaxation.
1232-
bool RelaxedFrag = relaxFragment(Layout, Frag);
1233-
if (RelaxedFrag && !FirstRelaxedFragment)
1234-
FirstRelaxedFragment = &Frag;
1235-
}
1236-
if (FirstRelaxedFragment) {
1237-
Layout.invalidateFragmentsFrom(FirstRelaxedFragment);
1238-
return true;
1239-
}
1240-
return false;
1241-
}
1242-
12431217
bool MCAssembler::layoutOnce(MCAsmLayout &Layout) {
12441218
++stats::RelaxationSteps;
12451219

1246-
bool WasRelaxed = false;
1247-
for (MCSection &Sec : *this) {
1248-
while (layoutSectionOnce(Layout, Sec))
1249-
WasRelaxed = true;
1250-
}
1251-
1252-
return WasRelaxed;
1220+
bool Changed = false;
1221+
for (MCSection &Sec : *this)
1222+
for (MCFragment &Frag : Sec)
1223+
if (relaxFragment(Layout, Frag))
1224+
Changed = true;
1225+
return Changed;
12531226
}
12541227

12551228
void MCAssembler::finishLayout(MCAsmLayout &Layout) {
12561229
assert(getBackendPtr() && "Expected assembler backend");
1257-
// The layout is done. Mark every fragment as valid.
1258-
for (unsigned int i = 0, n = Layout.getSectionOrder().size(); i != n; ++i) {
1259-
MCSection &Section = *Layout.getSectionOrder()[i];
1260-
Layout.getFragmentOffset(&*Section.getFragmentList().rbegin());
1261-
computeFragmentSize(Layout, *Section.getFragmentList().rbegin());
1262-
}
12631230
getBackend().finishLayout(*this, Layout);
12641231
}
12651232

llvm/lib/MC/MCExpr.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -645,10 +645,6 @@ static void AttemptToFoldSymbolOffsetDifference(
645645
Addend += SA.getOffset() - SB.getOffset();
646646
return FinalizeFolding();
647647
}
648-
// One of the symbol involved is part of a fragment being laid out. Quit now
649-
// to avoid a self loop.
650-
if (!Layout->canGetFragmentOffset(FA) || !Layout->canGetFragmentOffset(FB))
651-
return;
652648

653649
// Eagerly evaluate when layout is finalized.
654650
Addend += Layout->getSymbolOffset(A->getSymbol()) -

llvm/lib/MC/MCFragment.cpp

Lines changed: 2 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -39,64 +39,8 @@ MCAsmLayout::MCAsmLayout(MCAssembler &Asm) : Assembler(Asm) {
3939
SectionOrder.push_back(&Sec);
4040
}
4141

42-
bool MCAsmLayout::isFragmentValid(const MCFragment *F) const {
43-
const MCSection *Sec = F->getParent();
44-
const MCFragment *LastValid = LastValidFragment.lookup(Sec);
45-
if (!LastValid)
46-
return false;
47-
assert(LastValid->getParent() == Sec);
48-
return F->getLayoutOrder() <= LastValid->getLayoutOrder();
49-
}
50-
51-
bool MCAsmLayout::canGetFragmentOffset(const MCFragment *F) const {
52-
MCSection *Sec = F->getParent();
53-
MCSection::iterator I;
54-
if (MCFragment *LastValid = LastValidFragment[Sec]) {
55-
// Fragment already valid, offset is available.
56-
if (F->getLayoutOrder() <= LastValid->getLayoutOrder())
57-
return true;
58-
I = ++MCSection::iterator(LastValid);
59-
} else
60-
I = Sec->begin();
61-
62-
// A fragment ordered before F is currently being laid out.
63-
const MCFragment *FirstInvalidFragment = &*I;
64-
if (FirstInvalidFragment->IsBeingLaidOut)
65-
return false;
66-
67-
return true;
68-
}
69-
7042
void MCAsmLayout::invalidateFragmentsFrom(MCFragment *F) {
71-
// If this fragment wasn't already valid, we don't need to do anything.
72-
if (!isFragmentValid(F))
73-
return;
74-
75-
// Otherwise, reset the last valid fragment to the previous fragment
76-
// (if this is the first fragment, it will be NULL).
77-
LastValidFragment[F->getParent()] = F->getPrevNode();
78-
}
79-
80-
void MCAsmLayout::ensureValid(const MCFragment *F) const {
81-
MCSection *Sec = F->getParent();
82-
MCSection::iterator I;
83-
if (MCFragment *Cur = LastValidFragment[Sec])
84-
I = ++MCSection::iterator(Cur);
85-
else
86-
I = Sec->begin();
87-
88-
// Advance the layout position until the fragment is valid.
89-
while (!isFragmentValid(F)) {
90-
assert(I != Sec->end() && "Layout bookkeeping error");
91-
const_cast<MCAsmLayout *>(this)->layoutFragment(&*I);
92-
++I;
93-
}
94-
}
95-
96-
uint64_t MCAsmLayout::getFragmentOffset(const MCFragment *F) const {
97-
ensureValid(F);
98-
assert(F->Offset != ~UINT64_C(0) && "Address not set!");
99-
return F->Offset;
43+
F->getParent()->setHasLayout(false);
10044
}
10145

10246
// Simple getSymbolOffset helper for the non-variable case.
@@ -258,7 +202,7 @@ void ilist_alloc_traits<MCFragment>::deleteNode(MCFragment *V) { V->destroy(); }
258202
MCFragment::MCFragment(FragmentType Kind, bool HasInstructions,
259203
MCSection *Parent)
260204
: Parent(Parent), Atom(nullptr), Offset(~UINT64_C(0)), LayoutOrder(0),
261-
Kind(Kind), IsBeingLaidOut(false), HasInstructions(HasInstructions) {
205+
Kind(Kind), HasInstructions(HasInstructions) {
262206
if (Parent && !isa<MCDummyFragment>(*this))
263207
Parent->addFragment(*this);
264208
}

llvm/lib/MC/MCSection.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ using namespace llvm;
2323
MCSection::MCSection(SectionVariant V, StringRef Name, SectionKind K,
2424
MCSymbol *Begin)
2525
: Begin(Begin), BundleGroupBeforeFirstInst(false), HasInstructions(false),
26-
IsRegistered(false), DummyFragment(this), Name(Name), Variant(V),
27-
Kind(K) {}
26+
HasLayout(false), IsRegistered(false), DummyFragment(this), Name(Name),
27+
Variant(V), Kind(K) {}
2828

2929
MCSymbol *MCSection::getEndSymbol(MCContext &Ctx) {
3030
if (!End)
Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,10 @@
1-
# RUN: not llvm-mc --filetype=obj %s -o /dev/null 2>&1 | FileCheck %s
2-
# REQUIRES: object-emission
3-
# UNSUPPORTED: target={{.*}}-zos{{.*}}
1+
# RUN: not llvm-mc -filetype=obj -triple=x86_64 %s -o /dev/null 2>&1 | FileCheck %s
42

53
fct_end:
64

7-
# CHECK: layout-interdependency.s:[[#@LINE+1]]:7: error: expected assembly-time absolute expression
5+
# CHECK: layout-interdependency.s:[[#@LINE+1]]:7: error: invalid number of bytes
86
.fill (data_start - fct_end), 1, 42
9-
# CHECK: layout-interdependency.s:[[#@LINE+1]]:7: error: expected assembly-time absolute expression
7+
# CHECK: layout-interdependency.s:[[#@LINE+1]]:7: error: invalid number of bytes
108
.fill (fct_end - data_start), 1, 42
119

1210
data_start:

llvm/test/MC/ELF/relax-recompute-align.s

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
// RUN: llvm-mc -filetype=obj -triple i386 %s -o - | llvm-objdump -d --no-show-raw-insn - | FileCheck %s
22

3-
// This is a case where llvm-mc computes a better layout than Darwin 'as'. This
3+
/// This is a case where the computed layout is not optimal. The
44
// issue is that after the first jmp slides, the .align size must be
55
// recomputed -- otherwise the second jump will appear to be out-of-range for a
66
// 1-byte jump.
77

88
// CHECK: int3
9-
// CHECK-NEXT: ce: int3
10-
// CHECK: d0: pushal
11-
// CHECK: 130: jl 0xd0
9+
// CHECK-NEXT: d2: int3
10+
// CHECK: e0: pushal
11+
// CHECK: 140: jl 0xe0
1212

1313
L0:
1414
.space 0x8a, 0x90

llvm/test/MC/ELF/subsection-if.s

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
# RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o %t
22
# RUN: llvm-readelf -x .text %t | FileCheck %s
3-
# RUN: not llvm-mc -filetype=obj -triple=x86_64 --defsym ERR=1 %s -o /dev/null 2>&1 | FileCheck %s --check-prefix=ERR
3+
# RUN: llvm-mc -filetype=obj -triple=x86_64 --defsym ERR=1 %s -o %t1
4+
# RUN: llvm-readelf -x .text %t1 | FileCheck %s --check-prefix=CHECK1
45

56
# CHECK: 0x00000000 9090
7+
# CHECK1: 0x00000000 90909090 90
68

79
.subsection 1
810
661:

0 commit comments

Comments
 (0)