Skip to content
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.

Sign up for GitHub

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

riscv64: Use PCRelLo12I relocation on Loads #6938

Merged
merged 4 commits into from
Aug 31, 2023
Merged

riscv64: Use PCRelLo12I relocation on Loads #6938

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1923d3e
riscv64: Use `PCRelLo12I` relocation on Loads
afonso360 Aug 30, 2023
ce91409
riscv64: Strenghten pattern matching when emitting Load's
afonso360 Aug 31, 2023
d900d85
riscv64: Clarify some of the load address logic
afonso360 Aug 31, 2023
0b13b09
riscv64: Even stronger matching
afonso360 Aug 31, 2023
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
12 changes: 12 additions & 0 deletions cranelift/codegen/src/isa/riscv64/inst/args.rs
View file
Open in desktop
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
46 changes: 37 additions & 9 deletions cranelift/codegen/src/isa/riscv64/inst/emit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -620,18 +620,48 @@ 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), _) => (base, imm12),
Copy link
Member

@alexcrichton alexcrichton Aug 31, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this grow a None instead of _ to assert that label+base doesn't show up? Or, alternatively, could this runtime assert that label is None here?


// 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(_))
| (None, None, Some(_)) => {
unreachable!("Invalid load address")
}
};

let srcloc = state.cur_srcloc();
Expand All @@ -650,8 +680,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
Open in desktop
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
Open in desktop
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