Skip to content

Commit

Permalink
riscv64: Use PCRelLo12I relocation on Loads (#6938)
Browse files Browse the repository at this point in the history 
* riscv64: Use `PCRelLo12I` relocation on Loads

* riscv64: Strenghten pattern matching when emitting Load's

* riscv64: Clarify some of the load address logic

* riscv64: Even stronger matching
  • Loading branch information
@afonso360
afonso360 authored Aug 31, 2023
1 parent df8d369 commit 5ec7318
Show file tree
Hide file tree
Showing 14 changed files with 163 additions and 164 deletions.
12 changes: 12 additions & 0 deletions cranelift/codegen/src/isa/riscv64/inst/args.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,18 @@ impl AMode {
}
}

/// Retrieve a MachLabel that corresponds to this addressing mode, if it exists.
pub(crate) fn get_label_with_sink(&self, sink: &mut MachBuffer<Inst>) -> Option<MachLabel> {
match self {
&AMode::Const(addr) => Some(sink.get_label_for_constant(addr)),
&AMode::Label(label) => Some(label),
&AMode::RegOffset(..)
| &AMode::SPOffset(..)
| &AMode::FPOffset(..)
| &AMode::NominalSPOffset(..) => None,
}
}

pub(crate) fn to_string_with_alloc(&self, allocs: &mut AllocationConsumer<'_>) -> String {
format!("{}", self.clone().with_allocs(allocs))
}
Expand Down
47 changes: 38 additions & 9 deletions cranelift/codegen/src/isa/riscv64/inst/emit.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -620,18 +620,49 @@ impl MachInstEmit for Inst {
let base = from.get_base_register();
let offset = from.get_offset_with_state(state);
let offset_imm12 = Imm12::maybe_from_i64(offset);
let label = from.get_label_with_sink(sink);

// TODO: We shouldn't just fall back to `LoadAddr` immediately. For `MachLabel`s
// we should try to emit the `auipc` and add a relocation on this load.
let (addr, imm12) = match (base, offset_imm12) {
// If the offset fits into an imm12 we can directly encode it.
(Some(base), Some(imm12)) => (base, imm12),
// Otherwise load the address it into a reg and load from it.
_ => {
let (addr, imm12) = match (base, offset_imm12, label) {
// When loading from a Reg+Offset, if the offset fits into an imm12 we can directly encode it.
(Some(base), Some(imm12), None) => (base, imm12),

// Otherwise, if the offset does not fit into a imm12, we need to materialize it into a
// register and load from that.
(Some(_), None, None) => {
let tmp = writable_spilltmp_reg();
Inst::LoadAddr { rd: tmp, mem: from }.emit(&[], sink, emit_info, state);
(tmp.to_reg(), Imm12::zero())
}

// If the AMode contains a label we can emit an internal relocation that gets
// resolved with the correct address later.
(None, Some(imm), Some(label)) => {
debug_assert_eq!(imm.as_i16(), 0);

// Get the current PC.
sink.use_label_at_offset(sink.cur_offset(), label, LabelUse::PCRelHi20);
Inst::Auipc {
rd,
imm: Imm20::from_bits(0),
}
.emit(&[], sink, emit_info, state);

// Emit a relocation for the load. This patches the offset into the instruction.
sink.use_label_at_offset(sink.cur_offset(), label, LabelUse::PCRelLo12I);

// Imm12 here is meaningless since it's going to get replaced.
(rd.to_reg(), Imm12::zero())
}

// These cases are impossible with the current AModes that we have. We either
// always have a register, or always have a label. Never both, and never neither.
(None, None, None)
| (None, Some(_), None)
| (Some(_), None, Some(_))
| (Some(_), Some(_), Some(_))
| (None, None, Some(_)) => {
unreachable!("Invalid load address")
}
};

let srcloc = state.cur_srcloc();
Expand All @@ -650,8 +681,6 @@ impl MachInstEmit for Inst {
let offset = to.get_offset_with_state(state);
let offset_imm12 = Imm12::maybe_from_i64(offset);

// TODO: We shouldn't just fall back to `LoadAddr` immediately. For `MachLabel`s
// we should try to emit the `auipc` and add a relocation on this store.
let (addr, imm12) = match (base, offset_imm12) {
// If the offset fits into an imm12 we can directly encode it.
(Some(base), Some(imm12)) => (base, imm12),
Expand Down
78 changes: 36 additions & 42 deletions cranelift/filetests/filetests/isa/riscv64/constants.clif
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,10 @@ block0:
;
; Disassembled:
; block0: ; offset 0x0
; auipc t6, 0
; addi t6, t6, 0x10
; ld a0, 0(t6)
; auipc a0, 0
; ld a0, 0x10(a0)
; ret
; .byte 0x00, 0x00, 0x00, 0x00
; .byte 0x00, 0x00, 0xff, 0xff
; .byte 0x00, 0x00, 0x00, 0x00

Expand All @@ -101,11 +101,11 @@ block0:
;
; Disassembled:
; block0: ; offset 0x0
; auipc t6, 0
; addi t6, t6, 0x10
; ld a0, 0(t6)
; auipc a0, 0
; ld a0, 0x10(a0)
; ret
; .byte 0x00, 0x00, 0x00, 0x00
; .byte 0x00, 0x00, 0x00, 0x00
; .byte 0xff, 0xff, 0x00, 0x00

function %f() -> i64 {
Expand All @@ -121,11 +121,11 @@ block0:
;
; Disassembled:
; block0: ; offset 0x0
; auipc t6, 0
; addi t6, t6, 0x10
; ld a0, 0(t6)
; auipc a0, 0
; ld a0, 0x10(a0)
; ret
; .byte 0x00, 0x00, 0x00, 0x00
; .byte 0x00, 0x00, 0x00, 0x00
; .byte 0x00, 0x00, 0xff, 0xff

function %f() -> i64 {
Expand Down Expand Up @@ -173,10 +173,10 @@ block0:
;
; Disassembled:
; block0: ; offset 0x0
; auipc t6, 0
; addi t6, t6, 0x10
; ld a0, 0(t6)
; auipc a0, 0
; ld a0, 0x10(a0)
; ret
; .byte 0x00, 0x00, 0x00, 0x00
; .byte 0xff, 0xff, 0x00, 0x00
; .byte 0xff, 0xff, 0xff, 0xff

Expand All @@ -193,10 +193,10 @@ block0:
;
; Disassembled:
; block0: ; offset 0x0
; auipc t6, 0
; addi t6, t6, 0x10
; ld a0, 0(t6)
; auipc a0, 0
; ld a0, 0x10(a0)
; ret
; .byte 0x00, 0x00, 0x00, 0x00
; .byte 0xff, 0xff, 0xff, 0xff
; .byte 0x00, 0x00, 0xff, 0xff

Expand All @@ -213,10 +213,10 @@ block0:
;
; Disassembled:
; block0: ; offset 0x0
; auipc t6, 0
; addi t6, t6, 0x10
; ld a0, 0(t6)
; auipc a0, 0
; ld a0, 0x10(a0)
; ret
; .byte 0x00, 0x00, 0x00, 0x00
; .byte 0xff, 0xff, 0xff, 0xff
; .byte 0xff, 0xff, 0x00, 0x00

Expand All @@ -233,10 +233,10 @@ block0:
;
; Disassembled:
; block0: ; offset 0x0
; auipc t6, 0
; addi t6, t6, 0x10
; ld a0, 0(t6)
; auipc a0, 0
; ld a0, 0x10(a0)
; ret
; .byte 0x00, 0x00, 0x00, 0x00
; .byte 0x3a, 0x00, 0x12, 0x12
; .byte 0xa3, 0xf0, 0x4b, 0xf3

Expand All @@ -253,10 +253,10 @@ block0:
;
; Disassembled:
; block0: ; offset 0x0
; auipc t6, 0
; addi t6, t6, 0x10
; ld a0, 0(t6)
; auipc a0, 0
; ld a0, 0x10(a0)
; ret
; .byte 0x00, 0x00, 0x00, 0x00
; .byte 0x00, 0x00, 0xf4, 0x1e
; .byte 0x00, 0x00, 0xe9, 0x12

Expand All @@ -273,10 +273,10 @@ block0:
;
; Disassembled:
; block0: ; offset 0x0
; auipc t6, 0
; addi t6, t6, 0x10
; ld a0, 0(t6)
; auipc a0, 0
; ld a0, 0x10(a0)
; ret
; .byte 0x00, 0x00, 0x00, 0x00
; .byte 0xff, 0xff, 0xf4, 0x1e
; .byte 0xff, 0xff, 0xe9, 0x12

Expand Down Expand Up @@ -325,10 +325,10 @@ block0:
;
; Disassembled:
; block0: ; offset 0x0
; auipc t6, 0
; addi t6, t6, 0x10
; ld a0, 0(t6)
; auipc a0, 0
; ld a0, 0x10(a0)
; ret
; .byte 0x00, 0x00, 0x00, 0x00
; .byte 0xf7, 0xff, 0xff, 0xff
; .byte 0x00, 0x00, 0x00, 0x00

Expand Down Expand Up @@ -362,13 +362,11 @@ block0:
;
; Disassembled:
; block0: ; offset 0x0
; auipc t6, 0
; addi t6, t6, 0x18
; ld t0, 0(t6)
; auipc t0, 0
; ld t0, 0x10(t0)
; fmv.d.x fa0, t0
; ret
; .byte 0x00, 0x00, 0x00, 0x00
; .byte 0x00, 0x00, 0x00, 0x00
; .byte 0x00, 0x00, 0xf0, 0x3f

function %f() -> f32 {
Expand Down Expand Up @@ -403,13 +401,11 @@ block0:
;
; Disassembled:
; block0: ; offset 0x0
; auipc t6, 0
; addi t6, t6, 0x18
; ld t0, 0(t6)
; auipc t0, 0
; ld t0, 0x10(t0)
; fmv.d.x fa0, t0
; ret
; .byte 0x00, 0x00, 0x00, 0x00
; .byte 0x00, 0x00, 0x00, 0x00
; .byte 0x00, 0x00, 0x49, 0x40

function %f() -> f32 {
Expand Down Expand Up @@ -480,13 +476,11 @@ block0:
;
; Disassembled:
; block0: ; offset 0x0
; auipc t6, 0
; addi t6, t6, 0x18
; ld t0, 0(t6)
; auipc t0, 0
; ld t0, 0x10(t0)
; fmv.d.x fa0, t0
; ret
; .byte 0x00, 0x00, 0x00, 0x00
; .byte 0x00, 0x00, 0x00, 0x00
; .byte 0x00, 0x00, 0x30, 0xc0

function %f() -> f32 {
Expand Down
26 changes: 10 additions & 16 deletions cranelift/filetests/filetests/isa/riscv64/fcvt-small.clif
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,9 @@ block0(v0: f32):
; Disassembled:
; block0: ; offset 0x0
; feq.s a0, fa0, fa0
; beqz a0, 0x44
; beqz a0, 0x40
; auipc t6, 0
; addi t6, t6, 0x10
; lw t6, 0(t6)
; lw t6, 0xc(t6)
; j 8
; .byte 0x00, 0x00, 0x80, 0xbf
; fmv.w.x ft3, t6
Expand Down Expand Up @@ -126,10 +125,9 @@ block0(v0: f64):
; Disassembled:
; block0: ; offset 0x0
; feq.d a0, fa0, fa0
; beqz a0, 0x5c
; beqz a0, 0x54
; auipc t6, 0
; addi t6, t6, 0x10
; ld t6, 0(t6)
; ld t6, 0xc(t6)
; j 0xc
; .byte 0x00, 0x00, 0x00, 0x00
; .byte 0x00, 0x00, 0xf0, 0xbf
Expand All @@ -138,8 +136,7 @@ block0(v0: f64):
; beqz a0, 8
; .byte 0x00, 0x00, 0x00, 0x00 ; trap: int_ovf
; auipc t6, 0
; addi t6, t6, 0x10
; ld t6, 0(t6)
; ld t6, 0xc(t6)
; j 0xc
; .byte 0x00, 0x00, 0x00, 0x00
; .byte 0x00, 0x00, 0x70, 0x40
Expand All @@ -166,10 +163,9 @@ block0(v0: f32):
; Disassembled:
; block0: ; offset 0x0
; feq.s a0, fa0, fa0
; beqz a0, 0x44
; beqz a0, 0x40
; auipc t6, 0
; addi t6, t6, 0x10
; lw t6, 0(t6)
; lw t6, 0xc(t6)
; j 8
; .byte 0x00, 0x00, 0x80, 0xbf
; fmv.w.x ft3, t6
Expand Down Expand Up @@ -200,10 +196,9 @@ block0(v0: f64):
; Disassembled:
; block0: ; offset 0x0
; feq.d a0, fa0, fa0
; beqz a0, 0x5c
; beqz a0, 0x54
; auipc t6, 0
; addi t6, t6, 0x10
; ld t6, 0(t6)
; ld t6, 0xc(t6)
; j 0xc
; .byte 0x00, 0x00, 0x00, 0x00
; .byte 0x00, 0x00, 0xf0, 0xbf
Expand All @@ -212,8 +207,7 @@ block0(v0: f64):
; beqz a0, 8
; .byte 0x00, 0x00, 0x00, 0x00 ; trap: int_ovf
; auipc t6, 0
; addi t6, t6, 0x10
; ld t6, 0(t6)
; ld t6, 0xc(t6)
; j 0xc
; .byte 0x00, 0x00, 0x00, 0x00
; .byte 0x00, 0x00, 0xf0, 0x40
Expand Down
Loading

0 comments on commit 5ec7318

Please sign in to comment.