Skip to content

RISCV: Remove shouldForceRelocation and unneeded relocations #140692

New issue

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

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

Already on GitHub? Sign in to your account

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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions llvm/include/llvm/MC/MCAssembler.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ class MCAssembler {
std::unique_ptr<MCObjectWriter> Writer;

bool HasLayout = false;
bool HasFinalLayout = false;
bool RelaxAll = false;

SectionListType Sections;
Expand Down Expand Up @@ -197,6 +198,7 @@ class MCAssembler {
void layout();

bool hasLayout() const { return HasLayout; }
bool hasFinalLayout() const { return HasFinalLayout; }
bool getRelaxAll() const { return RelaxAll; }
void setRelaxAll(bool Value) { RelaxAll = Value; }

Expand Down
10 changes: 5 additions & 5 deletions llvm/include/llvm/MC/MCFixup.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,9 @@ class MCFixup {
/// determine how the operand value should be encoded into the instruction.
MCFixupKind Kind = FK_NONE;

/// Used by RISC-V style linker relaxation. If the fixup is unresolved,
/// whether a RELAX relocation should follow.
bool NeedsRelax = false;
/// Used by RISC-V style linker relaxation. Whether the fixup is
/// linker-relaxable.
bool LinkerRelaxable = false;

/// Consider bit fields if we need more flags.

Expand Down Expand Up @@ -105,8 +105,8 @@ class MCFixup {

const MCExpr *getValue() const { return Value; }

bool needsRelax() const { return NeedsRelax; }
void setNeedsRelax() { NeedsRelax = true; }
bool isLinkerRelaxable() const { return LinkerRelaxable; }
void setLinkerRelaxable() { LinkerRelaxable = true; }

/// Return the generic fixup kind for a value with the given size. It
/// is an error to pass an unsupported size.
Expand Down
7 changes: 7 additions & 0 deletions llvm/include/llvm/MC/MCSection.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,10 @@ class MCSection {

bool IsVirtual : 1;

/// Whether the section contains linker-relaxable fragments. If true, the
/// offset between two locations may not be fully resolved.
bool LinkerRelaxable : 1;

MCDummyFragment DummyFragment;

// Mapping from subsection number to fragment list. At layout time, the
Expand Down Expand Up @@ -175,6 +179,9 @@ class MCSection {
bool isRegistered() const { return IsRegistered; }
void setIsRegistered(bool Value) { IsRegistered = Value; }

bool isLinkerRelaxable() const { return LinkerRelaxable; }
void setLinkerRelaxable() { LinkerRelaxable = true; }

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

Expand Down
4 changes: 4 additions & 0 deletions llvm/lib/MC/MCAssembler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -897,6 +897,10 @@ void MCAssembler::layout() {
// example, to set the index fields in the symbol data).
getWriter().executePostLayoutBinding(*this);

// Fragment sizes are finalized. For RISC-V linker relaxation, this flag
// helps check whether a PC-relative fixup is fully resolved.
this->HasFinalLayout = true;

// Evaluate and apply the fixups, generating relocation entries as necessary.
for (MCSection &Sec : *this) {
for (MCFragment &Frag : Sec) {
Expand Down
4 changes: 3 additions & 1 deletion llvm/lib/MC/MCELFStreamer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -450,8 +450,10 @@ void MCELFStreamer::emitInstToData(const MCInst &Inst,
auto Fixups = MutableArrayRef(DF->getFixups()).slice(FixupStartIndex);
for (auto &Fixup : Fixups) {
Fixup.setOffset(Fixup.getOffset() + CodeOffset);
if (Fixup.needsRelax())
if (Fixup.isLinkerRelaxable()) {
DF->setLinkerRelaxable();
getCurrentSectionOnly()->setLinkerRelaxable();
}
}

DF->setHasInstructions(STI);
Expand Down
6 changes: 6 additions & 0 deletions llvm/lib/MC/MCExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,12 @@ static void attemptToFoldSymbolOffsetDifference(const MCAssembler *Asm,
unsigned Count;
if (DF) {
Displacement += DF->getContents().size();
} else if (auto *RF = dyn_cast<MCRelaxableFragment>(FI);
RF && Asm->hasFinalLayout()) {
// Before finishLayout, a relaxable fragment's size is indeterminate.
// After layout, during relocation generation, it can be treated as a
// data fragment.
Displacement += RF->getContents().size();
} else if (auto *AF = dyn_cast<MCAlignFragment>(FI);
AF && Layout && AF->hasEmitNops() &&
!Asm->getBackend().shouldInsertExtraNopBytesForCodeAlign(
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/MC/MCSection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ MCSection::MCSection(SectionVariant V, StringRef Name, bool IsText,
bool IsVirtual, MCSymbol *Begin)
: Begin(Begin), BundleGroupBeforeFirstInst(false), HasInstructions(false),
HasLayout(false), IsRegistered(false), IsText(IsText),
IsVirtual(IsVirtual), Name(Name), Variant(V) {
IsVirtual(IsVirtual), LinkerRelaxable(false), Name(Name), Variant(V) {
DummyFragment.setParent(this);
// The initial subsection number is 0. Create a fragment list.
CurFragList = &Subsections.emplace_back(0u, FragList{}).second;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ bool LoongArchAsmBackend::addReloc(MCAssembler &Asm, const MCFragment &F,
IsResolved = Fallback();
// If linker relaxation is enabled and supported by the current relocation,
// append a RELAX relocation.
if (Fixup.needsRelax()) {
if (Fixup.isLinkerRelaxable()) {
auto FA = MCFixup::create(Fixup.getOffset(), nullptr, ELF::R_LARCH_RELAX);
Asm.getWriter().recordRelocation(Asm, &F, FA, MCValue::get(nullptr),
FixedValueA);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ LoongArchMCCodeEmitter::getExprOpValue(const MCInst &MI, const MCOperand &MO,
// a bit so that if fixup is unresolved, a R_LARCH_RELAX relocation will be
// appended.
if (EnableRelax && RelaxCandidate)
Fixups.back().setNeedsRelax();
Fixups.back().setLinkerRelaxable();

return 0;
}
Expand Down Expand Up @@ -254,7 +254,7 @@ void LoongArchMCCodeEmitter::expandAddTPRel(const MCInst &MI,
Fixups.push_back(
MCFixup::create(0, Expr, ELF::R_LARCH_TLS_LE_ADD_R, MI.getLoc()));
if (STI.hasFeature(LoongArch::FeatureRelax))
Fixups.back().setNeedsRelax();
Fixups.back().setLinkerRelaxable();

// Emit a normal ADD instruction with the given operands.
unsigned ADD = MI.getOpcode() == LoongArch::PseudoAddTPRel_D
Expand Down
15 changes: 0 additions & 15 deletions llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2885,21 +2885,6 @@ bool RISCVAsmParser::parseOperand(OperandVector &Operands, StringRef Mnemonic) {
bool RISCVAsmParser::parseInstruction(ParseInstructionInfo &Info,
StringRef Name, SMLoc NameLoc,
OperandVector &Operands) {
// Ensure that if the instruction occurs when relaxation is enabled,
// relocations are forced for the file. Ideally this would be done when there
// is enough information to reliably determine if the instruction itself may
// cause relaxations. Unfortunately instruction processing stage occurs in the
// same pass as relocation emission, so it's too late to set a 'sticky bit'
// for the entire file.
if (getSTI().hasFeature(RISCV::FeatureRelax)) {
auto *Assembler = getTargetStreamer().getStreamer().getAssemblerPtr();
if (Assembler != nullptr) {
RISCVAsmBackend &MAB =
static_cast<RISCVAsmBackend &>(Assembler->getBackend());
MAB.setForceRelocs();
}
}

// Apply mnemonic aliases because the destination mnemonic may have require
// custom operand parsing. The generic tblgen'erated code does this later, at
// the start of MatchInstructionImpl(), but that's too late for custom
Expand Down
60 changes: 33 additions & 27 deletions llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "llvm/MC/MCContext.h"
#include "llvm/MC/MCELFObjectWriter.h"
#include "llvm/MC/MCExpr.h"
#include "llvm/MC/MCFragment.h"
#include "llvm/MC/MCObjectWriter.h"
#include "llvm/MC/MCSymbol.h"
#include "llvm/MC/MCValue.h"
Expand Down Expand Up @@ -104,29 +105,6 @@ MCFixupKindInfo RISCVAsmBackend::getFixupKindInfo(MCFixupKind Kind) const {
return Infos[Kind - FirstTargetFixupKind];
}

// If linker relaxation is enabled, or the relax option had previously been
// enabled, always emit relocations even if the fixup can be resolved. This is
// necessary for correctness as offsets may change during relaxation.
bool RISCVAsmBackend::shouldForceRelocation(const MCAssembler &Asm,
const MCFixup &Fixup,
const MCValue &Target,
const MCSubtargetInfo *STI) {
switch (Fixup.getTargetKind()) {
default:
break;
case FK_Data_1:
case FK_Data_2:
case FK_Data_4:
case FK_Data_8:
case FK_Data_leb128:
if (Target.isAbsolute())
return false;
break;
}

return STI->hasFeature(RISCV::FeatureRelax) || ForceRelocs;
}

bool RISCVAsmBackend::fixupNeedsRelaxationAdvanced(const MCAssembler &,
const MCFixup &Fixup,
const MCValue &,
Expand Down Expand Up @@ -570,6 +548,27 @@ static uint64_t adjustFixupValue(const MCFixup &Fixup, uint64_t Value,
}
}

bool RISCVAsmBackend::isPCRelFixupResolved(const MCAssembler &Asm,
const MCSymbol *SymA,
const MCFragment &F) {
// If the section does not contain linker-relaxable instructions, PC-relative
// fixups can be resolved.
if (!F.getParent()->isLinkerRelaxable())
return true;

// Otherwise, check if the offset between the symbol and fragment is fully
// resolved, unaffected by linker-relaxable fragments (e.g. instructions or
// offset-affected MCAlignFragment). Complements the generic
// isSymbolRefDifferenceFullyResolvedImpl.
if (!PCRelTemp)
PCRelTemp = Asm.getContext().createTempSymbol();
PCRelTemp->setFragment(const_cast<MCFragment *>(&F));
MCValue Res;
MCExpr::evaluateSymbolicAdd(&Asm, false, MCValue::get(SymA),
MCValue::get(nullptr, PCRelTemp), Res);
return !Res.getSubSym();
}

bool RISCVAsmBackend::evaluateTargetFixup(
const MCAssembler &Asm, const MCFixup &Fixup, const MCFragment *DF,
const MCValue &Target, const MCSubtargetInfo *STI, uint64_t &Value) {
Expand Down Expand Up @@ -613,7 +612,8 @@ bool RISCVAsmBackend::evaluateTargetFixup(
Value = Asm.getSymbolOffset(SA) + AUIPCTarget.getConstant();
Value -= Asm.getFragmentOffset(*AUIPCDF) + AUIPCFixup->getOffset();

return AUIPCFixup->getTargetKind() == RISCV::fixup_riscv_pcrel_hi20;
return AUIPCFixup->getTargetKind() == RISCV::fixup_riscv_pcrel_hi20 &&
isPCRelFixupResolved(Asm, AUIPCTarget.getAddSym(), *AUIPCDF);
}

bool RISCVAsmBackend::addReloc(MCAssembler &Asm, const MCFragment &F,
Expand Down Expand Up @@ -659,11 +659,17 @@ bool RISCVAsmBackend::addReloc(MCAssembler &Asm, const MCFragment &F,
return false;
}

// If linker relaxation is enabled and supported by the current relocation,
// generate a relocation and then append a RELAX.
if (Fixup.isLinkerRelaxable())
IsResolved = false;
if (IsResolved &&
(getFixupKindInfo(Fixup.getKind()).Flags & MCFixupKindInfo::FKF_IsPCRel))
IsResolved = isPCRelFixupResolved(Asm, Target.getAddSym(), F);
IsResolved = MCAsmBackend::addReloc(Asm, F, Fixup, Target, FixedValue,
IsResolved, STI);
// If linker relaxation is enabled and supported by the current relocation,
// append a RELAX relocation.
if (Fixup.needsRelax()) {

if (Fixup.isLinkerRelaxable()) {
auto FA = MCFixup::create(Fixup.getOffset(), nullptr, ELF::R_RISCV_RELAX);
Asm.getWriter().recordRelocation(Asm, &F, FA, MCValue::get(nullptr),
FixedValueA);
Expand Down
12 changes: 5 additions & 7 deletions llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,18 @@ class RISCVAsmBackend : public MCAsmBackend {
const MCSubtargetInfo &STI;
uint8_t OSABI;
bool Is64Bit;
bool ForceRelocs = false;
const MCTargetOptions &TargetOptions;
// Temporary symbol used to check whether a PC-relative fixup is resolved.
MCSymbol *PCRelTemp = nullptr;

bool isPCRelFixupResolved(const MCAssembler &Asm, const MCSymbol *SymA,
const MCFragment &F);

public:
RISCVAsmBackend(const MCSubtargetInfo &STI, uint8_t OSABI, bool Is64Bit,
const MCTargetOptions &Options);
~RISCVAsmBackend() override = default;

void setForceRelocs() { ForceRelocs = true; }

// Return Size with extra Nop Bytes for alignment directive in code section.
bool shouldInsertExtraNopBytesForCodeAlign(const MCAlignFragment &AF,
unsigned &Size) override;
Expand All @@ -60,10 +62,6 @@ class RISCVAsmBackend : public MCAsmBackend {
std::unique_ptr<MCObjectTargetWriter>
createObjectTargetWriter() const override;

bool shouldForceRelocation(const MCAssembler &Asm, const MCFixup &Fixup,
const MCValue &Target,
const MCSubtargetInfo *STI) override;

bool fixupNeedsRelaxationAdvanced(const MCAssembler &,
const MCFixup &, const MCValue &, uint64_t,
bool) const override;
Expand Down
7 changes: 0 additions & 7 deletions llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,6 @@ RISCVTargetELFStreamer::RISCVTargetELFStreamer(MCStreamer &S,
setTargetABI(RISCVABI::computeTargetABI(STI.getTargetTriple(), Features,
MAB.getTargetOptions().getABIName()));
setFlagsFromFeatures(STI);
// `j label` in `.option norelax; j label; .option relax; ...; label:` needs a
// relocation to ensure the jump target is correct after linking. This is due
// to a limitation that shouldForceRelocation has to make the decision upfront
// without knowing a possibly future .option relax. When RISCVAsmParser is used,
// its ParseInstruction may call setForceRelocs as well.
if (STI.hasFeature(RISCV::FeatureRelax))
static_cast<RISCVAsmBackend &>(MAB).setForceRelocs();
}

RISCVELFStreamer &RISCVTargetELFStreamer::getStreamer() {
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ void RISCVMCCodeEmitter::expandAddTPRel(const MCInst &MI,
Fixups.push_back(
MCFixup::create(0, Expr, ELF::R_RISCV_TPREL_ADD, MI.getLoc()));
if (STI.hasFeature(RISCV::FeatureRelax))
Fixups.back().setNeedsRelax();
Fixups.back().setLinkerRelaxable();

// Emit a normal ADD instruction with the given operands.
MCInst TmpInst = MCInstBuilder(RISCV::ADD)
Expand Down Expand Up @@ -654,7 +654,7 @@ uint64_t RISCVMCCodeEmitter::getImmOpValue(const MCInst &MI, unsigned OpNo,
// a bit so that if fixup is unresolved, a R_RISCV_RELAX relocation will be
// appended.
if (EnableRelax && RelaxCandidate)
Fixups.back().setNeedsRelax();
Fixups.back().setLinkerRelaxable();
++MCNumFixups;

return 0;
Expand Down
1 change: 0 additions & 1 deletion llvm/test/CodeGen/RISCV/option-relax-relocation.ll
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
; RELAX-NEXT: R_RISCV_RELAX *ABS*
; CHECK-NEXT: jalr ra
; CHECK-NEXT: j {{.*}}
; RELAX-NEXT: R_RISCV_JAL .LBB0_{{.*}}
; CHECK-NEXT: j {{.*}}
; RELAX-NEXT: R_RISCV_JAL .L0
; NORELAX-NEXT: li a0, 0x0
Expand Down
3 changes: 1 addition & 2 deletions llvm/test/MC/RISCV/fixups-binary-expression.s
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,14 @@ beq a0, a1, .LBB1+32

c.j .+32
# CHECK-INSTR: c.j 0x30
# CHECK-RELOC-NEXT: R_RISCV_RVC_JUMP

c.j .LBB2+4
# CHECK-INSTR: c.j 0x22
# CHECK-RELOC-NEXT: R_RISCV_RVC_JUMP

c.beqz a0, .-2
# CHECK-INSTR: c.beqz a0, 0x12
# CHECK-RELOC-NEXT: R_RISCV_RVC_BRANCH

call relax
# CHECK-RELOC-NEXT: R_RISCV_CALL_PLT
.LBB2:
7 changes: 5 additions & 2 deletions llvm/test/MC/RISCV/linker-relaxation.s
Original file line number Diff line number Diff line change
Expand Up @@ -58,14 +58,17 @@ bar:

beq s1, s1, bar
# NORELAX-RELOC-NOT: R_RISCV_BRANCH
# RELAX-RELOC: R_RISCV_BRANCH bar 0x0

call bar
# NORELAX-RELOC-NOT: R_RISCV_CALL
# NORELAX-RELOC-NOT: R_RISCV_RELAX
# RELAX-RELOC: R_RISCV_CALL_PLT bar 0x0
# RELAX-RELOC-NEXT: R_RISCV_CALL_PLT bar 0x0
# RELAX-RELOC: R_RISCV_RELAX - 0x0

beq s1, s1, bar
# NORELAX-RELOC-NOT: R_RISCV_BRANCH
# RELAX-RELOC-NEXT: R_RISCV_BRANCH bar 0x0

lui t1, %hi(bar)
# NORELAX-RELOC: R_RISCV_HI20 bar 0x0
# NORELAX-RELOC-NOT: R_RISCV_RELAX
Expand Down
Loading
Loading