MC: Store MCRelaxableFragment MCInst out-of-line #147229
Open
+99
−89
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:ARM
backend:AMDGPU
backend:Hexagon
backend:RISC-V
backend:X86
mc
|
@llvm/pr-subscribers-mc @llvm/pr-subscribers-backend-arm Author: Fangrui Song (MaskRay) ChangesFollow-up to #146307 Moved MCInst storage to MCSection, enabling trivial ~MCRelaxableFragment Updated MCRelaxableFragment::getInst to construct an MCInst on demand. There is a small decrease in max-rss (stage1-ReleaseLTO-g (link only)) Next: Enable MCDataFragment to store optional Opcode/Operands data (when Patch is 25.05 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/147229.diff 18 Files Affected:
diff --git a/llvm/include/llvm/MC/MCAsmBackend.h b/llvm/include/llvm/MC/MCAsmBackend.h
index ebc98ebfce9f7..6b81bdba25e67 100644
--- a/llvm/include/llvm/MC/MCAsmBackend.h
+++ b/llvm/include/llvm/MC/MCAsmBackend.h
@@ -32,6 +32,7 @@ class MCInst;
class MCObjectStreamer;
class MCObjectTargetWriter;
class MCObjectWriter;
+class MCOperand;
class MCSubtargetInfo;
class MCValue;
class raw_pwrite_stream;
@@ -147,12 +148,9 @@ class LLVM_ABI MCAsmBackend {
/// \name Target Relaxation Interfaces
/// @{
- /// Check whether the given instruction may need relaxation.
- ///
- /// \param Inst - The instruction to test.
- /// \param STI - The MCSubtargetInfo in effect when the instruction was
- /// encoded.
- virtual bool mayNeedRelaxation(const MCInst &Inst,
+ /// Check whether the given instruction (encoded as Opcode+Operands) may need
+ /// relaxation.
+ virtual bool mayNeedRelaxation(unsigned Opcode, ArrayRef<MCOperand> Operands,
const MCSubtargetInfo &STI) const {
return false;
}
diff --git a/llvm/include/llvm/MC/MCInst.h b/llvm/include/llvm/MC/MCInst.h
index f26af88d92140..b0db6b8408da5 100644
--- a/llvm/include/llvm/MC/MCInst.h
+++ b/llvm/include/llvm/MC/MCInst.h
@@ -15,6 +15,7 @@
#ifndef LLVM_MC_MCINST_H
#define LLVM_MC_MCINST_H
+#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/bit.h"
@@ -210,7 +211,11 @@ class MCInst {
MCOperand &getOperand(unsigned i) { return Operands[i]; }
unsigned getNumOperands() const { return Operands.size(); }
+ ArrayRef<MCOperand> getOperands() const { return Operands; }
void addOperand(const MCOperand Op) { Operands.push_back(Op); }
+ void setOperands(ArrayRef<MCOperand> Ops) {
+ Operands.assign(Ops.begin(), Ops.end());
+ }
using iterator = SmallVectorImpl<MCOperand>::iterator;
using const_iterator = SmallVectorImpl<MCOperand>::const_iterator;
diff --git a/llvm/include/llvm/MC/MCSection.h b/llvm/include/llvm/MC/MCSection.h
index 3abe5e0fe3c6f..f6f62d017dd6a 100644
--- a/llvm/include/llvm/MC/MCSection.h
+++ b/llvm/include/llvm/MC/MCSection.h
@@ -133,6 +133,7 @@ class LLVM_ABI MCSection {
friend MCAssembler;
friend MCObjectStreamer;
friend class MCEncodedFragment;
+ friend class MCRelaxableFragment;
static constexpr unsigned NonUniqueID = ~0U;
enum SectionVariant {
@@ -213,6 +214,7 @@ class LLVM_ABI MCSection {
// Content and fixup storage for fragments
SmallVector<char, 0> ContentStorage;
SmallVector<MCFixup, 0> FixupStorage;
+ SmallVector<MCOperand, 0> MCOperandStorage;
protected:
// TODO Make Name private when possible.
@@ -221,7 +223,8 @@ class LLVM_ABI MCSection {
MCSection(SectionVariant V, StringRef Name, bool IsText, bool IsVirtual,
MCSymbol *Begin);
- ~MCSection();
+ // Protected non-virtual dtor prevents destroy through a base class pointer.
+ ~MCSection() {}
public:
MCSection(const MCSection &) = delete;
@@ -431,16 +434,38 @@ class MCDataFragment : public MCEncodedFragment {
///
class MCRelaxableFragment : public MCEncodedFragment {
/// The instruction this is a fragment for.
- MCInst Inst;
+ unsigned Opcode = 0;
+ uint32_t OperandStart = 0;
+ uint32_t OperandSize = 0;
public:
- MCRelaxableFragment(const MCInst &Inst, const MCSubtargetInfo &STI)
- : MCEncodedFragment(FT_Relaxable, true), Inst(Inst) {
+ MCRelaxableFragment(const MCSubtargetInfo &STI)
+ : MCEncodedFragment(FT_Relaxable, true) {
this->STI = &STI;
}
- const MCInst &getInst() const { return Inst; }
- void setInst(const MCInst &Value) { Inst = Value; }
+ unsigned getOpcode() const { return Opcode; }
+ ArrayRef<MCOperand> getOperands() const {
+ return MutableArrayRef(getParent()->MCOperandStorage)
+ .slice(OperandStart, OperandSize);
+ }
+ MCInst getInst() const {
+ MCInst Inst;
+ Inst.setOpcode(Opcode);
+ Inst.setOperands(ArrayRef(getParent()->MCOperandStorage)
+ .slice(OperandStart, OperandSize));
+ return Inst;
+ }
+ void setInst(const MCInst &Inst) {
+ Opcode = Inst.getOpcode();
+ auto &S = getParent()->MCOperandStorage;
+ if (Inst.getNumOperands() > OperandSize) {
+ OperandStart = S.size();
+ S.resize_for_overwrite(S.size() + Inst.getNumOperands());
+ }
+ OperandSize = Inst.getNumOperands();
+ llvm::copy(Inst, S.begin() + OperandStart);
+ }
bool getAllowAutoPadding() const { return AllowAutoPadding; }
void setAllowAutoPadding(bool V) { AllowAutoPadding = V; }
diff --git a/llvm/lib/MC/MCAssembler.cpp b/llvm/lib/MC/MCAssembler.cpp
index 496d66b1876b2..480e6fe027beb 100644
--- a/llvm/lib/MC/MCAssembler.cpp
+++ b/llvm/lib/MC/MCAssembler.cpp
@@ -869,7 +869,8 @@ bool MCAssembler::relaxInstruction(MCRelaxableFragment &F) {
// If this inst doesn't ever need relaxation, ignore it. This occurs when we
// are intentionally pushing out inst fragments, or because we relaxed a
// previous instruction to one that doesn't need relaxation.
- if (!getBackend().mayNeedRelaxation(F.getInst(), *F.getSubtargetInfo()))
+ if (!getBackend().mayNeedRelaxation(F.getOpcode(), F.getOperands(),
+ *F.getSubtargetInfo()))
return false;
bool DoRelax = false;
@@ -881,6 +882,8 @@ bool MCAssembler::relaxInstruction(MCRelaxableFragment &F) {
++stats::RelaxedInstructions;
+ // TODO Refactor relaxInstruction to accept MCRelaxableFragment and remove
+ // `setInst`.
MCInst Relaxed = F.getInst();
getBackend().relaxInstruction(Relaxed, *F.getSubtargetInfo());
diff --git a/llvm/lib/MC/MCObjectStreamer.cpp b/llvm/lib/MC/MCObjectStreamer.cpp
index bd1a77467679b..5cc9bed2669d4 100644
--- a/llvm/lib/MC/MCObjectStreamer.cpp
+++ b/llvm/lib/MC/MCObjectStreamer.cpp
@@ -352,7 +352,7 @@ void MCObjectStreamer::emitInstructionImpl(const MCInst &Inst,
// If this instruction doesn't need relaxation, just emit it as data.
MCAssembler &Assembler = getAssembler();
MCAsmBackend &Backend = Assembler.getBackend();
- if (!(Backend.mayNeedRelaxation(Inst, STI) ||
+ if (!(Backend.mayNeedRelaxation(Inst.getOpcode(), Inst.getOperands(), STI) ||
Backend.allowEnhancedRelaxation())) {
emitInstToData(Inst, STI);
return;
@@ -366,7 +366,8 @@ void MCObjectStreamer::emitInstructionImpl(const MCInst &Inst,
if (Assembler.getRelaxAll() ||
(Assembler.isBundlingEnabled() && Sec->isBundleLocked())) {
MCInst Relaxed = Inst;
- while (Backend.mayNeedRelaxation(Relaxed, STI))
+ while (Backend.mayNeedRelaxation(Relaxed.getOpcode(), Relaxed.getOperands(),
+ STI))
Backend.relaxInstruction(Relaxed, STI);
emitInstToData(Relaxed, STI);
return;
@@ -397,8 +398,9 @@ void MCObjectStreamer::emitInstToFragment(const MCInst &Inst,
// Always create a new, separate fragment here, because its size can change
// during relaxation.
MCRelaxableFragment *IF =
- getContext().allocFragment<MCRelaxableFragment>(Inst, STI);
+ getContext().allocFragment<MCRelaxableFragment>(STI);
insert(IF);
+ IF->setInst(Inst);
SmallVector<MCFixup, 1> Fixups;
getAssembler().getEmitter().encodeInstruction(
diff --git a/llvm/lib/MC/MCSection.cpp b/llvm/lib/MC/MCSection.cpp
index 09d09e8f997e6..56450033529bc 100644
--- a/llvm/lib/MC/MCSection.cpp
+++ b/llvm/lib/MC/MCSection.cpp
@@ -36,18 +36,6 @@ MCSymbol *MCSection::getEndSymbol(MCContext &Ctx) {
bool MCSection::hasEnded() const { return End && End->isInSection(); }
-MCSection::~MCSection() {
- // If ~MCRelaxableFragment becomes trivial (no longer store a MCInst member),
- // this dtor can be made empty.
- for (auto &[_, Chain] : Subsections) {
- for (MCFragment *X = Chain.Head, *Y; X; X = Y) {
- Y = X->Next;
- if (auto *F = dyn_cast<MCRelaxableFragment>(X))
- F->~MCRelaxableFragment();
- }
- }
-}
-
void MCSection::setBundleLockState(BundleLockStateType NewState) {
if (NewState == NotBundleLocked) {
if (BundleLockNestingDepth == 0) {
diff --git a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUAsmBackend.cpp b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUAsmBackend.cpp
index b333c97ecc378..8f89168754180 100644
--- a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUAsmBackend.cpp
+++ b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUAsmBackend.cpp
@@ -40,7 +40,7 @@ class AMDGPUAsmBackend : public MCAsmBackend {
void relaxInstruction(MCInst &Inst,
const MCSubtargetInfo &STI) const override;
- bool mayNeedRelaxation(const MCInst &Inst,
+ bool mayNeedRelaxation(unsigned Opcode, ArrayRef<MCOperand> Operands,
const MCSubtargetInfo &STI) const override;
unsigned getMinimumNopSize() const override;
@@ -70,12 +70,13 @@ bool AMDGPUAsmBackend::fixupNeedsRelaxation(const MCFixup &Fixup,
return (((int64_t(Value)/4)-1) == 0x3f);
}
-bool AMDGPUAsmBackend::mayNeedRelaxation(const MCInst &Inst,
- const MCSubtargetInfo &STI) const {
+bool AMDGPUAsmBackend::mayNeedRelaxation(unsigned Opcode,
+ ArrayRef<MCOperand> Operands,
+ const MCSubtargetInfo &STI) const {
if (!STI.hasFeature(AMDGPU::FeatureOffset3fBug))
return false;
- if (AMDGPU::getSOPPWithRelaxation(Inst.getOpcode()) >= 0)
+ if (AMDGPU::getSOPPWithRelaxation(Opcode) >= 0)
return true;
return false;
diff --git a/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp b/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
index fc9a32072a627..376bddb120d5f 100644
--- a/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
+++ b/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
@@ -201,11 +201,9 @@ unsigned ARMAsmBackend::getRelaxedOpcode(unsigned Op,
}
}
-bool ARMAsmBackend::mayNeedRelaxation(const MCInst &Inst,
+bool ARMAsmBackend::mayNeedRelaxation(unsigned Opcode, ArrayRef<MCOperand>,
const MCSubtargetInfo &STI) const {
- if (getRelaxedOpcode(Inst.getOpcode(), STI) != Inst.getOpcode())
- return true;
- return false;
+ return getRelaxedOpcode(Opcode, STI) != Opcode;
}
static const char *checkPCRelOffset(uint64_t Value, int64_t Min, int64_t Max) {
diff --git a/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.h b/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.h
index 52279a3b6936f..877e3afdb1d57 100644
--- a/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.h
+++ b/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.h
@@ -45,7 +45,7 @@ class ARMAsmBackend : public MCAsmBackend {
unsigned getRelaxedOpcode(unsigned Op, const MCSubtargetInfo &STI) const;
- bool mayNeedRelaxation(const MCInst &Inst,
+ bool mayNeedRelaxation(unsigned Opcode, ArrayRef<MCOperand> Operands,
const MCSubtargetInfo &STI) const override;
const char *reasonForFixupRelaxation(const MCFixup &Fixup,
diff --git a/llvm/lib/Target/CSKY/MCTargetDesc/CSKYAsmBackend.cpp b/llvm/lib/Target/CSKY/MCTargetDesc/CSKYAsmBackend.cpp
index 0922d037149e5..ce1da6e58b9cd 100644
--- a/llvm/lib/Target/CSKY/MCTargetDesc/CSKYAsmBackend.cpp
+++ b/llvm/lib/Target/CSKY/MCTargetDesc/CSKYAsmBackend.cpp
@@ -239,9 +239,9 @@ void CSKYAsmBackend::applyFixup(const MCFragment &F, const MCFixup &Fixup,
}
}
-bool CSKYAsmBackend::mayNeedRelaxation(const MCInst &Inst,
+bool CSKYAsmBackend::mayNeedRelaxation(unsigned Opcode, ArrayRef<MCOperand>,
const MCSubtargetInfo &STI) const {
- switch (Inst.getOpcode()) {
+ switch (Opcode) {
default:
return false;
case CSKY::JBR32:
diff --git a/llvm/lib/Target/CSKY/MCTargetDesc/CSKYAsmBackend.h b/llvm/lib/Target/CSKY/MCTargetDesc/CSKYAsmBackend.h
index 142c667a6175e..1d3a22c2bbbb4 100644
--- a/llvm/lib/Target/CSKY/MCTargetDesc/CSKYAsmBackend.h
+++ b/llvm/lib/Target/CSKY/MCTargetDesc/CSKYAsmBackend.h
@@ -33,12 +33,11 @@ class CSKYAsmBackend : public MCAsmBackend {
bool fixupNeedsRelaxation(const MCFixup &Fixup,
uint64_t Value) const override;
+ bool mayNeedRelaxation(unsigned Opcode, ArrayRef<MCOperand> Operands,
+ const MCSubtargetInfo &STI) const override;
void relaxInstruction(MCInst &Inst,
const MCSubtargetInfo &STI) const override;
- bool mayNeedRelaxation(const MCInst &Inst,
- const MCSubtargetInfo &STI) const override;
-
bool fixupNeedsRelaxationAdvanced(const MCFixup &, const MCValue &, uint64_t,
bool) const override;
diff --git a/llvm/lib/Target/Hexagon/MCTargetDesc/HexagonAsmBackend.cpp b/llvm/lib/Target/Hexagon/MCTargetDesc/HexagonAsmBackend.cpp
index 847895f84ca9d..de7bd5d4b2c66 100644
--- a/llvm/lib/Target/Hexagon/MCTargetDesc/HexagonAsmBackend.cpp
+++ b/llvm/lib/Target/Hexagon/MCTargetDesc/HexagonAsmBackend.cpp
@@ -40,7 +40,7 @@ class HexagonAsmBackend : public MCAsmBackend {
uint8_t OSABI;
StringRef CPU;
mutable uint64_t relaxedCnt;
- mutable const MCInst *RelaxedMCB = nullptr;
+ mutable MCInst RelaxedMCB;
std::unique_ptr <MCInstrInfo> MCII;
std::unique_ptr <MCInst *> RelaxTarget;
MCInst * Extender;
@@ -428,13 +428,11 @@ class HexagonAsmBackend : public MCAsmBackend {
return Relaxable;
}
- /// MayNeedRelaxation - Check whether the given instruction may need
- /// relaxation.
- ///
- /// \param Inst - The instruction to test.
- bool mayNeedRelaxation(MCInst const &Inst,
+ bool mayNeedRelaxation(unsigned Opcode, ArrayRef<MCOperand> Operands,
const MCSubtargetInfo &STI) const override {
- RelaxedMCB = &Inst;
+ RelaxedMCB.clear();
+ RelaxedMCB.setOpcode(Opcode);
+ RelaxedMCB.setOperands(Operands);
return true;
}
@@ -443,7 +441,7 @@ class HexagonAsmBackend : public MCAsmBackend {
bool fixupNeedsRelaxationAdvanced(const MCFixup &Fixup, const MCValue &,
uint64_t Value,
bool Resolved) const override {
- MCInst const &MCB = *RelaxedMCB;
+ MCInst const &MCB = RelaxedMCB;
assert(HexagonMCInstrInfo::isBundle(MCB));
*RelaxTarget = nullptr;
@@ -598,7 +596,7 @@ class HexagonAsmBackend : public MCAsmBackend {
case MCFragment::FT_Relaxable: {
MCContext &Context = getContext();
auto &RF = cast<MCRelaxableFragment>(*Frags[K]);
- auto &Inst = const_cast<MCInst &>(RF.getInst());
+ MCInst Inst = RF.getInst();
const bool WouldTraverseLabel = llvm::any_of(
Asm.symbols(), [&Asm, &RF, &Inst](MCSymbol const &sym) {
diff --git a/llvm/lib/Target/M68k/MCTargetDesc/M68kAsmBackend.cpp b/llvm/lib/Target/M68k/MCTargetDesc/M68kAsmBackend.cpp
index f0c3728ec0eea..5e039033704f9 100644
--- a/llvm/lib/Target/M68k/MCTargetDesc/M68kAsmBackend.cpp
+++ b/llvm/lib/Target/M68k/MCTargetDesc/M68kAsmBackend.cpp
@@ -56,7 +56,7 @@ class M68kAsmBackend : public MCAsmBackend {
MutableArrayRef<char> Data, uint64_t Value,
bool IsResolved) override;
- bool mayNeedRelaxation(const MCInst &Inst,
+ bool mayNeedRelaxation(unsigned Opcode, ArrayRef<MCOperand> Operands,
const MCSubtargetInfo &STI) const override;
bool fixupNeedsRelaxation(const MCFixup &Fixup,
@@ -183,10 +183,10 @@ static unsigned getRelaxedOpcode(unsigned Opcode) {
return getRelaxedOpcodeBranch(Opcode);
}
-bool M68kAsmBackend::mayNeedRelaxation(const MCInst &Inst,
+bool M68kAsmBackend::mayNeedRelaxation(unsigned Opcode, ArrayRef<MCOperand>,
const MCSubtargetInfo &STI) const {
// Branches can always be relaxed in either mode.
- return getRelaxedOpcode(Inst.getOpcode()) != Inst.getOpcode();
+ return getRelaxedOpcode(Opcode) != Opcode;
// NOTE will change for x20 mem
}
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
index f88e4d8fc8c71..89a87798d71e4 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
@@ -140,9 +140,9 @@ bool RISCVAsmBackend::fixupNeedsRelaxationAdvanced(const MCFixup &Fixup,
// Given a compressed control flow instruction this function returns
// the expanded instruction, or the original instruction code if no
// expansion is available.
-static unsigned getRelaxedOpcode(const MCInst &Inst,
+static unsigned getRelaxedOpcode(unsigned Opcode, ArrayRef<MCOperand> Operands,
const MCSubtargetInfo &STI) {
- switch (Inst.getOpcode()) {
+ switch (Opcode) {
case RISCV::C_BEQZ:
return RISCV::BEQ;
case RISCV::C_BNEZ:
@@ -158,7 +158,7 @@ static unsigned getRelaxedOpcode(const MCInst &Inst,
break;
// And only if it is using X0 or X1 for rd.
- MCRegister Reg = Inst.getOperand(0).getReg();
+ MCRegister Reg = Operands[0].getReg();
if (Reg == RISCV::X0)
return RISCV::QC_E_J;
if (Reg == RISCV::X1)
@@ -205,7 +205,7 @@ static unsigned getRelaxedOpcode(const MCInst &Inst,
}
// Returning the original opcode means we cannot relax the instruction.
- return Inst.getOpcode();
+ return Opcode;
}
void RISCVAsmBackend::relaxInstruction(MCInst &Inst,
@@ -223,7 +223,8 @@ void RISCVAsmBackend::relaxInstruction(MCInst &Inst,
case RISCV::C_JAL: {
[[maybe_unused]] bool Success = RISCVRVC::uncompress(Res, Inst, STI);
assert(Success && "Can't uncompress instruction");
- assert(Res.getOpcode() == getRelaxedOpcode(Inst, STI) &&
+ assert(Res.getOpcode() ==
+ getRelaxedOpcode(Inst.getOpcode(), Inst.getOperands(), STI) &&
"Branch Relaxation Error");
break;
}
@@ -235,7 +236,7 @@ void RISCVAsmBackend::relaxInstruction(MCInst &Inst,
assert((Inst.getOperand(0).getReg() == RISCV::X0 ||
Inst.getOperand(0).getReg() == RISCV::X1) &&
"JAL only relaxable with rd=x0 or rd=x1");
- Res.setOpcode(getRelaxedOpcode(Inst, STI));
+ Res.setOpcode(getRelaxedOpcode(Inst.getOpcode(), Inst.getOperands(), STI));
Res.addOperand(Inst.getOperand(1));
break;
}
@@ -257,7 +258,7 @@ void RISCVAsmBackend::relaxInstruction(MCInst &Inst,
case RISCV::QC_E_BGEI:
case RISCV::QC_E_BLTUI:
case RISCV::QC_E_BGEUI:
- Res.setOpcode(getRelaxedOpcode(Inst, STI));
+ Res.setOpcode(getRelaxedOpcode(Inst.getOpcode(), Inst.getOperands(), STI));
Res.addOperand(Inst.getOperand(0));
Res.addOperand(Inst.getOperand(1));
Res.addOperand(Inst.getOperand(2));
@@ -399,7 +400,8 @@ std::pair<bool, bool> RISCVAsmBackend::relaxLEB128(MCLEBFragment &LF,
return std::make_pair(Expr.evaluateKnownAbsolute(Value, *Asm), false);
}
-bool RISCVAsmBackend::mayNeedRelaxation(const MCInst &Inst,
+bool RISCVAsmBackend::mayNeedRelaxation(unsigned Opcode,
+ ArrayRef<MCOperand> Operands,
const MCSubtargetInfo &STI) const {
// This function has access to two STIs, the member of the AsmBackend, and the
// one passed as an argument. The latter is more specific, so we query it for
@@ -407,7 +409,7 @@ bool RISCVAsmBackend::mayNeedRelaxation(const MCInst &Inst,
if (STI.hasFeature(RISCV::FeatureExactAssembly))
return false;
- return getRelaxedOpcode(Inst, STI) != Inst.getOpcode();
+ return getRelaxedOpcode(Opcode, Operands, STI) != Opcode;
}
bool RISCVAsmBackend::writeNopData(raw_ostream &OS, uint64_t Count,
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
index 305240af88ec5..1f1a6f5fe31a0 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
@@ -67,9 +67,8 @@ class RISCVAsmBackend : public MCAsmBackend {
MCFixupKindInfo getFixupKindInfo(MCFixupKind Kind) const override;
- bool mayNeedRelaxation(const MCInst &Inst,
+ bool mayNeedRelaxation(unsigned Opcode, ArrayRef<MCOperand> Operands,
const MCSubtargetInfo &STI) const override;
-
void relaxInstruction(MCInst &Inst,
const MCSubtargetInfo &STI) const override;
diff --git a/llvm/lib/Target/VE/MCTargetDesc/VEAsmBackend.cpp b/llvm/lib/Target/VE/MCTargetDesc/VEAsmBackend....
[truncated]
|
|
@llvm/pr-subscribers-backend-amdgpu Author: Fangrui Song (MaskRay) ChangesFollow-up to #146307 Moved MCInst storage to MCSection, enabling trivial ~MCRelaxableFragment Updated MCRelaxableFragment::getInst to construct an MCInst on demand. There is a small decrease in max-rss (stage1-ReleaseLTO-g (link only)) Next: Enable MCDataFragment to store optional Opcode/Operands data (when Patch is 25.05 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/147229.diff 18 Files Affected:
diff --git a/llvm/include/llvm/MC/MCAsmBackend.h b/llvm/include/llvm/MC/MCAsmBackend.h
index ebc98ebfce9f7..6b81bdba25e67 100644
--- a/llvm/include/llvm/MC/MCAsmBackend.h
+++ b/llvm/include/llvm/MC/MCAsmBackend.h
@@ -32,6 +32,7 @@ class MCInst;
class MCObjectStreamer;
class MCObjectTargetWriter;
class MCObjectWriter;
+class MCOperand;
class MCSubtargetInfo;
class MCValue;
class raw_pwrite_stream;
@@ -147,12 +148,9 @@ class LLVM_ABI MCAsmBackend {
/// \name Target Relaxation Interfaces
/// @{
- /// Check whether the given instruction may need relaxation.
- ///
- /// \param Inst - The instruction to test.
- /// \param STI - The MCSubtargetInfo in effect when the instruction was
- /// encoded.
- virtual bool mayNeedRelaxation(const MCInst &Inst,
+ /// Check whether the given instruction (encoded as Opcode+Operands) may need
+ /// relaxation.
+ virtual bool mayNeedRelaxation(unsigned Opcode, ArrayRef<MCOperand> Operands,
const MCSubtargetInfo &STI) const {
return false;
}
diff --git a/llvm/include/llvm/MC/MCInst.h b/llvm/include/llvm/MC/MCInst.h
index f26af88d92140..b0db6b8408da5 100644
--- a/llvm/include/llvm/MC/MCInst.h
+++ b/llvm/include/llvm/MC/MCInst.h
@@ -15,6 +15,7 @@
#ifndef LLVM_MC_MCINST_H
#define LLVM_MC_MCINST_H
+#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/bit.h"
@@ -210,7 +211,11 @@ class MCInst {
MCOperand &getOperand(unsigned i) { return Operands[i]; }
unsigned getNumOperands() const { return Operands.size(); }
+ ArrayRef<MCOperand> getOperands() const { return Operands; }
void addOperand(const MCOperand Op) { Operands.push_back(Op); }
+ void setOperands(ArrayRef<MCOperand> Ops) {
+ Operands.assign(Ops.begin(), Ops.end());
+ }
using iterator = SmallVectorImpl<MCOperand>::iterator;
using const_iterator = SmallVectorImpl<MCOperand>::const_iterator;
diff --git a/llvm/include/llvm/MC/MCSection.h b/llvm/include/llvm/MC/MCSection.h
index 3abe5e0fe3c6f..f6f62d017dd6a 100644
--- a/llvm/include/llvm/MC/MCSection.h
+++ b/llvm/include/llvm/MC/MCSection.h
@@ -133,6 +133,7 @@ class LLVM_ABI MCSection {
friend MCAssembler;
friend MCObjectStreamer;
friend class MCEncodedFragment;
+ friend class MCRelaxableFragment;
static constexpr unsigned NonUniqueID = ~0U;
enum SectionVariant {
@@ -213,6 +214,7 @@ class LLVM_ABI MCSection {
// Content and fixup storage for fragments
SmallVector<char, 0> ContentStorage;
SmallVector<MCFixup, 0> FixupStorage;
+ SmallVector<MCOperand, 0> MCOperandStorage;
protected:
// TODO Make Name private when possible.
@@ -221,7 +223,8 @@ class LLVM_ABI MCSection {
MCSection(SectionVariant V, StringRef Name, bool IsText, bool IsVirtual,
MCSymbol *Begin);
- ~MCSection();
+ // Protected non-virtual dtor prevents destroy through a base class pointer.
+ ~MCSection() {}
public:
MCSection(const MCSection &) = delete;
@@ -431,16 +434,38 @@ class MCDataFragment : public MCEncodedFragment {
///
class MCRelaxableFragment : public MCEncodedFragment {
/// The instruction this is a fragment for.
- MCInst Inst;
+ unsigned Opcode = 0;
+ uint32_t OperandStart = 0;
+ uint32_t OperandSize = 0;
public:
- MCRelaxableFragment(const MCInst &Inst, const MCSubtargetInfo &STI)
- : MCEncodedFragment(FT_Relaxable, true), Inst(Inst) {
+ MCRelaxableFragment(const MCSubtargetInfo &STI)
+ : MCEncodedFragment(FT_Relaxable, true) {
this->STI = &STI;
}
- const MCInst &getInst() const { return Inst; }
- void setInst(const MCInst &Value) { Inst = Value; }
+ unsigned getOpcode() const { return Opcode; }
+ ArrayRef<MCOperand> getOperands() const {
+ return MutableArrayRef(getParent()->MCOperandStorage)
+ .slice(OperandStart, OperandSize);
+ }
+ MCInst getInst() const {
+ MCInst Inst;
+ Inst.setOpcode(Opcode);
+ Inst.setOperands(ArrayRef(getParent()->MCOperandStorage)
+ .slice(OperandStart, OperandSize));
+ return Inst;
+ }
+ void setInst(const MCInst &Inst) {
+ Opcode = Inst.getOpcode();
+ auto &S = getParent()->MCOperandStorage;
+ if (Inst.getNumOperands() > OperandSize) {
+ OperandStart = S.size();
+ S.resize_for_overwrite(S.size() + Inst.getNumOperands());
+ }
+ OperandSize = Inst.getNumOperands();
+ llvm::copy(Inst, S.begin() + OperandStart);
+ }
bool getAllowAutoPadding() const { return AllowAutoPadding; }
void setAllowAutoPadding(bool V) { AllowAutoPadding = V; }
diff --git a/llvm/lib/MC/MCAssembler.cpp b/llvm/lib/MC/MCAssembler.cpp
index 496d66b1876b2..480e6fe027beb 100644
--- a/llvm/lib/MC/MCAssembler.cpp
+++ b/llvm/lib/MC/MCAssembler.cpp
@@ -869,7 +869,8 @@ bool MCAssembler::relaxInstruction(MCRelaxableFragment &F) {
// If this inst doesn't ever need relaxation, ignore it. This occurs when we
// are intentionally pushing out inst fragments, or because we relaxed a
// previous instruction to one that doesn't need relaxation.
- if (!getBackend().mayNeedRelaxation(F.getInst(), *F.getSubtargetInfo()))
+ if (!getBackend().mayNeedRelaxation(F.getOpcode(), F.getOperands(),
+ *F.getSubtargetInfo()))
return false;
bool DoRelax = false;
@@ -881,6 +882,8 @@ bool MCAssembler::relaxInstruction(MCRelaxableFragment &F) {
++stats::RelaxedInstructions;
+ // TODO Refactor relaxInstruction to accept MCRelaxableFragment and remove
+ // `setInst`.
MCInst Relaxed = F.getInst();
getBackend().relaxInstruction(Relaxed, *F.getSubtargetInfo());
diff --git a/llvm/lib/MC/MCObjectStreamer.cpp b/llvm/lib/MC/MCObjectStreamer.cpp
index bd1a77467679b..5cc9bed2669d4 100644
--- a/llvm/lib/MC/MCObjectStreamer.cpp
+++ b/llvm/lib/MC/MCObjectStreamer.cpp
@@ -352,7 +352,7 @@ void MCObjectStreamer::emitInstructionImpl(const MCInst &Inst,
// If this instruction doesn't need relaxation, just emit it as data.
MCAssembler &Assembler = getAssembler();
MCAsmBackend &Backend = Assembler.getBackend();
- if (!(Backend.mayNeedRelaxation(Inst, STI) ||
+ if (!(Backend.mayNeedRelaxation(Inst.getOpcode(), Inst.getOperands(), STI) ||
Backend.allowEnhancedRelaxation())) {
emitInstToData(Inst, STI);
return;
@@ -366,7 +366,8 @@ void MCObjectStreamer::emitInstructionImpl(const MCInst &Inst,
if (Assembler.getRelaxAll() ||
(Assembler.isBundlingEnabled() && Sec->isBundleLocked())) {
MCInst Relaxed = Inst;
- while (Backend.mayNeedRelaxation(Relaxed, STI))
+ while (Backend.mayNeedRelaxation(Relaxed.getOpcode(), Relaxed.getOperands(),
+ STI))
Backend.relaxInstruction(Relaxed, STI);
emitInstToData(Relaxed, STI);
return;
@@ -397,8 +398,9 @@ void MCObjectStreamer::emitInstToFragment(const MCInst &Inst,
// Always create a new, separate fragment here, because its size can change
// during relaxation.
MCRelaxableFragment *IF =
- getContext().allocFragment<MCRelaxableFragment>(Inst, STI);
+ getContext().allocFragment<MCRelaxableFragment>(STI);
insert(IF);
+ IF->setInst(Inst);
SmallVector<MCFixup, 1> Fixups;
getAssembler().getEmitter().encodeInstruction(
diff --git a/llvm/lib/MC/MCSection.cpp b/llvm/lib/MC/MCSection.cpp
index 09d09e8f997e6..56450033529bc 100644
--- a/llvm/lib/MC/MCSection.cpp
+++ b/llvm/lib/MC/MCSection.cpp
@@ -36,18 +36,6 @@ MCSymbol *MCSection::getEndSymbol(MCContext &Ctx) {
bool MCSection::hasEnded() const { return End && End->isInSection(); }
-MCSection::~MCSection() {
- // If ~MCRelaxableFragment becomes trivial (no longer store a MCInst member),
- // this dtor can be made empty.
- for (auto &[_, Chain] : Subsections) {
- for (MCFragment *X = Chain.Head, *Y; X; X = Y) {
- Y = X->Next;
- if (auto *F = dyn_cast<MCRelaxableFragment>(X))
- F->~MCRelaxableFragment();
- }
- }
-}
-
void MCSection::setBundleLockState(BundleLockStateType NewState) {
if (NewState == NotBundleLocked) {
if (BundleLockNestingDepth == 0) {
diff --git a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUAsmBackend.cpp b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUAsmBackend.cpp
index b333c97ecc378..8f89168754180 100644
--- a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUAsmBackend.cpp
+++ b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUAsmBackend.cpp
@@ -40,7 +40,7 @@ class AMDGPUAsmBackend : public MCAsmBackend {
void relaxInstruction(MCInst &Inst,
const MCSubtargetInfo &STI) const override;
- bool mayNeedRelaxation(const MCInst &Inst,
+ bool mayNeedRelaxation(unsigned Opcode, ArrayRef<MCOperand> Operands,
const MCSubtargetInfo &STI) const override;
unsigned getMinimumNopSize() const override;
@@ -70,12 +70,13 @@ bool AMDGPUAsmBackend::fixupNeedsRelaxation(const MCFixup &Fixup,
return (((int64_t(Value)/4)-1) == 0x3f);
}
-bool AMDGPUAsmBackend::mayNeedRelaxation(const MCInst &Inst,
- const MCSubtargetInfo &STI) const {
+bool AMDGPUAsmBackend::mayNeedRelaxation(unsigned Opcode,
+ ArrayRef<MCOperand> Operands,
+ const MCSubtargetInfo &STI) const {
if (!STI.hasFeature(AMDGPU::FeatureOffset3fBug))
return false;
- if (AMDGPU::getSOPPWithRelaxation(Inst.getOpcode()) >= 0)
+ if (AMDGPU::getSOPPWithRelaxation(Opcode) >= 0)
return true;
return false;
diff --git a/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp b/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
index fc9a32072a627..376bddb120d5f 100644
--- a/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
+++ b/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
@@ -201,11 +201,9 @@ unsigned ARMAsmBackend::getRelaxedOpcode(unsigned Op,
}
}
-bool ARMAsmBackend::mayNeedRelaxation(const MCInst &Inst,
+bool ARMAsmBackend::mayNeedRelaxation(unsigned Opcode, ArrayRef<MCOperand>,
const MCSubtargetInfo &STI) const {
- if (getRelaxedOpcode(Inst.getOpcode(), STI) != Inst.getOpcode())
- return true;
- return false;
+ return getRelaxedOpcode(Opcode, STI) != Opcode;
}
static const char *checkPCRelOffset(uint64_t Value, int64_t Min, int64_t Max) {
diff --git a/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.h b/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.h
index 52279a3b6936f..877e3afdb1d57 100644
--- a/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.h
+++ b/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.h
@@ -45,7 +45,7 @@ class ARMAsmBackend : public MCAsmBackend {
unsigned getRelaxedOpcode(unsigned Op, const MCSubtargetInfo &STI) const;
- bool mayNeedRelaxation(const MCInst &Inst,
+ bool mayNeedRelaxation(unsigned Opcode, ArrayRef<MCOperand> Operands,
const MCSubtargetInfo &STI) const override;
const char *reasonForFixupRelaxation(const MCFixup &Fixup,
diff --git a/llvm/lib/Target/CSKY/MCTargetDesc/CSKYAsmBackend.cpp b/llvm/lib/Target/CSKY/MCTargetDesc/CSKYAsmBackend.cpp
index 0922d037149e5..ce1da6e58b9cd 100644
--- a/llvm/lib/Target/CSKY/MCTargetDesc/CSKYAsmBackend.cpp
+++ b/llvm/lib/Target/CSKY/MCTargetDesc/CSKYAsmBackend.cpp
@@ -239,9 +239,9 @@ void CSKYAsmBackend::applyFixup(const MCFragment &F, const MCFixup &Fixup,
}
}
-bool CSKYAsmBackend::mayNeedRelaxation(const MCInst &Inst,
+bool CSKYAsmBackend::mayNeedRelaxation(unsigned Opcode, ArrayRef<MCOperand>,
const MCSubtargetInfo &STI) const {
- switch (Inst.getOpcode()) {
+ switch (Opcode) {
default:
return false;
case CSKY::JBR32:
diff --git a/llvm/lib/Target/CSKY/MCTargetDesc/CSKYAsmBackend.h b/llvm/lib/Target/CSKY/MCTargetDesc/CSKYAsmBackend.h
index 142c667a6175e..1d3a22c2bbbb4 100644
--- a/llvm/lib/Target/CSKY/MCTargetDesc/CSKYAsmBackend.h
+++ b/llvm/lib/Target/CSKY/MCTargetDesc/CSKYAsmBackend.h
@@ -33,12 +33,11 @@ class CSKYAsmBackend : public MCAsmBackend {
bool fixupNeedsRelaxation(const MCFixup &Fixup,
uint64_t Value) const override;
+ bool mayNeedRelaxation(unsigned Opcode, ArrayRef<MCOperand> Operands,
+ const MCSubtargetInfo &STI) const override;
void relaxInstruction(MCInst &Inst,
const MCSubtargetInfo &STI) const override;
- bool mayNeedRelaxation(const MCInst &Inst,
- const MCSubtargetInfo &STI) const override;
-
bool fixupNeedsRelaxationAdvanced(const MCFixup &, const MCValue &, uint64_t,
bool) const override;
diff --git a/llvm/lib/Target/Hexagon/MCTargetDesc/HexagonAsmBackend.cpp b/llvm/lib/Target/Hexagon/MCTargetDesc/HexagonAsmBackend.cpp
index 847895f84ca9d..de7bd5d4b2c66 100644
--- a/llvm/lib/Target/Hexagon/MCTargetDesc/HexagonAsmBackend.cpp
+++ b/llvm/lib/Target/Hexagon/MCTargetDesc/HexagonAsmBackend.cpp
@@ -40,7 +40,7 @@ class HexagonAsmBackend : public MCAsmBackend {
uint8_t OSABI;
StringRef CPU;
mutable uint64_t relaxedCnt;
- mutable const MCInst *RelaxedMCB = nullptr;
+ mutable MCInst RelaxedMCB;
std::unique_ptr <MCInstrInfo> MCII;
std::unique_ptr <MCInst *> RelaxTarget;
MCInst * Extender;
@@ -428,13 +428,11 @@ class HexagonAsmBackend : public MCAsmBackend {
return Relaxable;
}
- /// MayNeedRelaxation - Check whether the given instruction may need
- /// relaxation.
- ///
- /// \param Inst - The instruction to test.
- bool mayNeedRelaxation(MCInst const &Inst,
+ bool mayNeedRelaxation(unsigned Opcode, ArrayRef<MCOperand> Operands,
const MCSubtargetInfo &STI) const override {
- RelaxedMCB = &Inst;
+ RelaxedMCB.clear();
+ RelaxedMCB.setOpcode(Opcode);
+ RelaxedMCB.setOperands(Operands);
return true;
}
@@ -443,7 +441,7 @@ class HexagonAsmBackend : public MCAsmBackend {
bool fixupNeedsRelaxationAdvanced(const MCFixup &Fixup, const MCValue &,
uint64_t Value,
bool Resolved) const override {
- MCInst const &MCB = *RelaxedMCB;
+ MCInst const &MCB = RelaxedMCB;
assert(HexagonMCInstrInfo::isBundle(MCB));
*RelaxTarget = nullptr;
@@ -598,7 +596,7 @@ class HexagonAsmBackend : public MCAsmBackend {
case MCFragment::FT_Relaxable: {
MCContext &Context = getContext();
auto &RF = cast<MCRelaxableFragment>(*Frags[K]);
- auto &Inst = const_cast<MCInst &>(RF.getInst());
+ MCInst Inst = RF.getInst();
const bool WouldTraverseLabel = llvm::any_of(
Asm.symbols(), [&Asm, &RF, &Inst](MCSymbol const &sym) {
diff --git a/llvm/lib/Target/M68k/MCTargetDesc/M68kAsmBackend.cpp b/llvm/lib/Target/M68k/MCTargetDesc/M68kAsmBackend.cpp
index f0c3728ec0eea..5e039033704f9 100644
--- a/llvm/lib/Target/M68k/MCTargetDesc/M68kAsmBackend.cpp
+++ b/llvm/lib/Target/M68k/MCTargetDesc/M68kAsmBackend.cpp
@@ -56,7 +56,7 @@ class M68kAsmBackend : public MCAsmBackend {
MutableArrayRef<char> Data, uint64_t Value,
bool IsResolved) override;
- bool mayNeedRelaxation(const MCInst &Inst,
+ bool mayNeedRelaxation(unsigned Opcode, ArrayRef<MCOperand> Operands,
const MCSubtargetInfo &STI) const override;
bool fixupNeedsRelaxation(const MCFixup &Fixup,
@@ -183,10 +183,10 @@ static unsigned getRelaxedOpcode(unsigned Opcode) {
return getRelaxedOpcodeBranch(Opcode);
}
-bool M68kAsmBackend::mayNeedRelaxation(const MCInst &Inst,
+bool M68kAsmBackend::mayNeedRelaxation(unsigned Opcode, ArrayRef<MCOperand>,
const MCSubtargetInfo &STI) const {
// Branches can always be relaxed in either mode.
- return getRelaxedOpcode(Inst.getOpcode()) != Inst.getOpcode();
+ return getRelaxedOpcode(Opcode) != Opcode;
// NOTE will change for x20 mem
}
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
index f88e4d8fc8c71..89a87798d71e4 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
@@ -140,9 +140,9 @@ bool RISCVAsmBackend::fixupNeedsRelaxationAdvanced(const MCFixup &Fixup,
// Given a compressed control flow instruction this function returns
// the expanded instruction, or the original instruction code if no
// expansion is available.
-static unsigned getRelaxedOpcode(const MCInst &Inst,
+static unsigned getRelaxedOpcode(unsigned Opcode, ArrayRef<MCOperand> Operands,
const MCSubtargetInfo &STI) {
- switch (Inst.getOpcode()) {
+ switch (Opcode) {
case RISCV::C_BEQZ:
return RISCV::BEQ;
case RISCV::C_BNEZ:
@@ -158,7 +158,7 @@ static unsigned getRelaxedOpcode(const MCInst &Inst,
break;
// And only if it is using X0 or X1 for rd.
- MCRegister Reg = Inst.getOperand(0).getReg();
+ MCRegister Reg = Operands[0].getReg();
if (Reg == RISCV::X0)
return RISCV::QC_E_J;
if (Reg == RISCV::X1)
@@ -205,7 +205,7 @@ static unsigned getRelaxedOpcode(const MCInst &Inst,
}
// Returning the original opcode means we cannot relax the instruction.
- return Inst.getOpcode();
+ return Opcode;
}
void RISCVAsmBackend::relaxInstruction(MCInst &Inst,
@@ -223,7 +223,8 @@ void RISCVAsmBackend::relaxInstruction(MCInst &Inst,
case RISCV::C_JAL: {
[[maybe_unused]] bool Success = RISCVRVC::uncompress(Res, Inst, STI);
assert(Success && "Can't uncompress instruction");
- assert(Res.getOpcode() == getRelaxedOpcode(Inst, STI) &&
+ assert(Res.getOpcode() ==
+ getRelaxedOpcode(Inst.getOpcode(), Inst.getOperands(), STI) &&
"Branch Relaxation Error");
break;
}
@@ -235,7 +236,7 @@ void RISCVAsmBackend::relaxInstruction(MCInst &Inst,
assert((Inst.getOperand(0).getReg() == RISCV::X0 ||
Inst.getOperand(0).getReg() == RISCV::X1) &&
"JAL only relaxable with rd=x0 or rd=x1");
- Res.setOpcode(getRelaxedOpcode(Inst, STI));
+ Res.setOpcode(getRelaxedOpcode(Inst.getOpcode(), Inst.getOperands(), STI));
Res.addOperand(Inst.getOperand(1));
break;
}
@@ -257,7 +258,7 @@ void RISCVAsmBackend::relaxInstruction(MCInst &Inst,
case RISCV::QC_E_BGEI:
case RISCV::QC_E_BLTUI:
case RISCV::QC_E_BGEUI:
- Res.setOpcode(getRelaxedOpcode(Inst, STI));
+ Res.setOpcode(getRelaxedOpcode(Inst.getOpcode(), Inst.getOperands(), STI));
Res.addOperand(Inst.getOperand(0));
Res.addOperand(Inst.getOperand(1));
Res.addOperand(Inst.getOperand(2));
@@ -399,7 +400,8 @@ std::pair<bool, bool> RISCVAsmBackend::relaxLEB128(MCLEBFragment &LF,
return std::make_pair(Expr.evaluateKnownAbsolute(Value, *Asm), false);
}
-bool RISCVAsmBackend::mayNeedRelaxation(const MCInst &Inst,
+bool RISCVAsmBackend::mayNeedRelaxation(unsigned Opcode,
+ ArrayRef<MCOperand> Operands,
const MCSubtargetInfo &STI) const {
// This function has access to two STIs, the member of the AsmBackend, and the
// one passed as an argument. The latter is more specific, so we query it for
@@ -407,7 +409,7 @@ bool RISCVAsmBackend::mayNeedRelaxation(const MCInst &Inst,
if (STI.hasFeature(RISCV::FeatureExactAssembly))
return false;
- return getRelaxedOpcode(Inst, STI) != Inst.getOpcode();
+ return getRelaxedOpcode(Opcode, Operands, STI) != Opcode;
}
bool RISCVAsmBackend::writeNopData(raw_ostream &OS, uint64_t Count,
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
index 305240af88ec5..1f1a6f5fe31a0 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
@@ -67,9 +67,8 @@ class RISCVAsmBackend : public MCAsmBackend {
MCFixupKindInfo getFixupKindInfo(MCFixupKind Kind) const override;
- bool mayNeedRelaxation(const MCInst &Inst,
+ bool mayNeedRelaxation(unsigned Opcode, ArrayRef<MCOperand> Operands,
const MCSubtargetInfo &STI) const override;
-
void relaxInstruction(MCInst &Inst,
const MCSubtargetInfo &STI) const override;
diff --git a/llvm/lib/Target/VE/MCTargetDesc/VEAsmBackend.cpp b/llvm/lib/Target/VE/MCTargetDesc/VEAsmBackend....
[truncated]
|
MaskRay
added a commit
to MaskRay/llvm-project
that referenced
this pull request
Jul 7, 2025
Follow-up to llvm#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. 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=408e87184f8274e30d188a2fe198facf55243733&to=5e0997dca139e08c6ace0d404a311e1f1b2d3b3f&stat=max-rss&linkStats=on Next: Enable MCDataFragment to store optional Opcode/Operands data (when OperandSize != 0), allowing re-encoding of Data+Relax+Data+Relax sequences as Data+Data. The saving should outweigh the downside of larger MCDataFragment. Pull Request: llvm#147229
MaskRay
added a commit
to MaskRay/llvm-project
that referenced
this pull request
Jul 7, 2025
Follow-up to llvm#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. 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=408e87184f8274e30d188a2fe198facf55243733&to=5e0997dca139e08c6ace0d404a311e1f1b2d3b3f&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: llvm#147229
aengelke
reviewed
Jul 7, 2025
There was a problem hiding this comment.
Tentative LGTM. MCFragment is now always trivially destructible, which is good. MCInst re-constructions seems a bit hacky, but ok. Flags might be a problem. This introduces 1--2 additional operand copies for relaxed instructions, but most instructions go into data fragments, so the impact is negligible.
| @@ -431,16 +428,38 @@ class MCDataFragment : public MCEncodedFragment { | |||
| /// | |||
| class MCRelaxableFragment : public MCEncodedFragment { | |||
| /// The instruction this is a fragment for. | |||
| MCInst Inst; | |||
There was a problem hiding this comment.
Is it ok to always drop Flags? We might end up with a different encoding for the relaxed instruction.
There was a problem hiding this comment.
MCInst::setFlags is only called by X86 (mostly disassembly, the MCInstLower uses don't use MCRelaxableFragment) and SPIRV (not using MCRelaxableFragment). Added an assert (Inst.getFlags()==0).
There was a problem hiding this comment.
X86MCInstLower::Lower calls setFlags and with allowEnhancedRelaxation, these can end up in a relaxable fragment?
There was a problem hiding this comment.
You are right. I take back my previous comment.. Updated the description
Unfortunately, we also have to encode MCInst::Flags to support the EVEX prefix, e.g.
{evex} xorw $foo, %ax.
I only noticed the case after adding the getFlags() == 0 assertion... The #78545 case should probably switch to llvm-objdump -dr
Yes, this will significantly reduce the number of fragments on x86 due to relaxable jumps. For several other architectures, e.g. AArch64, it will probably have a slightly negative impact. |
Agreed. The slightly negative impact should be offset by previous optimizations. After Data+Relax => Frag, I also plan to do pursue Data+Align => Frag (almost all text sections start with |
Created using spr 1.3.5-bogner
Sounds good (intuitively, align+data seems preferable over data+align). But I wonder, for e.g. AArch64, if we could reduce the number of fragments further. E.g. if we have align(16), data, align(16), data, then we could store these in a single fragment. |
lenary
reviewed
Jul 7, 2025
Comment on lines
+885
to
888
| // TODO Refactor relaxInstruction to accept MCRelaxableFragment and remove | ||
| // `setInst`. | ||
| MCInst Relaxed = F.getInst(); | ||
| getBackend().relaxInstruction(Relaxed, *F.getSubtargetInfo()); |
There was a problem hiding this comment.
It would be great if we could turn one instruction in a relaxable fragment into two. This is commonly done on RISC-V, and we have to use pseudos right now (which get expanded into two instructions later), but being able to emit the two instructions directly would be a lot more helpful, especially if we want to do another step and relax one of those instructions again.
This would be possible if we got access to the whole fragment, but I guess with this change it becomes impossible as the relaxable fragment can now only contain one instruction.
There was a problem hiding this comment.
Thanks for mentioning the RISC-V requirements. Right now, I need to focus on stabilizing the generic representation, as it's currently quite inefficient.
The MCInst representation is too large to ever support more than one instruction. I plan to refactor MCFragment to contain a fixed and a variable part (e.g., a span-dependent instruction), and delete all MCFragment subclasses. With the end state, encoding multiple instructions might become feasible.
MaskRay
added a commit
to MaskRay/llvm-project
that referenced
this pull request
Jul 8, 2025
Follow-up to llvm#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=408e87184f8274e30d188a2fe198facf55243733&to=5e0997dca139e08c6ace0d404a311e1f1b2d3b3f&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: llvm#147229
Created using spr 1.3.5-bogner
MaskRay
added a commit
to MaskRay/llvm-project
that referenced
this pull request
Jul 8, 2025
Follow-up to llvm#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: llvm#147229
aengelke
approved these changes
Jul 8, 2025
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.
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::Flagsto supportthe EVEX prefix, e.g.
{evex} xorw $foo, %axThere 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.