Skip to content

[AVR] Force relocations for non-encodable jumps #121498

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
merged 1 commit into from
Jan 20, 2025
Merged
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
1 change: 1 addition & 0 deletions llvm/include/llvm/MC/MCAsmBackend.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ class MCAsmBackend {
virtual bool shouldForceRelocation(const MCAssembler &Asm,
const MCFixup &Fixup,
const MCValue &Target,
const uint64_t Value,
const MCSubtargetInfo *STI) {
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/MC/MCAssembler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ bool MCAssembler::evaluateFixup(const MCFixup &Fixup, const MCFragment *DF,

// Let the backend force a relocation if needed.
if (IsResolved &&
getBackend().shouldForceRelocation(*this, Fixup, Target, STI)) {
getBackend().shouldForceRelocation(*this, Fixup, Target, Value, STI)) {
IsResolved = false;
WasForced = true;
}
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ class AArch64AsmBackend : public MCAsmBackend {
unsigned getFixupKindContainereSizeInBytes(unsigned Kind) const;

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

Expand Down Expand Up @@ -520,6 +520,7 @@ bool AArch64AsmBackend::writeNopData(raw_ostream &OS, uint64_t Count,
bool AArch64AsmBackend::shouldForceRelocation(const MCAssembler &Asm,
const MCFixup &Fixup,
const MCValue &Target,
const uint64_t,
const MCSubtargetInfo *STI) {
unsigned Kind = Fixup.getKind();
if (Kind >= FirstLiteralRelocationKind)
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUAsmBackend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class AMDGPUAsmBackend : public MCAsmBackend {
std::optional<MCFixupKind> getFixupKind(StringRef Name) const override;
const MCFixupKindInfo &getFixupKindInfo(MCFixupKind Kind) const override;
bool shouldForceRelocation(const MCAssembler &Asm, const MCFixup &Fixup,
const MCValue &Target,
const MCValue &Target, uint64_t Value,
const MCSubtargetInfo *STI) override;
};

Expand Down Expand Up @@ -196,7 +196,7 @@ const MCFixupKindInfo &AMDGPUAsmBackend::getFixupKindInfo(

bool AMDGPUAsmBackend::shouldForceRelocation(const MCAssembler &,
const MCFixup &Fixup,
const MCValue &,
const MCValue &, const uint64_t,
const MCSubtargetInfo *STI) {
return Fixup.getKind() >= FirstLiteralRelocationKind;
}
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -955,7 +955,7 @@ unsigned ARMAsmBackend::adjustFixupValue(const MCAssembler &Asm,

bool ARMAsmBackend::shouldForceRelocation(const MCAssembler &Asm,
const MCFixup &Fixup,
const MCValue &Target,
const MCValue &Target, const uint64_t,
const MCSubtargetInfo *STI) {
const MCSymbolRefExpr *A = Target.getSymA();
const MCSymbol *Sym = A ? &A->getSymbol() : nullptr;
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class ARMAsmBackend : public MCAsmBackend {
const MCFixupKindInfo &getFixupKindInfo(MCFixupKind Kind) const override;

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

unsigned adjustFixupValue(const MCAssembler &Asm, const MCFixup &Fixup,
Expand Down
58 changes: 31 additions & 27 deletions llvm/lib/Target/AVR/MCTargetDesc/AVRAsmBackend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,22 +31,6 @@ namespace adjust {

using namespace llvm;

static void signed_width(unsigned Width, uint64_t Value,
std::string Description, const MCFixup &Fixup,
MCContext *Ctx) {
if (!isIntN(Width, Value)) {
std::string Diagnostic = "out of range " + Description;

int64_t Min = minIntN(Width);
int64_t Max = maxIntN(Width);

Diagnostic += " (expected an integer in the range " + std::to_string(Min) +
" to " + std::to_string(Max) + ")";

Ctx->reportError(Fixup.getLoc(), Diagnostic);
}
}

static void unsigned_width(unsigned Width, uint64_t Value,
std::string Description, const MCFixup &Fixup,
MCContext *Ctx) {
Expand Down Expand Up @@ -74,17 +58,18 @@ static void adjustBranch(unsigned Size, const MCFixup &Fixup, uint64_t &Value,
}

/// Adjusts the value of a relative branch target before fixup application.
static void adjustRelativeBranch(unsigned Size, const MCFixup &Fixup,
uint64_t &Value, MCContext *Ctx) {
static bool adjustRelativeBranch(unsigned Size, const MCFixup &Fixup,
uint64_t &Value, const MCSubtargetInfo *STI) {
// Jumps are relative to the current instruction.
Value -= 2;

// We have one extra bit of precision because the value is rightshifted by
// one.
Size += 1;

if (!isIntN(Size, Value) &&
Ctx->getSubtargetInfo()->hasFeature(AVR::FeatureWrappingRjmp)) {
assert(STI && "STI can not be NULL");

if (!isIntN(Size, Value) && STI->hasFeature(AVR::FeatureWrappingRjmp)) {
const int32_t FlashSize = 0x2000;
int32_t SignedValue = Value;

Expand All @@ -96,10 +81,14 @@ static void adjustRelativeBranch(unsigned Size, const MCFixup &Fixup,
}
}

signed_width(Size, Value, std::string("branch target"), Fixup, Ctx);
if (!isIntN(Size, Value)) {
return false;
}

// Rightshifts the value by one.
AVR::fixups::adjustBranchTarget(Value);

return true;
}

/// 22-bit absolute fixup.
Expand All @@ -126,7 +115,9 @@ static void fixup_call(unsigned Size, const MCFixup &Fixup, uint64_t &Value,
/// Offset of 0 (so the result is left shifted by 3 bits before application).
static void fixup_7_pcrel(unsigned Size, const MCFixup &Fixup, uint64_t &Value,
MCContext *Ctx) {
adjustRelativeBranch(Size, Fixup, Value, Ctx);
if (!adjustRelativeBranch(Size, Fixup, Value, Ctx->getSubtargetInfo())) {
llvm_unreachable("should've been emitted as a relocation");
}

// Because the value may be negative, we must mask out the sign bits
Value &= 0x7f;
Expand All @@ -140,7 +131,9 @@ static void fixup_7_pcrel(unsigned Size, const MCFixup &Fixup, uint64_t &Value,
/// Offset of 0 (so the result isn't left-shifted before application).
static void fixup_13_pcrel(unsigned Size, const MCFixup &Fixup, uint64_t &Value,
MCContext *Ctx) {
adjustRelativeBranch(Size, Fixup, Value, Ctx);
if (!adjustRelativeBranch(Size, Fixup, Value, Ctx->getSubtargetInfo())) {
llvm_unreachable("should've been emitted as a relocation");
}

// Because the value may be negative, we must mask out the sign bits
Value &= 0xfff;
Expand Down Expand Up @@ -181,7 +174,7 @@ static void fixup_port5(const MCFixup &Fixup, uint64_t &Value, MCContext *Ctx) {
Value <<= 3;
}

/// 6-bit port number fixup on the `IN` family of instructions.
/// 6-bit port number fixup on the IN family of instructions.
///
/// Resolves to:
/// 1011 0AAd dddd AAAA
Expand Down Expand Up @@ -512,14 +505,25 @@ bool AVRAsmBackend::writeNopData(raw_ostream &OS, uint64_t Count,
bool AVRAsmBackend::shouldForceRelocation(const MCAssembler &Asm,
const MCFixup &Fixup,
const MCValue &Target,
const uint64_t Value,
const MCSubtargetInfo *STI) {
switch ((unsigned)Fixup.getKind()) {
default:
return Fixup.getKind() >= FirstLiteralRelocationKind;

case AVR::fixup_7_pcrel:
case AVR::fixup_13_pcrel:
// Always resolve relocations for PC-relative branches
return false;
case AVR::fixup_13_pcrel: {
uint64_t ValueEx = Value;
uint64_t Size = AVRAsmBackend::getFixupKindInfo(Fixup.getKind()).TargetSize;

// If the jump is too large to encode it, fall back to a relocation.
//
// Note that trying to actually link that relocation *would* fail, but the
// hopes are that the module we're currently compiling won't be actually
// linked to the final binary.
return !adjust::adjustRelativeBranch(Size, Fixup, ValueEx, STI);
}

case AVR::fixup_call:
return true;
}
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/AVR/MCTargetDesc/AVRAsmBackend.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class AVRAsmBackend : public MCAsmBackend {
const MCSubtargetInfo *STI) const override;

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

private:
Expand Down
1 change: 1 addition & 0 deletions llvm/lib/Target/CSKY/MCTargetDesc/CSKYAsmBackend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,7 @@ bool CSKYAsmBackend::mayNeedRelaxation(const MCInst &Inst,
bool CSKYAsmBackend::shouldForceRelocation(const MCAssembler &Asm,
const MCFixup &Fixup,
const MCValue &Target,
const uint64_t /*Value*/,
const MCSubtargetInfo * /*STI*/) {
if (Fixup.getKind() >= FirstLiteralRelocationKind)
return true;
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/CSKY/MCTargetDesc/CSKYAsmBackend.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class CSKYAsmBackend : public MCAsmBackend {
const MCSubtargetInfo *STI) const override;

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

std::unique_ptr<MCObjectTargetWriter>
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/Hexagon/MCTargetDesc/HexagonAsmBackend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ class HexagonAsmBackend : public MCAsmBackend {
}

bool shouldForceRelocation(const MCAssembler &Asm, const MCFixup &Fixup,
const MCValue &Target,
const MCValue &Target, const uint64_t,
const MCSubtargetInfo *STI) override {
switch(Fixup.getTargetKind()) {
default:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,7 @@ bool LoongArchAsmBackend::shouldInsertFixupForCodeAlign(MCAssembler &Asm,
bool LoongArchAsmBackend::shouldForceRelocation(const MCAssembler &Asm,
const MCFixup &Fixup,
const MCValue &Target,
const uint64_t,
const MCSubtargetInfo *STI) {
if (Fixup.getKind() >= FirstLiteralRelocationKind)
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class LoongArchAsmBackend : public MCAsmBackend {
MCAlignFragment &AF) override;

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

unsigned getNumFixupKinds() const override {
Expand Down
1 change: 1 addition & 0 deletions llvm/lib/Target/Mips/MCTargetDesc/MipsAsmBackend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -542,6 +542,7 @@ bool MipsAsmBackend::writeNopData(raw_ostream &OS, uint64_t Count,
bool MipsAsmBackend::shouldForceRelocation(const MCAssembler &Asm,
const MCFixup &Fixup,
const MCValue &Target,
const uint64_t,
const MCSubtargetInfo *STI) {
if (Fixup.getKind() >= FirstLiteralRelocationKind)
return true;
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/Mips/MCTargetDesc/MipsAsmBackend.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class MipsAsmBackend : public MCAsmBackend {
const MCSubtargetInfo *STI) const override;

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

bool isMicroMips(const MCSymbol *Sym) const override;
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/PowerPC/MCTargetDesc/PPCAsmBackend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ class PPCAsmBackend : public MCAsmBackend {
}

bool shouldForceRelocation(const MCAssembler &Asm, const MCFixup &Fixup,
const MCValue &Target,
const MCValue &Target, const uint64_t,
const MCSubtargetInfo *STI) override {
MCFixupKind Kind = Fixup.getKind();
switch ((unsigned)Kind) {
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ RISCVAsmBackend::getFixupKindInfo(MCFixupKind Kind) const {
bool RISCVAsmBackend::shouldForceRelocation(const MCAssembler &Asm,
const MCFixup &Fixup,
const MCValue &Target,
const uint64_t,
const MCSubtargetInfo *STI) {
if (Fixup.getKind() >= FirstLiteralRelocationKind)
return true;
Expand Down Expand Up @@ -567,7 +568,7 @@ bool RISCVAsmBackend::evaluateTargetFixup(const MCAssembler &Asm,
Value = Asm.getSymbolOffset(SA) + AUIPCTarget.getConstant();
Value -= Asm.getFragmentOffset(*AUIPCDF) + AUIPCFixup->getOffset();

if (shouldForceRelocation(Asm, *AUIPCFixup, AUIPCTarget, STI)) {
if (shouldForceRelocation(Asm, *AUIPCFixup, AUIPCTarget, Value, STI)) {
WasForced = true;
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class RISCVAsmBackend : public MCAsmBackend {
createObjectTargetWriter() const override;

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

bool fixupNeedsRelaxationAdvanced(const MCAssembler &Asm,
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/Sparc/MCTargetDesc/SparcAsmBackend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ namespace {
}

bool shouldForceRelocation(const MCAssembler &Asm, const MCFixup &Fixup,
const MCValue &Target,
const MCValue &Target, const uint64_t,
const MCSubtargetInfo *STI) override {
if (Fixup.getKind() >= FirstLiteralRelocationKind)
return true;
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/Target/SystemZ/MCTargetDesc/SystemZMCAsmBackend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ class SystemZMCAsmBackend : public MCAsmBackend {
std::optional<MCFixupKind> getFixupKind(StringRef Name) const override;
const MCFixupKindInfo &getFixupKindInfo(MCFixupKind Kind) const override;
bool shouldForceRelocation(const MCAssembler &Asm, const MCFixup &Fixup,
const MCValue &Target,
const MCValue &Target, const uint64_t Value,
const MCSubtargetInfo *STI) override;
void applyFixup(const MCAssembler &Asm, const MCFixup &Fixup,
const MCValue &Target, MutableArrayRef<char> Data,
Expand Down Expand Up @@ -161,7 +161,7 @@ SystemZMCAsmBackend::getFixupKindInfo(MCFixupKind Kind) const {

bool SystemZMCAsmBackend::shouldForceRelocation(const MCAssembler &,
const MCFixup &Fixup,
const MCValue &,
const MCValue &, const uint64_t,
const MCSubtargetInfo *STI) {
return Fixup.getKind() >= FirstLiteralRelocationKind;
}
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/VE/MCTargetDesc/VEAsmBackend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ class VEAsmBackend : public MCAsmBackend {
}

bool shouldForceRelocation(const MCAssembler &Asm, const MCFixup &Fixup,
const MCValue &Target,
const MCValue &Target, const uint64_t,
const MCSubtargetInfo *STI) override {
switch ((VE::Fixups)Fixup.getKind()) {
default:
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ class X86AsmBackend : public MCAsmBackend {
const MCFixupKindInfo &getFixupKindInfo(MCFixupKind Kind) const override;

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

void applyFixup(const MCAssembler &Asm, const MCFixup &Fixup,
Expand Down Expand Up @@ -659,6 +659,7 @@ const MCFixupKindInfo &X86AsmBackend::getFixupKindInfo(MCFixupKind Kind) const {

bool X86AsmBackend::shouldForceRelocation(const MCAssembler &,
const MCFixup &Fixup, const MCValue &,
const uint64_t,
const MCSubtargetInfo *STI) {
return Fixup.getKind() >= FirstLiteralRelocationKind;
}
Expand Down
11 changes: 9 additions & 2 deletions llvm/test/CodeGen/AVR/branch-relaxation-long-backward.ll
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
; RUN: llc < %s -mtriple=avr -mcpu=attiny85 -filetype=obj -o - | llvm-objdump --mcpu=attiny85 -dr --no-show-raw-insn --no-leading-addr - | FileCheck --check-prefix=ATTINY85 %s
; RUN: not llc < %s -mtriple=avr -mcpu=avr25 -filetype=obj -o - 2>&1 | FileCheck --check-prefix=AVR25 %s
; RUN: llc < %s -mtriple=avr -mcpu=avr25 -filetype=obj -o - | llvm-objdump --mcpu=avr25 -dr --no-show-raw-insn --no-leading-addr - | FileCheck --check-prefix=AVR25 %s
; RUN: llc < %s -mtriple=avr -mcpu=avr3 -filetype=obj -o - | llvm-objdump --mcpu=avr3 -dr --no-show-raw-insn --no-leading-addr - | FileCheck --check-prefix=AVR3 %s

; ATTINY85: <main>:
Expand All @@ -10,7 +10,14 @@
; ATTINY85: ldi r24, 0x3
; ATTINY85-NEXT: ret

; AVR25: error: out of range branch target (expected an integer in the range -4096 to 4095)
; AVR25: <main>:
; AVR25-NEXT: andi r24, 0x1
; AVR25: cpi r24, 0x0
; AVR25-NEXT: breq .+2
; AVR25-NEXT: rjmp .-2
; AVR25-NEXT: R_AVR_13_PCREL .text+0x2
; AVR25: ldi r24, 0x3
; AVR25-NEXT: ret

; AVR3: <main>:
; AVR3-NEXT: andi r24, 0x1
Expand Down
11 changes: 9 additions & 2 deletions llvm/test/CodeGen/AVR/branch-relaxation-long-forward.ll
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
; RUN: llc < %s -mtriple=avr -mcpu=attiny85 -filetype=obj -o - | llvm-objdump --mcpu=attiny85 -dr --no-show-raw-insn --no-leading-addr - | FileCheck --check-prefix=ATTINY85 %s
; RUN: not llc < %s -mtriple=avr -mcpu=avr25 -filetype=obj -o - 2>&1 | FileCheck --check-prefix=AVR25 %s
; RUN: llc < %s -mtriple=avr -mcpu=avr25 -filetype=obj -o - | llvm-objdump --mcpu=avr25 -dr --no-show-raw-insn --no-leading-addr - | FileCheck --check-prefix=AVR25 %s
; RUN: llc < %s -mtriple=avr -mcpu=avr3 -filetype=obj -o - | llvm-objdump --mcpu=avr3 -dr --no-show-raw-insn --no-leading-addr - | FileCheck --check-prefix=AVR3 %s

; ATTINY85: <main>:
Expand All @@ -10,7 +10,14 @@
; ATTINY85: ldi r24, 0x3
; ATTINY85-NEXT: ret

; AVR25: error: out of range branch target (expected an integer in the range -4096 to 4095)
; AVR25: <main>:
; AVR25-NEXT: andi r24, 0x1
; AVR25-NEXT: cpi r24, 0x0
; AVR25-NEXT: brne .+2
; AVR25-NEXT: rjmp .-2
; AVR25-NEXT: R_AVR_13_PCREL .text+0x100c
; AVR25: ldi r24, 0x3
; AVR25-NEXT: ret

; AVR3: <main>:
; AVR3-NEXT: andi r24, 0x1
Expand Down
Loading