Skip to content

Commit

Permalink
[AVR] Force relocations for jumps and calls
Browse files Browse the repository at this point in the history
  • Loading branch information
@Patryk27
Patryk27 committed Nov 28, 2024
1 parent 69d66fa commit d519636
Show file tree
Hide file tree
Showing 26 changed files with 247 additions and 265 deletions.
22 changes: 14 additions & 8 deletions llvm/lib/Target/AVR/AVRInstrInfo.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -557,17 +557,23 @@ void AVRInstrInfo::insertIndirectBranch(MachineBasicBlock &MBB,
MachineBasicBlock &RestoreBB,
const DebugLoc &DL, int64_t BrOffset,
RegScavenger *RS) const {
// This method inserts a *direct* branch (JMP), despite its name.
// LLVM calls this method to fixup unconditional branches; it never calls
// insertBranch or some hypothetical "insertDirectBranch".
// See lib/CodeGen/RegisterRelaxation.cpp for details.
// We end up here when a jump is too long for a RJMP instruction.
// When an instruction's jump target is too far away¹, the branch relaxation
// pass might split an existing, large basic block into two pieces - if that
// happens, this function gets called to create a new jump between the blocks.
//
// Since this new jump might be too large for an `rjmp` anyway, let's
// conservatively emit `jmp` and back off to `rjmp` only if the target arch
// doesn't support `jmp`.
//
// TODO maybe we could use `BrOffset` to determine whether it's safe to emit
// `rjmp`? (it takes one byte less to encode, so it'd be a small win)
//
// ¹ "too far" in the sense of "the instruction encoding doesn't allow for an
// offset that large"

if (STI.hasJMPCALL())
BuildMI(&MBB, DL, get(AVR::JMPk)).addMBB(&NewDestBB);
else
// The RJMP may jump to a far place beyond its legal range. We let the
// linker to report 'out of range' rather than crash, or silently emit
// incorrect assembly code.
BuildMI(&MBB, DL, get(AVR::RJMPk)).addMBB(&NewDestBB);
}

Expand Down
155 changes: 26 additions & 129 deletions llvm/lib/Target/AVR/MCTargetDesc/AVRAsmBackend.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,129 +27,32 @@
#include "llvm/Support/MathExtras.h"
#include "llvm/Support/raw_ostream.h"

// FIXME: we should be doing checks to make sure asm operands
// are not out of bounds.

namespace adjust {

using namespace llvm;

static void signed_width(unsigned Width, uint64_t Value,
std::string Description, const MCFixup &Fixup,
MCContext *Ctx = nullptr) {
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) + ")";

if (Ctx) {
Ctx->reportError(Fixup.getLoc(), Diagnostic);
} else {
llvm_unreachable(Diagnostic.c_str());
}
}
}

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

int64_t Max = maxUIntN(Width);

Diagnostic +=
" (expected an integer in the range 0 to " + std::to_string(Max) + ")";
Diagnostic += " (expected an unsigned integer in the range 0 to " +
std::to_string(Max) + ", got " + std::to_string(Value) + ")";

if (Ctx) {
Ctx->reportError(Fixup.getLoc(), Diagnostic);
} else {
llvm_unreachable(Diagnostic.c_str());
}
Ctx->reportError(Fixup.getLoc(), Diagnostic);
}
}

/// Adjusts the value of a branch target before fixup application.
static void adjustBranch(unsigned Size, const MCFixup &Fixup, uint64_t &Value,
MCContext *Ctx = nullptr) {
// We have one extra bit of precision because the value is rightshifted by
// one.
unsigned_width(Size + 1, Value, std::string("branch target"), Fixup, Ctx);

// Rightshifts the value by one.
AVR::fixups::adjustBranchTarget(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 = nullptr) {
// Jumps are relative to the current instruction.
Value -= 2;

// We have one extra bit of precision because the value is rightshifted by
// one.
signed_width(Size + 1, Value, std::string("branch target"), Fixup, Ctx);

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

/// 22-bit absolute fixup.
///
/// Resolves to:
/// 1001 kkkk 010k kkkk kkkk kkkk 111k kkkk
///
/// Offset of 0 (so the result is left shifted by 3 bits before application).
static void fixup_call(unsigned Size, const MCFixup &Fixup, uint64_t &Value,
MCContext *Ctx = nullptr) {
adjustBranch(Size, Fixup, Value, Ctx);

auto top = Value & (0xf00000 << 6); // the top four bits
auto middle = Value & (0x1ffff << 5); // the middle 13 bits
auto bottom = Value & 0x1f; // end bottom 5 bits

Value = (top << 6) | (middle << 3) | (bottom << 0);
}

/// 7-bit PC-relative fixup.
///
/// Resolves to:
/// 0000 00kk kkkk k000
/// 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 = nullptr) {
adjustRelativeBranch(Size, Fixup, Value, Ctx);

// Because the value may be negative, we must mask out the sign bits
Value &= 0x7f;
}

/// 12-bit PC-relative fixup.
/// Yes, the fixup is 12 bits even though the name says otherwise.
///
/// Resolves to:
/// 0000 kkkk kkkk kkkk
/// 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 = nullptr) {
adjustRelativeBranch(Size, Fixup, Value, Ctx);

// Because the value may be negative, we must mask out the sign bits
Value &= 0xfff;
}

/// 6-bit fixup for the immediate operand of the STD/LDD family of
/// instructions.
///
/// Resolves to:
/// 10q0 qq10 0000 1qqq
static void fixup_6(const MCFixup &Fixup, uint64_t &Value,
MCContext *Ctx = nullptr) {
unsigned_width(6, Value, std::string("immediate"), Fixup, Ctx);
static void fixup_6(const MCFixup &Fixup, uint64_t &Value, MCContext *Ctx) {
ensureUnsignedWidth(6, Value, std::string("immediate"), Fixup, Ctx);

Value = ((Value & 0x20) << 8) | ((Value & 0x18) << 7) | (Value & 0x07);
}
Expand All @@ -160,8 +63,8 @@ static void fixup_6(const MCFixup &Fixup, uint64_t &Value,
/// Resolves to:
/// 0000 0000 kk00 kkkk
static void fixup_6_adiw(const MCFixup &Fixup, uint64_t &Value,
MCContext *Ctx = nullptr) {
unsigned_width(6, Value, std::string("immediate"), Fixup, Ctx);
MCContext *Ctx) {
ensureUnsignedWidth(6, Value, std::string("immediate"), Fixup, Ctx);

Value = ((Value & 0x30) << 2) | (Value & 0x0f);
}
Expand All @@ -170,9 +73,8 @@ static void fixup_6_adiw(const MCFixup &Fixup, uint64_t &Value,
///
/// Resolves to:
/// 0000 0000 AAAA A000
static void fixup_port5(const MCFixup &Fixup, uint64_t &Value,
MCContext *Ctx = nullptr) {
unsigned_width(5, Value, std::string("port number"), Fixup, Ctx);
static void fixup_port5(const MCFixup &Fixup, uint64_t &Value, MCContext *Ctx) {
ensureUnsignedWidth(5, Value, std::string("port number"), Fixup, Ctx);

Value &= 0x1f;

Expand All @@ -183,9 +85,8 @@ static void fixup_port5(const MCFixup &Fixup, uint64_t &Value,
///
/// Resolves to:
/// 1011 0AAd dddd AAAA
static void fixup_port6(const MCFixup &Fixup, uint64_t &Value,
MCContext *Ctx = nullptr) {
unsigned_width(6, Value, std::string("port number"), Fixup, Ctx);
static void fixup_port6(const MCFixup &Fixup, uint64_t &Value, MCContext *Ctx) {
ensureUnsignedWidth(6, Value, std::string("port number"), Fixup, Ctx);

Value = ((Value & 0x30) << 5) | (Value & 0x0f);
}
Expand All @@ -195,8 +96,8 @@ static void fixup_port6(const MCFixup &Fixup, uint64_t &Value,
/// Resolves to:
/// 1010 ikkk dddd kkkk
static void fixup_lds_sts_16(const MCFixup &Fixup, uint64_t &Value,
MCContext *Ctx = nullptr) {
unsigned_width(7, Value, std::string("immediate"), Fixup, Ctx);
MCContext *Ctx) {
ensureUnsignedWidth(7, Value, std::string("immediate"), Fixup, Ctx);
Value = ((Value & 0x70) << 8) | (Value & 0x0f);
}

Expand All @@ -213,7 +114,7 @@ namespace ldi {
/// 0000 KKKK 0000 KKKK
/// Offset of 0 (so the result isn't left-shifted before application).
static void fixup(unsigned Size, const MCFixup &Fixup, uint64_t &Value,
MCContext *Ctx = nullptr) {
MCContext *Ctx) {
uint64_t upper = Value & 0xf0;
uint64_t lower = Value & 0x0f;

Expand All @@ -223,25 +124,25 @@ static void fixup(unsigned Size, const MCFixup &Fixup, uint64_t &Value,
static void neg(uint64_t &Value) { Value *= -1; }

static void lo8(unsigned Size, const MCFixup &Fixup, uint64_t &Value,
MCContext *Ctx = nullptr) {
MCContext *Ctx) {
Value &= 0xff;
ldi::fixup(Size, Fixup, Value, Ctx);
}

static void hi8(unsigned Size, const MCFixup &Fixup, uint64_t &Value,
MCContext *Ctx = nullptr) {
MCContext *Ctx) {
Value = (Value & 0xff00) >> 8;
ldi::fixup(Size, Fixup, Value, Ctx);
}

static void hh8(unsigned Size, const MCFixup &Fixup, uint64_t &Value,
MCContext *Ctx = nullptr) {
MCContext *Ctx) {
Value = (Value & 0xff0000) >> 16;
ldi::fixup(Size, Fixup, Value, Ctx);
}

static void ms8(unsigned Size, const MCFixup &Fixup, uint64_t &Value,
MCContext *Ctx = nullptr) {
MCContext *Ctx) {
Value = (Value & 0xff000000) >> 24;
ldi::fixup(Size, Fixup, Value, Ctx);
}
Expand All @@ -263,13 +164,9 @@ void AVRAsmBackend::adjustFixupValue(const MCFixup &Fixup,
default:
llvm_unreachable("unhandled fixup");
case AVR::fixup_7_pcrel:
adjust::fixup_7_pcrel(Size, Fixup, Value, Ctx);
break;
case AVR::fixup_13_pcrel:
adjust::fixup_13_pcrel(Size, Fixup, Value, Ctx);
break;
case AVR::fixup_call:
adjust::fixup_call(Size, Fixup, Value, Ctx);
// Handled by linker, see `shouldForceRelocation()`
break;
case AVR::fixup_ldi:
adjust::ldi::fixup(Size, Fixup, Value, Ctx);
Expand Down Expand Up @@ -330,13 +227,15 @@ void AVRAsmBackend::adjustFixupValue(const MCFixup &Fixup,
adjust::ldi::ms8(Size, Fixup, Value, Ctx);
break;
case AVR::fixup_16:
adjust::unsigned_width(16, Value, std::string("port number"), Fixup, Ctx);
adjust::ensureUnsignedWidth(16, Value, std::string("port number"), Fixup,
Ctx);

Value &= 0xffff;
break;
case AVR::fixup_16_pm:
Value >>= 1; // Flash addresses are always shifted.
adjust::unsigned_width(16, Value, std::string("port number"), Fixup, Ctx);
adjust::ensureUnsignedWidth(16, Value, std::string("port number"), Fixup,
Ctx);

Value &= 0xffff;
break;
Expand Down Expand Up @@ -517,8 +416,6 @@ bool AVRAsmBackend::shouldForceRelocation(const MCAssembler &Asm,
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_call:
return true;
}
Expand Down
74 changes: 38 additions & 36 deletions llvm/test/CodeGen/AVR/branch-relaxation-long.ll
View file
Original file line number Diff line number Diff line change
@@ -1,24 +1,25 @@
; RUN: llc < %s -march=avr -mattr=avr3 | FileCheck %s
; RUN: llc < %s -march=avr -mattr=avr2 | FileCheck --check-prefix=AVR2 %s
; RUN: llc < %s -march=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 -march=avr -mcpu=avr3 -filetype=obj -o - | llvm-objdump --mcpu=avr3 -dr --no-show-raw-insn --no-leading-addr - | FileCheck --check-prefix=AVR3 %s

; CHECK-LABEL: relax_to_jmp:
; CHECK: cpi r{{[0-9]+}}, 0
; CHECK: brne [[BB1:.LBB[0-9]+_[0-9]+]]
; CHECK: jmp [[BB2:.LBB[0-9]+_[0-9]+]]
; CHECK: [[BB1]]:
; CHECK: nop
; CHECK: [[BB2]]:
; AVR25: <relax_to_jmp>:
; AVR25-NEXT: andi r24, 0x1
; AVR25-NEXT: cpi r24, 0x0
; AVR25-NEXT: brne .+0
; AVR25-NEXT: R_AVR_7_PCREL .text+0x8
; AVR25-NEXT: rjmp .+0
; AVR25-NEXT: R_AVR_13_PCREL .text+0x100c
; AVR25: ldi r24, 0x3
; AVR25-NEXT: ret

;; A `RJMP` is generated instead of expected `JMP` for AVR2,
;; and it is up to the linker to report 'out of range' or
;; 'exceed flash maximum size'.
; AVR2-LABEL: relax_to_jmp:
; AVR2: cpi r{{[0-9]+}}, 0
; AVR2: brne [[BB1:.LBB[0-9]+_[0-9]+]]
; AVR2: rjmp [[BB2:.LBB[0-9]+_[0-9]+]]
; AVR2: [[BB1]]:
; AVR2: nop
; AVR2: [[BB2]]:
; AVR3: <relax_to_jmp>:
; AVR3-NEXT: andi r24, 0x1
; AVR3-NEXT: cpi r24, 0x0
; AVR3-NEXT: brne .+0
; AVR3-NEXT: R_AVR_7_PCREL .text+0xa
; AVR3-NEXT: jmp 0x0
; AVR3-NEXT: R_AVR_CALL .text+0x100e
; AVR3: ldi r24, 0x3
; AVR3-NEXT: ret

define i8 @relax_to_jmp(i1 %a) {
entry-block:
Expand Down Expand Up @@ -2081,24 +2082,25 @@ finished:
ret i8 3
}

; CHECK-LABEL: relax_to_jmp_backwards:
; CHECK: [[BB1:.LBB[0-9]+_[0-9]+]]
; CHECK: nop
; CHECK: cpi r{{[0-9]+}}, 0
; CHECK: breq [[BB2:.LBB[0-9]+_[0-9]+]]
; CHECK: jmp [[BB1]]
; CHECK: [[BB2]]:
; AVR25: <relax_to_jmp_backwards>:
; AVR25-NEXT: andi r24, 0x1
; AVR25: cpi r24, 0x0
; AVR25-NEXT: breq .+0
; AVR25-NEXT: R_AVR_7_PCREL .text+0x201c
; AVR25-NEXT: rjmp .+0
; AVR25-NEXT: R_AVR_13_PCREL .text+0x1012
; AVR25: ldi r24, 0x3
; AVR25-NEXT: ret

;; A `RJMP` is generated instead of expected `JMP` for AVR2,
;; and it is up to the linker to report 'out of range' or
;; 'exceed flash maximum size'.
; AVR2-LABEL: relax_to_jmp_backwards:
; AVR2: [[BB1:.LBB[0-9]+_[0-9]+]]
; AVR2: nop
; AVR2: cpi r{{[0-9]+}}, 0
; AVR2: breq [[BB2:.LBB[0-9]+_[0-9]+]]
; AVR2: rjmp [[BB1]]
; AVR2: [[BB2]]:
; AVR3: <relax_to_jmp_backwards>:
; AVR3-NEXT: andi r24, 0x1
; AVR3: cpi r24, 0x0
; AVR3-NEXT: breq .+0
; AVR3-NEXT: R_AVR_7_PCREL .text+0x2020
; AVR3-NEXT: jmp 0x0
; AVR3-NEXT: R_AVR_CALL .text+0x1014
; AVR3: ldi r24, 0x3
; AVR3-NEXT: ret

define i8 @relax_to_jmp_backwards(i1 %a) {
entry-block:
Expand Down
5 changes: 3 additions & 2 deletions llvm/test/CodeGen/AVR/jmp.ll
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,11 @@ bb2:

declare i8 @bar(i8);

; CHECK: rcall .-2
; CHECK: rcall .+0
; CHECK-NEXT: 00000000: R_AVR_13_PCREL bar
; CHECK-NEXT: cpi r24, 0x7b
; CHECK-NEXT: brne .+4
; CHECK-NEXT: brne .+0
; CHECK-NEXT: 00000004: R_AVR_7_PCREL .text+0xa
; CHECK-NEXT: ldi r24, 0x64
; CHECK-NEXT: ret
; CHECK-NEXT: ldi r24, 0xc8
Expand Down
Loading

0 comments on commit d519636

Please sign in to comment.