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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
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
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
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
51 changes: 25 additions & 26 deletions llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,29 +104,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 +547,21 @@ static uint64_t adjustFixupValue(const MCFixup &Fixup, uint64_t Value,
}
}

// Check if the offset between the symbol and fragment is fully resolved,
// unaffected by linker-relaxable instructions. Complements the generic
// isSymbolRefDifferenceFullyResolvedImpl.
bool RISCVAsmBackend::isPCRelFixupResolved(const MCAssembler &Asm,
const MCSymbol *SymA,
const MCFragment &F) {
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 +605,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,10 +652,16 @@ 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.needsRelax())
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()) {
auto FA = MCFixup::create(Fixup.getOffset(), nullptr, ELF::R_RISCV_RELAX);
Asm.getWriter().recordRelocation(Asm, &F, FA, MCValue::get(nullptr),
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
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
5 changes: 2 additions & 3 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
# CHECK-RELOC-NEXT: R_RISCV_RVC_JUMP .LBB2 0x4

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 relax 0x0
.LBB2:
3 changes: 1 addition & 2 deletions llvm/test/MC/RISCV/linker-relaxation.s
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,11 @@ 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

lui t1, %hi(bar)
Expand Down
18 changes: 0 additions & 18 deletions llvm/test/MC/RISCV/long-conditional-jump.s
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,8 @@ test:
# CHECK-INST-C-NEXT: jal zero, 0x28b2
# CHECK-INST-RELAX: bne a0, a1, 0x1464
# CHECK-INST-RELAX-NEXT: jal zero, {{.*}}
# CHECK-INST-RELAX-NEXT: R_RISCV_JAL .L2
# CHECK-INST-C-RELAX: bne a0, a1, 0x1462
# CHECK-INST-C-RELAX-NEXT: jal zero, {{.*}}
# CHECK-INST-C-RELAX-NEXT: R_RISCV_JAL .L2
beq a0, a1, .L2
.fill 1300, 4, 0
.L2:
Expand All @@ -49,10 +47,8 @@ test:
# CHECK-INST-C-NEXT: jal zero, 0x3d0c
# CHECK-INST-RELAX: bge a0, a1, 0x28c0
# CHECK-INST-RELAX-NEXT: jal zero, {{.*}}
# CHECK-INST-RELAX-NEXT: R_RISCV_JAL .L3
# CHECK-INST-C-RELAX: bge a0, a1, 0x28bc
# CHECK-INST-C-RELAX-NEXT: jal zero, {{.*}}
# CHECK-INST-C-RELAX-NEXT: R_RISCV_JAL .L3
blt a0, a1, .L3
.fill 1300, 4, 0
.L3:
Expand All @@ -63,10 +59,8 @@ test:
# CHECK-INST-C-NEXT: jal zero, 0x5166
# CHECK-INST-RELAX: blt a0, a1, 0x3d1c
# CHECK-INST-RELAX-NEXT: jal zero, {{.*}}
# CHECK-INST-RELAX-NEXT: R_RISCV_JAL .L4
# CHECK-INST-C-RELAX: blt a0, a1, 0x3d16
# CHECK-INST-C-RELAX-NEXT: jal zero, {{.*}}
# CHECK-INST-C-RELAX-NEXT: R_RISCV_JAL .L4
bge a0, a1, .L4
.fill 1300, 4, 0
.L4:
Expand All @@ -77,10 +71,8 @@ test:
# CHECK-INST-C-NEXT: jal zero, 0x65c0
# CHECK-INST-RELAX: bgeu a0, a1, 0x5178
# CHECK-INST-RELAX-NEXT: jal zero, {{.*}}
# CHECK-INST-RELAX-NEXT: R_RISCV_JAL .L5
# CHECK-INST-C-RELAX: bgeu a0, a1, 0x5170
# CHECK-INST-C-RELAX-NEXT: jal zero, {{.*}}
# CHECK-INST-C-RELAX-NEXT: R_RISCV_JAL .L5
bltu a0, a1, .L5
.fill 1300, 4, 0
.L5:
Expand All @@ -91,10 +83,8 @@ test:
# CHECK-INST-C-NEXT: jal zero, 0x7a1a
# CHECK-INST-RELAX: bltu a0, a1, 0x65d4
# CHECK-INST-RELAX-NEXT: jal zero, {{.*}}
# CHECK-INST-RELAX-NEXT: R_RISCV_JAL .L6
# CHECK-INST-C-RELAX: bltu a0, a1, 0x65ca
# CHECK-INST-C-RELAX-NEXT: jal zero, {{.*}}
# CHECK-INST-C-RELAX-NEXT: R_RISCV_JAL .L6
bgeu a0, a1, .L6
.fill 1300, 4, 0
.L6:
Expand All @@ -105,10 +95,8 @@ test:
# CHECK-INST-C-NEXT: jal zero, 0x8e72
# CHECK-INST-RELAX: bne a0, zero, 0x7a30
# CHECK-INST-RELAX-NEXT: jal zero, {{.*}}
# CHECK-INST-RELAX-NEXT: R_RISCV_JAL .L7
# CHECK-INST-C-RELAX: c.bnez a0, 0x7a22
# CHECK-INST-C-RELAX-NEXT: jal zero, {{.*}}
# CHECK-INST-C-RELAX-NEXT: R_RISCV_JAL .L7
beqz a0, .L7
.fill 1300, 4, 0
.L7:
Expand All @@ -119,10 +107,8 @@ test:
# CHECK-INST-C-NEXT: jal zero, 0xa2ca
# CHECK-INST-RELAX: bne zero, a0, 0x8e8c
# CHECK-INST-RELAX-NEXT: jal zero, {{.*}}
# CHECK-INST-RELAX-NEXT: R_RISCV_JAL .L8
# CHECK-INST-C-RELAX: c.bnez a0, 0x8e7a
# CHECK-INST-C-RELAX-NEXT: jal zero, {{.*}}
# CHECK-INST-C-RELAX-NEXT: R_RISCV_JAL .L8
beq x0, a0, .L8
.fill 1300, 4, 0
.L8:
Expand All @@ -133,10 +119,8 @@ test:
# CHECK-INST-C-NEXT: jal zero, 0xb722
# CHECK-INST-RELAX: beq a0, zero, 0xa2e8
# CHECK-INST-RELAX-NEXT: jal zero, {{.*}}
# CHECK-INST-RELAX-NEXT: R_RISCV_JAL .L9
# CHECK-INST-C-RELAX: c.beqz a0, 0xa2d2
# CHECK-INST-C-RELAX-NEXT: jal zero, {{.*}}
# CHECK-INST-C-RELAX-NEXT: R_RISCV_JAL .L9
bnez a0, .L9
.fill 1300, 4, 0
.L9:
Expand All @@ -147,10 +131,8 @@ test:
# CHECK-INST-C-NEXT: jal zero, 0xcb7c
# CHECK-INST-RELAX: beq a6, zero, 0xb744
# CHECK-INST-RELAX-NEXT: jal zero, {{.*}}
# CHECK-INST-RELAX-NEXT: R_RISCV_JAL .L10
# CHECK-INST-C-RELAX: beq a6, zero, 0xb72c
# CHECK-INST-C-RELAX-NEXT: jal zero, {{.*}}
# CHECK-INST-C-RELAX-NEXT: R_RISCV_JAL .L10
bnez x16, .L10
.fill 1300, 4, 0
.L10:
Expand Down
10 changes: 1 addition & 9 deletions llvm/test/MC/RISCV/option-relax.s
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,11 @@

# CHECK-INST: call foo
# CHECK-RELOC: R_RISCV_CALL_PLT foo 0x0
# CHECK-RELOC-NOT: R_RISCV_RELAX - 0x0
# CHECK-RELOC-NOT: R_RISCV
call foo

# CHECK-RELOC-NEXT: R_RISCV_ADD64
# CHECK-RELOC-NEXT: R_RISCV_SUB64
.dword .L2-.L1
# CHECK-RELOC-NEXT: R_RISCV_JAL
jal zero, .L1
# CHECK-RELOC-NEXT: R_RISCV_BRANCH
beq s1, s1, .L1

.L2:
Expand All @@ -41,8 +37,6 @@ beq s1, s1, .L1
# CHECK-RELOC-NEXT: R_RISCV_RELAX - 0x0
call bar

# CHECK-RELOC-NEXT: R_RISCV_ADD64
# CHECK-RELOC-NEXT: R_RISCV_SUB64
.dword .L2-.L1
# CHECK-RELOC-NEXT: R_RISCV_JAL
jal zero, .L1
Expand All @@ -57,8 +51,6 @@ beq s1, s1, .L1
# CHECK-RELOC-NOT: R_RISCV_RELAX - 0x0
call baz

# CHECK-RELOC-NEXT: R_RISCV_ADD64
# CHECK-RELOC-NEXT: R_RISCV_SUB64
.dword .L2-.L1
# CHECK-RELOC-NEXT: R_RISCV_JAL
jal zero, .L1
Expand Down
Loading
Loading