From 3f07d27dd46e87eafc3d82bb0c7fb973b9c68206 Mon Sep 17 00:00:00 2001 From: Afonso Bordado Date: Thu, 7 Sep 2023 17:03:22 +0100 Subject: [PATCH] riscv64: Check that the minimum set of feature flags is enabled (#6975) * riscv64: Add G extension predicate Also adds descriptions to some existing flags * riscv64: Restrict creating a backend without base features * riscv64: Clarify INSTRUCTION_SIZE constant * riscv64: Update wording on ISA flag error Co-authored-by: bjorn3 <17426603+bjorn3@users.noreply.github.com> * riscv64: Run rustfmt --------- Co-authored-by: bjorn3 <17426603+bjorn3@users.noreply.github.com> --- cranelift/codegen/meta/src/isa/riscv64.rs | 43 +++++++++++++++---- .../codegen/src/isa/riscv64/inst/emit.rs | 7 +-- cranelift/codegen/src/isa/riscv64/inst/mod.rs | 7 ++- cranelift/codegen/src/isa/riscv64/mod.rs | 41 ++++++++++++++---- 4 files changed, 76 insertions(+), 22 deletions(-) diff --git a/cranelift/codegen/meta/src/isa/riscv64.rs b/cranelift/codegen/meta/src/isa/riscv64.rs index f804ff1639ed..4f7483866cea 100644 --- a/cranelift/codegen/meta/src/isa/riscv64.rs +++ b/cranelift/codegen/meta/src/isa/riscv64.rs @@ -1,5 +1,5 @@ use crate::cdsl::isa::TargetIsa; -use crate::cdsl::settings::SettingGroupBuilder; +use crate::cdsl::settings::{PredicateNode, SettingGroupBuilder}; macro_rules! define_zvl_ext { (DEF: $settings:expr, $size:expr) => {{ @@ -38,12 +38,32 @@ pub(crate) fn define() -> TargetIsa { // * 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?", "", true); + let has_m = setting.add_bool( + "has_m", + "has extension M?", + "Integer multiplication and division", + true, + ); + let has_a = setting.add_bool("has_a", "has extension A?", "Atomic instructions", true); + let has_f = setting.add_bool( + "has_f", + "has extension F?", + "Single-precision floating point", + true, + ); + let has_d = setting.add_bool( + "has_d", + "has extension D?", + "Double-precision floating point", + true, + ); + let _has_v = setting.add_bool( + "has_v", + "has extension V?", + "Vector instruction support", + false, + ); + let _has_c = setting.add_bool("has_c", "has extension C?", "Compressed instructions", true); let _has_zbkb = setting.add_bool( "has_zbkb", @@ -76,13 +96,13 @@ pub(crate) fn define() -> TargetIsa { false, ); - let _has_zicsr = setting.add_bool( + let has_zicsr = setting.add_bool( "has_zicsr", "has extension zicsr?", "Zicsr: Control and Status Register (CSR) Instructions", true, ); - let _has_zifencei = setting.add_bool( + let has_zifencei = setting.add_bool( "has_zifencei", "has extension zifencei?", "Zifencei: Instruction-Fetch Fence", @@ -108,5 +128,10 @@ pub(crate) fn define() -> TargetIsa { let (_, zvl32768b) = define_zvl_ext!(setting, 32768, zvl16384b); let (_, _zvl65536b) = define_zvl_ext!(setting, 65536, zvl32768b); + setting.add_predicate( + "has_g", + predicate!(has_m && has_a && has_f && has_d && has_zicsr && has_zifencei), + ); + TargetIsa::new("riscv64", setting.build()) } diff --git a/cranelift/codegen/src/isa/riscv64/inst/emit.rs b/cranelift/codegen/src/isa/riscv64/inst/emit.rs index b65c82e069a5..a61de15543d1 100644 --- a/cranelift/codegen/src/isa/riscv64/inst/emit.rs +++ b/cranelift/codegen/src/isa/riscv64/inst/emit.rs @@ -1150,7 +1150,7 @@ impl MachInstEmit for Inst { Inst::Jalr { rd: writable_zero_reg(), base: tmp1.to_reg(), - offset: Imm12::from_bits((4 * Inst::INSTRUCTION_SIZE) as i16), + offset: Imm12::from_bits((4 * Inst::UNCOMPRESSED_INSTRUCTION_SIZE) as i16), } .emit(&[], sink, emit_info, state); @@ -1161,7 +1161,8 @@ impl MachInstEmit for Inst { // Each entry in the jump table is 2 instructions, so 8 bytes. Check if // we need to emit a jump table here to support that jump. - let distance = (targets.len() * 2 * Inst::INSTRUCTION_SIZE as usize) as u32; + let distance = + (targets.len() * 2 * Inst::UNCOMPRESSED_INSTRUCTION_SIZE as usize) as u32; if sink.island_needed(distance) { sink.emit_island(distance, &mut state.ctrl_plane); } @@ -3072,7 +3073,7 @@ fn emit_return_call_common_sequence( // instructions per word of stack argument space. let new_stack_words = new_stack_arg_size / 8; let insts = 4 + 2 + 2 * new_stack_words; - let space_needed = insts * u32::try_from(Inst::INSTRUCTION_SIZE).unwrap(); + let space_needed = insts * u32::try_from(Inst::UNCOMPRESSED_INSTRUCTION_SIZE).unwrap(); if sink.island_needed(space_needed) { let jump_around_label = sink.get_label(); Inst::Jal { diff --git a/cranelift/codegen/src/isa/riscv64/inst/mod.rs b/cranelift/codegen/src/isa/riscv64/inst/mod.rs index b0c5cd4fa72e..75a836da088c 100644 --- a/cranelift/codegen/src/isa/riscv64/inst/mod.rs +++ b/cranelift/codegen/src/isa/riscv64/inst/mod.rs @@ -181,7 +181,10 @@ pub(crate) fn gen_moves(rd: &[Writable], src: &[Reg]) -> SmallInstVec } impl Inst { - const INSTRUCTION_SIZE: i32 = 4; + /// RISC-V can have multiple instruction sizes. 2 bytes for compressed + /// instructions, 4 for regular instructions, 6 and 8 byte instructions + /// are also being considered. + const UNCOMPRESSED_INSTRUCTION_SIZE: i32 = 4; #[inline] pub(crate) fn load_imm12(rd: Writable, imm: Imm12) -> Inst { @@ -1390,7 +1393,7 @@ impl Inst { let mut buf = String::new(); write!(&mut buf, "auipc {},0; ", rd).unwrap(); write!(&mut buf, "ld {},12({}); ", rd, rd).unwrap(); - write!(&mut buf, "j {}; ", Inst::INSTRUCTION_SIZE + 8).unwrap(); + write!(&mut buf, "j {}; ", Inst::UNCOMPRESSED_INSTRUCTION_SIZE + 8).unwrap(); write!(&mut buf, ".8byte 0x{:x}", imm).unwrap(); buf } diff --git a/cranelift/codegen/src/isa/riscv64/mod.rs b/cranelift/codegen/src/isa/riscv64/mod.rs index 58ba51b14626..1b1c81994fb8 100644 --- a/cranelift/codegen/src/isa/riscv64/mod.rs +++ b/cranelift/codegen/src/isa/riscv64/mod.rs @@ -1,16 +1,16 @@ //! risc-v 64-bit Instruction Set Architecture. use crate::dominator_tree::DominatorTree; -use crate::ir; use crate::ir::{Function, Type}; use crate::isa::riscv64::settings as riscv_settings; -use crate::isa::{Builder as IsaBuilder, FunctionAlignment, TargetIsa}; +use crate::isa::{Builder as IsaBuilder, FunctionAlignment, OwnedTargetIsa, TargetIsa}; use crate::machinst::{ compile, CompiledCode, CompiledCodeStencil, MachInst, MachTextSectionBuilder, Reg, SigSet, TextSectionBuilder, VCode, }; use crate::result::CodegenResult; -use crate::settings as shared_settings; +use crate::settings::{self as shared_settings, Flags}; +use crate::{ir, CodegenError}; use alloc::{boxed::Box, vec::Vec}; use core::fmt; use cranelift_control::ControlPlane; @@ -219,10 +219,35 @@ pub fn isa_builder(triple: Triple) -> IsaBuilder { IsaBuilder { triple, setup: riscv_settings::builder(), - constructor: |triple, shared_flags, builder| { - let isa_flags = riscv_settings::Flags::new(&shared_flags, builder); - let backend = Riscv64Backend::new_with_flags(triple, shared_flags, isa_flags); - Ok(backend.wrapped()) - }, + constructor: isa_constructor, } } + +fn isa_constructor( + triple: Triple, + shared_flags: Flags, + builder: &shared_settings::Builder, +) -> CodegenResult { + let isa_flags = riscv_settings::Flags::new(&shared_flags, builder); + + // The RISC-V backend does not work without at least the G extension enabled. + // The G extension is simply a combination of the following extensions: + // - I: Base Integer Instruction Set + // - M: Integer Multiplication and Division + // - A: Atomic Instructions + // - F: Single-Precision Floating-Point + // - D: Double-Precision Floating-Point + // - Zicsr: Control and Status Register Instructions + // - Zifencei: Instruction-Fetch Fence + // + // Ensure that those combination of features is enabled. + if !isa_flags.has_g() { + return Err(CodegenError::Unsupported( + "The RISC-V Backend currently requires all the features in the G Extension enabled" + .into(), + )); + } + + let backend = Riscv64Backend::new_with_flags(triple, shared_flags, isa_flags); + Ok(backend.wrapped()) +}