From 3a060e8fd1bf2d2ad92428e8a52cf3c1ffb3bc9c Mon Sep 17 00:00:00 2001 From: Afonso Bordado Date: Wed, 6 Sep 2023 20:16:04 +0100 Subject: [PATCH] riscv64: Use labels for branches whenever possible (#6955) * riscv64: Use label based jump in Icmp * riscv64: Use labels in LoadExtData * riscv64: Use labels in BrTable * riscv64: Delete BranchTarget::offset * riscv64: Enable default extensions --- cranelift/codegen/meta/src/isa/riscv64.rs | 25 ++++++++---- .../codegen/src/isa/riscv64/inst/emit.rs | 39 ++++++++++--------- cranelift/codegen/src/isa/riscv64/inst/mod.rs | 6 +-- 3 files changed, 41 insertions(+), 29 deletions(-) diff --git a/cranelift/codegen/meta/src/isa/riscv64.rs b/cranelift/codegen/meta/src/isa/riscv64.rs index f080aaef3737..f804ff1639ed 100644 --- a/cranelift/codegen/meta/src/isa/riscv64.rs +++ b/cranelift/codegen/meta/src/isa/riscv64.rs @@ -28,12 +28,23 @@ macro_rules! define_zvl_ext { pub(crate) fn define() -> TargetIsa { let mut setting = SettingGroupBuilder::new("riscv64"); - let _has_m = setting.add_bool("has_m", "has extension M?", "", false); - let _has_a = setting.add_bool("has_a", "has extension A?", "", false); - let _has_f = setting.add_bool("has_f", "has extension F?", "", false); - let _has_d = setting.add_bool("has_d", "has extension D?", "", false); + // We target a minimum of riscv64gc. That means that we have the following extensions by default: + // + // * M (integer multiplication and division) + // * A (atomic instructions) + // * F (single-precision floating point) + // * D (double-precision floating point) + // * C (compressed instructions) + // * Zicsr (control and status register instructions) + // * Zifencei (instruction-fetch fence) + + let _has_m = setting.add_bool("has_m", "has extension M?", "", true); + let _has_a = setting.add_bool("has_a", "has extension A?", "", true); + let _has_f = setting.add_bool("has_f", "has extension F?", "", true); + let _has_d = setting.add_bool("has_d", "has extension D?", "", true); let _has_v = setting.add_bool("has_v", "has extension V?", "", false); - let _has_c = setting.add_bool("has_c", "has extension C?", "", false); + let _has_c = setting.add_bool("has_c", "has extension C?", "", true); + let _has_zbkb = setting.add_bool( "has_zbkb", "has extension zbkb?", @@ -69,13 +80,13 @@ pub(crate) fn define() -> TargetIsa { "has_zicsr", "has extension zicsr?", "Zicsr: Control and Status Register (CSR) Instructions", - false, + true, ); let _has_zifencei = setting.add_bool( "has_zifencei", "has extension zifencei?", "Zifencei: Instruction-Fetch Fence", - false, + true, ); // Zvl*: Minimum Vector Length Standard Extensions diff --git a/cranelift/codegen/src/isa/riscv64/inst/emit.rs b/cranelift/codegen/src/isa/riscv64/inst/emit.rs index 8f613ee327c0..b65c82e069a5 100644 --- a/cranelift/codegen/src/isa/riscv64/inst/emit.rs +++ b/cranelift/codegen/src/isa/riscv64/inst/emit.rs @@ -1042,6 +1042,8 @@ impl MachInstEmit for Inst { let tmp2 = allocs.next_writable(tmp2); let ext_index = writable_spilltmp_reg(); + let label_compute_target = sink.get_label(); + // The default target is passed in as the 0th element of `targets` // separate it here for clarity. let default_target = targets[0]; @@ -1094,7 +1096,7 @@ impl MachInstEmit for Inst { .iter() .for_each(|i| i.emit(&[], sink, emit_info, state)); Inst::CondBr { - taken: BranchTarget::offset(Inst::INSTRUCTION_SIZE * 3), + taken: BranchTarget::Label(label_compute_target), not_taken: BranchTarget::zero(), kind: IntegerCompare { kind: IntCC::UnsignedLessThan, @@ -1114,6 +1116,7 @@ impl MachInstEmit for Inst { // Compute the jump table offset. // We need to emit a PC relative offset, + sink.bind_label(label_compute_target, &mut state.ctrl_plane); // Get the current PC. Inst::Auipc { @@ -1363,6 +1366,8 @@ impl MachInstEmit for Inst { let rd = allocs.next_writable(rd); let label_true = sink.get_label(); let label_false = sink.get_label(); + let label_end = sink.get_label(); + Inst::lower_br_icmp( cc, a, @@ -1377,11 +1382,12 @@ impl MachInstEmit for Inst { sink.bind_label(label_true, &mut state.ctrl_plane); Inst::load_imm12(rd, Imm12::TRUE).emit(&[], sink, emit_info, state); Inst::Jal { - dest: BranchTarget::offset(Inst::INSTRUCTION_SIZE * 2), + dest: BranchTarget::Label(label_end), } .emit(&[], sink, emit_info, state); sink.bind_label(label_false, &mut state.ctrl_plane); Inst::load_imm12(rd, Imm12::FALSE).emit(&[], sink, emit_info, state); + sink.bind_label(label_end, &mut state.ctrl_plane); } &Inst::AtomicCas { offset, @@ -1958,33 +1964,30 @@ impl MachInstEmit for Inst { offset, } => { let rd = allocs.next_writable(rd); - // get the current pc. - Inst::Auipc { - rd: rd, - imm: Imm20::from_bits(0), - } - .emit(&[], sink, emit_info, state); - // load the value. + + let label_data = sink.get_label(); + let label_end = sink.get_label(); + + // Load the value from a label Inst::Load { - rd: rd, + rd, op: LoadOP::Ld, flags: MemFlags::trusted(), - from: AMode::RegOffset( - rd.to_reg(), - 12, // auipc load and jal. - I64, - ), + from: AMode::Label(label_data), } .emit(&[], sink, emit_info, state); - // jump over. + + // Jump over the data Inst::Jal { - // jal and abs8 size for 12. - dest: BranchTarget::offset(12), + dest: BranchTarget::Label(label_end), } .emit(&[], sink, emit_info, state); + sink.bind_label(label_data, &mut state.ctrl_plane); sink.add_reloc(Reloc::Abs8, name.as_ref(), offset); sink.put8(0); + + sink.bind_label(label_end, &mut state.ctrl_plane); } &Inst::TrapIfC { rs1, diff --git a/cranelift/codegen/src/isa/riscv64/inst/mod.rs b/cranelift/codegen/src/isa/riscv64/inst/mod.rs index 6c1bf65ab09d..f48008186f39 100644 --- a/cranelift/codegen/src/isa/riscv64/inst/mod.rs +++ b/cranelift/codegen/src/isa/riscv64/inst/mod.rs @@ -127,10 +127,7 @@ impl BranchTarget { pub(crate) fn zero() -> Self { Self::ResolvedOffset(0) } - #[inline] - pub(crate) fn offset(off: i32) -> Self { - Self::ResolvedOffset(off) - } + #[inline] pub(crate) fn is_zero(self) -> bool { match self { @@ -138,6 +135,7 @@ impl BranchTarget { BranchTarget::ResolvedOffset(off) => off == 0, } } + #[inline] pub(crate) fn as_offset(self) -> Option { match self {