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

@@ -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())
 }

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 {

cranelift/codegen/src/isa/riscv64/inst/mod.rs

@@ -181,7 +181,10 @@ pub(crate) fn gen_moves(rd: &[Writable<Reg>], src: &[Reg]) -> SmallInstVec<Inst>
 }
 
 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<Reg>, 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
             }

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<OwnedTargetIsa> {
+    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())
+}