move risc-v instructions stuff to isolated ones

singer-utils/src/constants.rs

@@ -31,62 +31,8 @@ pub enum OpcodeType {
     RISCV = 0xFF,
 }
 
-#[derive(Debug, Clone, Copy, EnumIter)]
-pub enum RV64Opcode {
-    // ADD = 0x33,
-    R = 0x33,
-    I_LOAD = 0x03,
-    I_ARITH = 0x13,
-    S = 0x63,
-    B = 0x23,
-    U_LUI = 0x37,
-    U_AUIPC = 0x7,
-    J = 0x6F,
-    JAR = 0x67,
-    SYS = 0x73,
-}
-
-impl RV64Opcode {
-    // Type R
-    pub const ADD: RV64Opcode = RV64Opcode::R;
-    pub const SUB: RV64Opcode = RV64Opcode::R;
-    pub const SLL: RV64Opcode = RV64Opcode::R;
-    pub const SLT: RV64Opcode = RV64Opcode::R;
-    pub const SLTU: RV64Opcode = RV64Opcode::R;
-    pub const XOR: RV64Opcode = RV64Opcode::R;
-    pub const SRL: RV64Opcode = RV64Opcode::R;
-    pub const SRA: RV64Opcode = RV64Opcode::R;
-    pub const OR: RV64Opcode = RV64Opcode::R;
-    pub const AND: RV64Opcode = RV64Opcode::R;
-    // Type I-LOAD
-    pub const ADDI: RV64Opcode = RV64Opcode::I_LOAD;
-    pub const LB: RV64Opcode = RV64Opcode::I_LOAD;
-    pub const LH: RV64Opcode = RV64Opcode::I_LOAD;
-    pub const LW: RV64Opcode = RV64Opcode::I_LOAD;
-    pub const LBU: RV64Opcode = RV64Opcode::I_LOAD;
-    pub const LHU: RV64Opcode = RV64Opcode::I_LOAD;
-}
-
-impl From<OpcodeType> for u8 {
-    fn from(opcode: OpcodeType) -> Self {
-        opcode as u8
-    }
-}
-
 impl From<OpcodeType> for u64 {
     fn from(opcode: OpcodeType) -> Self {
         opcode as u64
     }
 }
-
-impl From<RV64Opcode> for u8 {
-    fn from(opcode: RV64Opcode) -> Self {
-        opcode as u8
-    }
-}
-
-impl From<RV64Opcode> for u64 {
-    fn from(opcode: RV64Opcode) -> Self {
-        opcode as u64
-    }
-}

singer-utils/src/lib.rs

@@ -4,6 +4,7 @@ pub mod chip_handler;
 pub mod chips;
 pub mod constants;
 pub mod error;
+pub mod riscv_constant;
 pub mod structs;
 pub mod uint;
 

singer-utils/src/riscv_constant.rs

@@ -0,0 +1,171 @@
+use strum_macros::EnumIter;
+
+/// This struct is used to define the opcode format for RISC-V instructions,
+/// containing three main components: the opcode, funct3, and funct7 fields.
+/// These fields are crucial for specifying the
+/// exact operation and variants in the RISC-V instruction set architecture.
+#[derive(Default, Clone)]
+pub struct RvOpcode {
+    pub opcode: RV64IOpcode,
+    pub funct3: u8,
+    pub funct7: u8,
+}
+
+impl From<RvOpcode> for u64 {
+    fn from(opcode: RvOpcode) -> Self {
+        let mut result: u64 = 0;
+        result |= (opcode.opcode as u64) & 0xFF;
+        result |= ((opcode.funct3 as u64) & 0xFF) << 8;
+        result |= ((opcode.funct7 as u64) & 0xFF) << 16;
+        result
+    }
+}
+
+/// List all instruction formats in RV64I which contains
+/// R-Type, I-Type, S-Type, B-Type, U-Type, J-Type and special type.
+#[derive(Debug, Clone)]
+pub enum RV64IOpcode {
+    UNKNOWN = 0x00,
+
+    R = 0x33,
+    I_LOAD = 0x03,
+    I_ARITH = 0x13,
+    S = 0x63,
+    B = 0x23,
+    U_LUI = 0x37,
+    U_AUIPC = 0x7,
+    J = 0x6F,
+    JAR = 0x67,
+    SYS = 0x73,
+}
+
+impl Default for RV64IOpcode {
+    fn default() -> Self {
+        RV64IOpcode::UNKNOWN
+    }
+}
+
+impl From<RV64IOpcode> for u8 {
+    fn from(opcode: RV64IOpcode) -> Self {
+        opcode as u8
+    }
+}
+
+#[derive(Debug, Clone, Copy, EnumIter)]
+pub enum RvInstructions {
+    // Type R
+    ADD = 0,
+    SUB,
+    SLL,
+    SLTU,
+    SLT,
+    XOR,
+    SRL,
+    SRA,
+    OR,
+    AND,
+    // Type I-LOAD
+    LB,
+    LH,
+    LW,
+    LBU,
+    LHU,
+
+    // a workaround to get number of valid instructions
+    END,
+}
+
+impl From<RvInstructions> for RvOpcode {
+    fn from(ins: RvInstructions) -> Self {
+        // Find the instruction format here:
+        // https://fraserinnovations.com/risc-v/risc-v-instruction-set-explanation/
+        match ins {
+            // Type R
+            RvInstructions::ADD => RvOpcode {
+                opcode: RV64IOpcode::R,
+                funct3: 0b000 as u8,
+                funct7: 0,
+            },
+            RvInstructions::SUB => RvOpcode {
+                opcode: RV64IOpcode::R,
+                funct3: 0b000 as u8,
+                funct7: 0b010_0000,
+            },
+            RvInstructions::SLL => RvOpcode {
+                opcode: RV64IOpcode::R,
+                funct3: 0b001 as u8,
+                funct7: 0,
+            },
+            RvInstructions::SLT => RvOpcode {
+                opcode: RV64IOpcode::R,
+                funct3: 0b010 as u8,
+                funct7: 0,
+            },
+            RvInstructions::SLTU => RvOpcode {
+                opcode: RV64IOpcode::R,
+                funct3: 0b011 as u8,
+                funct7: 0,
+            },
+            RvInstructions::XOR => RvOpcode {
+                opcode: RV64IOpcode::R,
+                funct3: 0b100 as u8,
+                funct7: 0,
+            },
+            RvInstructions::SRL => RvOpcode {
+                opcode: RV64IOpcode::R,
+                funct3: 0b101 as u8,
+                funct7: 0,
+            },
+            RvInstructions::SRA => RvOpcode {
+                opcode: RV64IOpcode::R,
+                funct3: 0b101 as u8,
+                funct7: 0b010_0000,
+            },
+            RvInstructions::OR => RvOpcode {
+                opcode: RV64IOpcode::R,
+                funct3: 0b110 as u8,
+                funct7: 0,
+            },
+            RvInstructions::AND => RvOpcode {
+                opcode: RV64IOpcode::R,
+                funct3: 0b111 as u8,
+                funct7: 0,
+            },
+            // Type I-LOAD
+            RvInstructions::LB => RvOpcode {
+                opcode: RV64IOpcode::I_LOAD,
+                funct3: 0b000 as u8,
+                funct7: 0,
+            },
+            RvInstructions::LH => RvOpcode {
+                opcode: RV64IOpcode::I_LOAD,
+                funct3: 0b001 as u8,
+                funct7: 0,
+            },
+            RvInstructions::LW => RvOpcode {
+                opcode: RV64IOpcode::I_LOAD,
+                funct3: 0b010 as u8,
+                funct7: 0,
+            },
+            RvInstructions::LBU => RvOpcode {
+                opcode: RV64IOpcode::I_LOAD,
+                funct3: 0b100 as u8,
+                funct7: 0,
+            },
+            RvInstructions::LHU => RvOpcode {
+                opcode: RV64IOpcode::I_LOAD,
+                funct3: 0b101 as u8,
+                funct7: 0,
+            },
+            // TODO add more
+            _ => RvOpcode::default(),
+        }
+    }
+}
+
+impl From<RvInstructions> for u64 {
+    fn from(ins: RvInstructions) -> Self {
+        let opcode: RvOpcode = ins.into();
+        opcode.into()
+    }
+}

singer/benches/riscv/add.rs

@@ -36,7 +36,7 @@ const RAYON_NUM_THREADS: usize = 8;
 
 use singer::{
     instructions::{
-        riscv::add::AddInstruction, Instruction, InstructionGraph, SingerCircuitBuilder,
+        self, riscv::add::AddInstruction, Instruction, InstructionGraph, SingerCircuitBuilder,
     },
     scheme::GKRGraphProverState,
     CircuitWiresIn, SingerGraphBuilder, SingerParams,
@@ -69,7 +69,7 @@ fn bench_add(c: &mut Criterion) {
     };
     let chip_challenges = ChipChallenges::default();
     let circuit_builder =
-        SingerCircuitBuilder::<E>::new(chip_challenges).expect("circuit builder failed");
+        SingerCircuitBuilder::<E>::new_riscv(chip_challenges).expect("circuit builder failed");
 
     for instance_num_vars in 11..12 {
         // expand more input size once runtime is acceptable
@@ -115,7 +115,7 @@ fn bench_add(c: &mut Criterion) {
                         &mut singer_builder.graph_builder,
                         &mut singer_builder.chip_builder,
                         &circuit_builder.insts_circuits
-                            [<AddInstruction as Instruction<E>>::OPCODE as usize],
+                            [instructions::riscv::add::RV_INSTRUCTION as usize],
                         vec![phase0],
                         &real_challenges,
                         1 << instance_num_vars,

singer/src/instructions.rs

@@ -88,8 +88,6 @@ pub(crate) fn construct_instruction_circuits<E: ExtensionField>(
         0x93 => SwapInstruction::<4>::construct_circuits(challenges),
         0xF3 => ReturnInstruction::construct_circuits(challenges),
 
-        // RISC-V iSA
-        0x33 => riscv::add::AddInstruction::construct_circuits(challenges),
         _ => Ok(vec![]), // TODO: Add more instructions.
     }
 }

singer/src/instructions/riscv/add.rs

@@ -7,8 +7,9 @@ use singer_utils::{
     chip_handler::{
         GlobalStateChipOperations, OAMOperations, ROMOperations, RegisterChipOperations,
     },
-    constants::{OpcodeType, RV64Opcode},
+    constants::OpcodeType,
     register_witness,
+    riscv_constant::RvInstructions,
     structs::{PCUInt, RAMHandler, ROMHandler, RegisterUInt, TSUInt, UInt64},
     uint::{UIntAddSub, UIntCmp},
 };
@@ -51,13 +52,13 @@ register_witness!(
     }
 );
 
+// TODO a workaround to keep the risc-v instruction
+pub const RV_INSTRUCTION: RvInstructions = RvInstructions::ADD;
 impl<E: ExtensionField> Instruction<E> for AddInstruction {
-    // OPCODE is not used in RISC-V case, just for compatibilityj;
+    // OPCODE is not used in RISC-V case, just for compatibility
     const OPCODE: OpcodeType = OpcodeType::RISCV;
     const NAME: &'static str = "ADD";
     fn construct_circuit(challenges: ChipChallenges) -> Result<InstCircuit<E>, ZKVMError> {
-        const RV_OPCODE: RV64Opcode = RV64Opcode::ADD;
-
         let mut circuit_builder = CircuitBuilder::new();
         let (phase0_wire_id, phase0) = circuit_builder.create_witness_in(Self::phase0_size());
         let mut ram_handler = RAMHandler::new(&challenges);
@@ -69,9 +70,8 @@ impl<E: ExtensionField> Instruction<E> for AddInstruction {
         let clk_expr = MixedCell::Cell(clk);
         let zero_cell_ids = [0];
 
-        // TODO: opcode should consist funct3 and funct7
         // Bytecode check for (pc, add)
-        rom_handler.bytecode_with_pc(&mut circuit_builder, pc.values(), RV_OPCODE.into());
+        rom_handler.bytecode_with_pc(&mut circuit_builder, pc.values(), RV_INSTRUCTION.into());
 
         // State update
         ram_handler.state_in(
@@ -189,8 +189,8 @@ mod test {
 
     use crate::{
         instructions::{
-            riscv::add::AddInstruction, ChipChallenges, Instruction, InstructionGraph,
-            SingerCircuitBuilder,
+            riscv::add::{AddInstruction, RV_INSTRUCTION},
+            ChipChallenges, Instruction, InstructionGraph, SingerCircuitBuilder,
         },
         scheme::GKRGraphProverState,
         test::{get_uint_params, test_opcode_circuit, u2vec},
@@ -251,8 +251,6 @@ mod test {
 
         if cfg!(feature = "dbg-opcode") {
             println!("{:?}", inst_circuit.circuit.assert_consts);
-            println!("======");
-            // println!("{:?}", inst_circuit.circuit.layers);
         }
 
         let mut phase0_values_map = BTreeMap::<String, Vec<Goldilocks>>::new();
@@ -327,7 +325,7 @@ mod test {
         let c = GoldilocksExt2::from(66u64);
         let circuit_witness_challenges = vec![c; 3];
 
-        let circuit_witness = test_opcode_circuit(
+        test_opcode_circuit(
             &inst_circuit,
             &phase0_idx_map,
             phase0_witness_size,
@@ -339,7 +337,7 @@ mod test {
     fn bench_add_instruction_helper<E: ExtensionField>(instance_num_vars: usize) {
         let chip_challenges = ChipChallenges::default();
         let circuit_builder =
-            SingerCircuitBuilder::<E>::new(chip_challenges).expect("circuit builder failed");
+            SingerCircuitBuilder::<E>::new_riscv(chip_challenges).expect("circuit builder failed");
         let mut singer_builder = SingerGraphBuilder::<E>::new();
 
         let mut rng = test_rng();
@@ -361,7 +359,7 @@ mod test {
         let _ = AddInstruction::construct_graph_and_witness(
             &mut singer_builder.graph_builder,
             &mut singer_builder.chip_builder,
-            &circuit_builder.insts_circuits[<AddInstruction as Instruction<E>>::OPCODE as usize],
+            &circuit_builder.insts_circuits[RV_INSTRUCTION as usize],
             vec![phase0],
             &real_challenges,
             1 << instance_num_vars,