Skip to content

Commit 0393084

Browse files
authored
MC: Store MCRelaxableFragment MCInst out-of-line
Follow-up to #146307 Moved MCInst storage to MCSection, enabling trivial ~MCRelaxableFragment and eliminating the need for a fragment walk in ~MCSection. Updated MCRelaxableFragment::getInst to construct an MCInst on demand. Modified MCAssembler::relaxInstruction's mayNeedRelaxation to accept opcode and operands instead of an MCInst, avoiding redundant MCInst creation. Note that MCObjectStreamer::emitInstructionImpl calls mayNeedRelaxation before determining the target fragment for the MCInst. Unfortunately, we also have to encode `MCInst::Flags` to support the EVEX prefix, e.g. `{evex} xorw $foo, %ax` There is a small decrease in max-rss (stage1-ReleaseLTO-g (link only)) with negligible instructions:u change. https://llvm-compile-time-tracker.com/compare.php?from=0b533f2d9f0551aaffb13dcac8e0fd0a952185b5&to=f26b57f33bc7ccae749a57dfc841de7ce2acc2ef&stat=max-rss&linkStats=on Next: Enable MCFragment to store fixed-size data (was MCDataFragment's job) and optional Opcode/Operands data (was MCRelaxableFragment's job), and delete MCDataFragment/MCRelaxableFragment. This will allow re-encoding of Data+Relax+Data+Relax sequences as Frag+Frag. The saving should outweigh the downside of larger MCFragment. Pull Request: #147229
1 parent de732df commit 0393084

File tree

18 files changed

+99
-89
lines changed

18 files changed

+99
-89
lines changed

llvm/include/llvm/MC/MCAsmBackend.h

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ class MCInst;
3232
class MCObjectStreamer;
3333
class MCObjectTargetWriter;
3434
class MCObjectWriter;
35+
class MCOperand;
3536
class MCSubtargetInfo;
3637
class MCValue;
3738
class raw_pwrite_stream;
@@ -147,12 +148,9 @@ class LLVM_ABI MCAsmBackend {
147148
/// \name Target Relaxation Interfaces
148149
/// @{
149150

150-
/// Check whether the given instruction may need relaxation.
151-
///
152-
/// \param Inst - The instruction to test.
153-
/// \param STI - The MCSubtargetInfo in effect when the instruction was
154-
/// encoded.
155-
virtual bool mayNeedRelaxation(const MCInst &Inst,
151+
/// Check whether the given instruction (encoded as Opcode+Operands) may need
152+
/// relaxation.
153+
virtual bool mayNeedRelaxation(unsigned Opcode, ArrayRef<MCOperand> Operands,
156154
const MCSubtargetInfo &STI) const {
157155
return false;
158156
}

llvm/include/llvm/MC/MCInst.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#ifndef LLVM_MC_MCINST_H
1616
#define LLVM_MC_MCINST_H
1717

18+
#include "llvm/ADT/ArrayRef.h"
1819
#include "llvm/ADT/SmallVector.h"
1920
#include "llvm/ADT/StringRef.h"
2021
#include "llvm/ADT/bit.h"
@@ -210,7 +211,11 @@ class MCInst {
210211
MCOperand &getOperand(unsigned i) { return Operands[i]; }
211212
unsigned getNumOperands() const { return Operands.size(); }
212213

214+
ArrayRef<MCOperand> getOperands() const { return Operands; }
213215
void addOperand(const MCOperand Op) { Operands.push_back(Op); }
216+
void setOperands(ArrayRef<MCOperand> Ops) {
217+
Operands.assign(Ops.begin(), Ops.end());
218+
}
214219

215220
using iterator = SmallVectorImpl<MCOperand>::iterator;
216221
using const_iterator = SmallVectorImpl<MCOperand>::const_iterator;

llvm/include/llvm/MC/MCSection.h

Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@ class raw_ostream;
3939
class Triple;
4040

4141
// Represents a contiguous piece of code or data within a section. Its size is
42-
// determined by MCAssembler::layout. All subclasses (except
43-
// MCRelaxableFragment, which stores a MCInst) must have trivial destructors.
42+
// determined by MCAssembler::layout. All subclasses must have trivial
43+
// destructors.
4444
//
4545
// Declaration order: MCFragment, MCSection, then MCFragment's derived classes.
4646
// This allows MCSection's inline functions to access MCFragment members and
@@ -101,12 +101,6 @@ class MCFragment {
101101
MCFragment(const MCFragment &) = delete;
102102
MCFragment &operator=(const MCFragment &) = delete;
103103

104-
/// Destroys the current fragment.
105-
///
106-
/// This must be used instead of delete as MCFragment is non-virtual.
107-
/// This method will dispatch to the appropriate subclass.
108-
LLVM_ABI void destroy();
109-
110104
MCFragment *getNext() const { return Next; }
111105

112106
FragmentType getKind() const { return Kind; }
@@ -133,6 +127,7 @@ class LLVM_ABI MCSection {
133127
friend MCAssembler;
134128
friend MCObjectStreamer;
135129
friend class MCEncodedFragment;
130+
friend class MCRelaxableFragment;
136131
static constexpr unsigned NonUniqueID = ~0U;
137132

138133
enum SectionVariant {
@@ -213,6 +208,7 @@ class LLVM_ABI MCSection {
213208
// Content and fixup storage for fragments
214209
SmallVector<char, 0> ContentStorage;
215210
SmallVector<MCFixup, 0> FixupStorage;
211+
SmallVector<MCOperand, 0> MCOperandStorage;
216212

217213
protected:
218214
// TODO Make Name private when possible.
@@ -221,7 +217,8 @@ class LLVM_ABI MCSection {
221217

222218
MCSection(SectionVariant V, StringRef Name, bool IsText, bool IsVirtual,
223219
MCSymbol *Begin);
224-
~MCSection();
220+
// Protected non-virtual dtor prevents destroy through a base class pointer.
221+
~MCSection() {}
225222

226223
public:
227224
MCSection(const MCSection &) = delete;
@@ -430,17 +427,41 @@ class MCDataFragment : public MCEncodedFragment {
430427
/// relaxed during the assembler layout and relaxation stage.
431428
///
432429
class MCRelaxableFragment : public MCEncodedFragment {
433-
/// The instruction this is a fragment for.
434-
MCInst Inst;
430+
uint32_t Opcode = 0;
431+
uint32_t Flags = 0;
432+
uint32_t OperandStart = 0;
433+
uint32_t OperandSize = 0;
435434

436435
public:
437-
MCRelaxableFragment(const MCInst &Inst, const MCSubtargetInfo &STI)
438-
: MCEncodedFragment(FT_Relaxable, true), Inst(Inst) {
436+
MCRelaxableFragment(const MCSubtargetInfo &STI)
437+
: MCEncodedFragment(FT_Relaxable, true) {
439438
this->STI = &STI;
440439
}
441440

442-
const MCInst &getInst() const { return Inst; }
443-
void setInst(const MCInst &Value) { Inst = Value; }
441+
unsigned getOpcode() const { return Opcode; }
442+
ArrayRef<MCOperand> getOperands() const {
443+
return MutableArrayRef(getParent()->MCOperandStorage)
444+
.slice(OperandStart, OperandSize);
445+
}
446+
MCInst getInst() const {
447+
MCInst Inst;
448+
Inst.setOpcode(Opcode);
449+
Inst.setFlags(Flags);
450+
Inst.setOperands(ArrayRef(getParent()->MCOperandStorage)
451+
.slice(OperandStart, OperandSize));
452+
return Inst;
453+
}
454+
void setInst(const MCInst &Inst) {
455+
Opcode = Inst.getOpcode();
456+
Flags = Inst.getFlags();
457+
auto &S = getParent()->MCOperandStorage;
458+
if (Inst.getNumOperands() > OperandSize) {
459+
OperandStart = S.size();
460+
S.resize_for_overwrite(S.size() + Inst.getNumOperands());
461+
}
462+
OperandSize = Inst.getNumOperands();
463+
llvm::copy(Inst, S.begin() + OperandStart);
464+
}
444465

445466
bool getAllowAutoPadding() const { return AllowAutoPadding; }
446467
void setAllowAutoPadding(bool V) { AllowAutoPadding = V; }

llvm/lib/MC/MCAssembler.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -869,7 +869,8 @@ bool MCAssembler::relaxInstruction(MCRelaxableFragment &F) {
869869
// If this inst doesn't ever need relaxation, ignore it. This occurs when we
870870
// are intentionally pushing out inst fragments, or because we relaxed a
871871
// previous instruction to one that doesn't need relaxation.
872-
if (!getBackend().mayNeedRelaxation(F.getInst(), *F.getSubtargetInfo()))
872+
if (!getBackend().mayNeedRelaxation(F.getOpcode(), F.getOperands(),
873+
*F.getSubtargetInfo()))
873874
return false;
874875

875876
bool DoRelax = false;
@@ -881,6 +882,8 @@ bool MCAssembler::relaxInstruction(MCRelaxableFragment &F) {
881882

882883
++stats::RelaxedInstructions;
883884

885+
// TODO Refactor relaxInstruction to accept MCRelaxableFragment and remove
886+
// `setInst`.
884887
MCInst Relaxed = F.getInst();
885888
getBackend().relaxInstruction(Relaxed, *F.getSubtargetInfo());
886889

llvm/lib/MC/MCObjectStreamer.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,7 @@ void MCObjectStreamer::emitInstructionImpl(const MCInst &Inst,
352352
// If this instruction doesn't need relaxation, just emit it as data.
353353
MCAssembler &Assembler = getAssembler();
354354
MCAsmBackend &Backend = Assembler.getBackend();
355-
if (!(Backend.mayNeedRelaxation(Inst, STI) ||
355+
if (!(Backend.mayNeedRelaxation(Inst.getOpcode(), Inst.getOperands(), STI) ||
356356
Backend.allowEnhancedRelaxation())) {
357357
emitInstToData(Inst, STI);
358358
return;
@@ -366,7 +366,8 @@ void MCObjectStreamer::emitInstructionImpl(const MCInst &Inst,
366366
if (Assembler.getRelaxAll() ||
367367
(Assembler.isBundlingEnabled() && Sec->isBundleLocked())) {
368368
MCInst Relaxed = Inst;
369-
while (Backend.mayNeedRelaxation(Relaxed, STI))
369+
while (Backend.mayNeedRelaxation(Relaxed.getOpcode(), Relaxed.getOperands(),
370+
STI))
370371
Backend.relaxInstruction(Relaxed, STI);
371372
emitInstToData(Relaxed, STI);
372373
return;
@@ -397,8 +398,9 @@ void MCObjectStreamer::emitInstToFragment(const MCInst &Inst,
397398
// Always create a new, separate fragment here, because its size can change
398399
// during relaxation.
399400
MCRelaxableFragment *IF =
400-
getContext().allocFragment<MCRelaxableFragment>(Inst, STI);
401+
getContext().allocFragment<MCRelaxableFragment>(STI);
401402
insert(IF);
403+
IF->setInst(Inst);
402404

403405
SmallVector<MCFixup, 1> Fixups;
404406
getAssembler().getEmitter().encodeInstruction(

llvm/lib/MC/MCSection.cpp

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -36,18 +36,6 @@ MCSymbol *MCSection::getEndSymbol(MCContext &Ctx) {
3636

3737
bool MCSection::hasEnded() const { return End && End->isInSection(); }
3838

39-
MCSection::~MCSection() {
40-
// If ~MCRelaxableFragment becomes trivial (no longer store a MCInst member),
41-
// this dtor can be made empty.
42-
for (auto &[_, Chain] : Subsections) {
43-
for (MCFragment *X = Chain.Head, *Y; X; X = Y) {
44-
Y = X->Next;
45-
if (auto *F = dyn_cast<MCRelaxableFragment>(X))
46-
F->~MCRelaxableFragment();
47-
}
48-
}
49-
}
50-
5139
void MCSection::setBundleLockState(BundleLockStateType NewState) {
5240
if (NewState == NotBundleLocked) {
5341
if (BundleLockNestingDepth == 0) {

llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUAsmBackend.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ class AMDGPUAsmBackend : public MCAsmBackend {
4040
void relaxInstruction(MCInst &Inst,
4141
const MCSubtargetInfo &STI) const override;
4242

43-
bool mayNeedRelaxation(const MCInst &Inst,
43+
bool mayNeedRelaxation(unsigned Opcode, ArrayRef<MCOperand> Operands,
4444
const MCSubtargetInfo &STI) const override;
4545

4646
unsigned getMinimumNopSize() const override;
@@ -70,12 +70,13 @@ bool AMDGPUAsmBackend::fixupNeedsRelaxation(const MCFixup &Fixup,
7070
return (((int64_t(Value)/4)-1) == 0x3f);
7171
}
7272

73-
bool AMDGPUAsmBackend::mayNeedRelaxation(const MCInst &Inst,
74-
const MCSubtargetInfo &STI) const {
73+
bool AMDGPUAsmBackend::mayNeedRelaxation(unsigned Opcode,
74+
ArrayRef<MCOperand> Operands,
75+
const MCSubtargetInfo &STI) const {
7576
if (!STI.hasFeature(AMDGPU::FeatureOffset3fBug))
7677
return false;
7778

78-
if (AMDGPU::getSOPPWithRelaxation(Inst.getOpcode()) >= 0)
79+
if (AMDGPU::getSOPPWithRelaxation(Opcode) >= 0)
7980
return true;
8081

8182
return false;

llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -201,11 +201,9 @@ unsigned ARMAsmBackend::getRelaxedOpcode(unsigned Op,
201201
}
202202
}
203203

204-
bool ARMAsmBackend::mayNeedRelaxation(const MCInst &Inst,
204+
bool ARMAsmBackend::mayNeedRelaxation(unsigned Opcode, ArrayRef<MCOperand>,
205205
const MCSubtargetInfo &STI) const {
206-
if (getRelaxedOpcode(Inst.getOpcode(), STI) != Inst.getOpcode())
207-
return true;
208-
return false;
206+
return getRelaxedOpcode(Opcode, STI) != Opcode;
209207
}
210208

211209
static const char *checkPCRelOffset(uint64_t Value, int64_t Min, int64_t Max) {

llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ class ARMAsmBackend : public MCAsmBackend {
4545

4646
unsigned getRelaxedOpcode(unsigned Op, const MCSubtargetInfo &STI) const;
4747

48-
bool mayNeedRelaxation(const MCInst &Inst,
48+
bool mayNeedRelaxation(unsigned Opcode, ArrayRef<MCOperand> Operands,
4949
const MCSubtargetInfo &STI) const override;
5050

5151
const char *reasonForFixupRelaxation(const MCFixup &Fixup,

llvm/lib/Target/CSKY/MCTargetDesc/CSKYAsmBackend.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -239,9 +239,9 @@ void CSKYAsmBackend::applyFixup(const MCFragment &F, const MCFixup &Fixup,
239239
}
240240
}
241241

242-
bool CSKYAsmBackend::mayNeedRelaxation(const MCInst &Inst,
242+
bool CSKYAsmBackend::mayNeedRelaxation(unsigned Opcode, ArrayRef<MCOperand>,
243243
const MCSubtargetInfo &STI) const {
244-
switch (Inst.getOpcode()) {
244+
switch (Opcode) {
245245
default:
246246
return false;
247247
case CSKY::JBR32:

0 commit comments

Comments
 (0)