Merge MSTORE_32BYTES and MLOAD_32BYTES columns (#1414)

* Merge MSTORE_32BYTES and MLOAD_32BYTES columns

* Fix circuit functions

* Apply comments

evm/src/cpu/byte_unpacking.rs

@@ -13,7 +13,9 @@ pub(crate) fn eval_packed<P: PackedField>(
     nv: &CpuColumnsView<P>,
     yield_constr: &mut ConstraintConsumer<P>,
 ) {
-    let filter = lv.op.mstore_32bytes;
+    // The MSTORE_32BYTES opcodes are differentiated from MLOAD_32BYTES
+    // by the 5th bit set to 0.
+    let filter = lv.op.m_op_32bytes * (lv.opcode_bits[5] - P::ONES);
     let new_offset = nv.mem_channels[0].value[0];
     let virt = lv.mem_channels[2].value[0];
     // Read len from opcode bits and constrain the pushed new offset.
@@ -32,7 +34,10 @@ pub(crate) fn eval_ext_circuit<F: RichField + Extendable<D>, const D: usize>(
     nv: &CpuColumnsView<ExtensionTarget<D>>,
     yield_constr: &mut RecursiveConstraintConsumer<F, D>,
 ) {
-    let filter = lv.op.mstore_32bytes;
+    // The MSTORE_32BYTES opcodes are differentiated from MLOAD_32BYTES
+    // by the 5th bit set to 0.
+    let filter =
+        builder.mul_sub_extension(lv.op.m_op_32bytes, lv.opcode_bits[5], lv.op.m_op_32bytes);
     let new_offset = nv.mem_channels[0].value[0];
     let virt = lv.mem_channels[2].value[0];
     // Read len from opcode bits and constrain the pushed new offset.

evm/src/cpu/columns/ops.rs

@@ -34,10 +34,8 @@ pub(crate) struct OpsColumnsView<T: Copy> {
     pub dup_swap: T,
     /// Combines GET_CONTEXT and SET_CONTEXT flags.
     pub context_op: T,
-    /// Flag for MSTORE_32BYTES.
+    /// Combines MSTORE_32BYTES and MLOAD_32BYTES.
-    pub mstore_32bytes: T,
+    pub m_op_32bytes: T,
-    /// Flag for MLOAD_32BYTES.
-    pub mload_32bytes: T,
     /// Flag for EXIT_KERNEL.
     pub exit_kernel: T,
     /// Combines MSTORE_GENERAL and MLOAD_GENERAL flags.

evm/src/cpu/contextops.rs

@@ -30,8 +30,7 @@ const KEEPS_CONTEXT: OpsColumnsView<bool> = OpsColumnsView {
     push: true,
     dup_swap: true,
     context_op: false,
-    mstore_32bytes: true,
+    m_op_32bytes: true,
-    mload_32bytes: true,
     exit_kernel: true,
     m_op_general: true,
     syscall: true,

evm/src/cpu/cpu_stark.rs

@@ -125,8 +125,15 @@ pub(crate) fn ctl_data_byte_packing<F: Field>() -> Vec<Column<F>> {
 }
 
 /// CTL filter for the `MLOAD_32BYTES` operation.
+/// MLOAD_32 BYTES is differentiated from MSTORE_32BYTES by its fifth bit set to 1.
 pub(crate) fn ctl_filter_byte_packing<F: Field>() -> Filter<F> {
-    Filter::new_simple(Column::single(COL_MAP.op.mload_32bytes))
+    Filter::new(
+        vec![(
+            Column::single(COL_MAP.op.m_op_32bytes),
+            Column::single(COL_MAP.opcode_bits[5]),
+        )],
+        vec![],
+    )
 }
 
 /// Creates the vector of `Columns` corresponding to the contents of General Purpose channels when calling byte unpacking.
@@ -159,8 +166,15 @@ pub(crate) fn ctl_data_byte_unpacking<F: Field>() -> Vec<Column<F>> {
 }
 
 /// CTL filter for the `MSTORE_32BYTES` operation.
+/// MSTORE_32BYTES is differentiated from MLOAD_32BYTES by its fifth bit set to 0.
 pub(crate) fn ctl_filter_byte_unpacking<F: Field>() -> Filter<F> {
-    Filter::new_simple(Column::single(COL_MAP.op.mstore_32bytes))
+    Filter::new(
+        vec![(
+            Column::single(COL_MAP.op.m_op_32bytes),
+            Column::linear_combination_with_constant([(COL_MAP.opcode_bits[5], -F::ONE)], F::ONE),
+        )],
+        vec![],
+    )
 }
 
 /// Creates the vector of `Columns` corresponding to the contents of the CPU registers when performing a `PUSH`.

evm/src/cpu/decode.rs

@@ -23,7 +23,7 @@ use crate::cpu::columns::{CpuColumnsView, COL_MAP};
 /// behavior.
 /// Note: invalid opcodes are not represented here. _Any_ opcode is permitted to decode to
 /// `is_invalid`. The kernel then verifies that the opcode was _actually_ invalid.
-const OPCODES: [(u8, usize, bool, usize); 9] = [
+const OPCODES: [(u8, usize, bool, usize); 7] = [
     // (start index of block, number of top bits to check (log2), kernel-only, flag column)
     // ADD, MUL, SUB, DIV, MOD, LT, GT and BYTE flags are handled partly manually here, and partly through the Arithmetic table CTL.
     // ADDMOD, MULMOD and SUBMOD flags are handled partly manually here, and partly through the Arithmetic table CTL.
@@ -34,12 +34,10 @@ const OPCODES: [(u8, usize, bool, usize); 9] = [
     // SHL and SHR flags are handled partly manually here, and partly through the Logic table CTL.
     // JUMPDEST and KECCAK_GENERAL are handled manually here.
     (0x49, 0, true, COL_MAP.op.prover_input),
-    (0x56, 1, false, COL_MAP.op.jumps),         // 0x56-0x57
+    (0x56, 1, false, COL_MAP.op.jumps),     // 0x56-0x57
-    (0x60, 5, false, COL_MAP.op.push),          // 0x60-0x7f
+    (0x60, 5, false, COL_MAP.op.push),      // 0x60-0x7f
-    (0x80, 5, false, COL_MAP.op.dup_swap),      // 0x80-0x9f
+    (0x80, 5, false, COL_MAP.op.dup_swap),  // 0x80-0x9f
-    (0xc0, 5, true, COL_MAP.op.mstore_32bytes), //0xc0-0xdf
+    (0xf6, 1, true, COL_MAP.op.context_op), //0xf6-0xf7
-    (0xf6, 1, true, COL_MAP.op.context_op),     //0xf6-0xf7
-    (0xf8, 0, true, COL_MAP.op.mload_32bytes),
     (0xf9, 0, true, COL_MAP.op.exit_kernel),
     // MLOAD_GENERAL and MSTORE_GENERAL flags are handled manually here.
 ];
@@ -47,7 +45,7 @@ const OPCODES: [(u8, usize, bool, usize); 9] = [
 /// List of combined opcodes requiring a special handling.
 /// Each index in the list corresponds to an arbitrary combination
 /// of opcodes defined in evm/src/cpu/columns/ops.rs.
-const COMBINED_OPCODES: [usize; 9] = [
+const COMBINED_OPCODES: [usize; 10] = [
     COL_MAP.op.logic_op,
     COL_MAP.op.fp254_op,
     COL_MAP.op.binary_op,
@@ -57,6 +55,7 @@ const COMBINED_OPCODES: [usize; 9] = [
     COL_MAP.op.jumpdest_keccak_general,
     COL_MAP.op.not_pop,
     COL_MAP.op.pc_push0,
+    COL_MAP.op.m_op_32bytes,
 ];
 
 /// Break up an opcode (which is 8 bits long) into its eight bits.
@@ -133,13 +132,18 @@ pub(crate) fn eval_packed_generic<P: PackedField>(
         yield_constr.constraint(lv[col] * (unavailable + opcode_mismatch));
     }
 
+    let opcode_high_bits = |num_high_bits| -> P {
+        lv.opcode_bits
+            .into_iter()
+            .enumerate()
+            .rev()
+            .take(num_high_bits)
+            .map(|(i, bit)| bit * P::Scalar::from_canonical_u64(1 << i))
+            .sum()
+    };
+
     // Manually check lv.op.m_op_constr
-    let opcode: P = lv
+    let opcode = opcode_high_bits(8);
-        .opcode_bits
-        .into_iter()
-        .enumerate()
-        .map(|(i, bit)| bit * P::Scalar::from_canonical_u64(1 << i))
-        .sum();
     yield_constr.constraint((P::ONES - kernel_mode) * lv.op.m_op_general);
 
     let m_op_constr = (opcode - P::Scalar::from_canonical_usize(0xfb_usize))
@@ -177,6 +181,32 @@ pub(crate) fn eval_packed_generic<P: PackedField>(
         * (opcode - P::Scalar::from_canonical_usize(0x50_usize))
         * lv.op.not_pop;
     yield_constr.constraint(not_pop_op);
+
+    // Manually check lv.op.m_op_32bytes.
+    // Both are kernel-only.
+    yield_constr.constraint((P::ONES - kernel_mode) * lv.op.m_op_32bytes);
+
+    // Check the MSTORE_32BYTES and MLOAD-32BYTES opcodes.
+    let opcode_high_three = opcode_high_bits(3);
+    let op_32bytes = (opcode_high_three - P::Scalar::from_canonical_usize(0xc0_usize))
+        * (opcode - P::Scalar::from_canonical_usize(0xf8_usize))
+        * lv.op.m_op_32bytes;
+    yield_constr.constraint(op_32bytes);
+}
+
+fn opcode_high_bits_circuit<F: RichField + Extendable<D>, const D: usize>(
+    builder: &mut plonky2::plonk::circuit_builder::CircuitBuilder<F, D>,
+    lv: &CpuColumnsView<ExtensionTarget<D>>,
+    num_high_bits: usize,
+) -> ExtensionTarget<D> {
+    lv.opcode_bits
+        .into_iter()
+        .enumerate()
+        .rev()
+        .take(num_high_bits)
+        .fold(builder.zero_extension(), |cumul, (i, bit)| {
+            builder.mul_const_add_extension(F::from_canonical_usize(1 << i), bit, cumul)
+        })
 }
 
 /// Circuit version of `eval_packed_generic`.
@@ -259,13 +289,7 @@ pub(crate) fn eval_ext_circuit<F: RichField + Extendable<D>, const D: usize>(
     }
 
     // Manually check lv.op.m_op_constr
-    let opcode = lv
+    let opcode = opcode_high_bits_circuit(builder, lv, 8);
-        .opcode_bits
-        .into_iter()
-        .rev()
-        .fold(builder.zero_extension(), |cumul, bit| {
-            builder.mul_const_add_extension(F::TWO, cumul, bit)
-        });
 
     let mload_opcode = builder.constant_extension(F::Extension::from_canonical_usize(0xfb_usize));
     let mstore_opcode = builder.constant_extension(F::Extension::from_canonical_usize(0xfc_usize));
@@ -332,4 +356,21 @@ pub(crate) fn eval_ext_circuit<F: RichField + Extendable<D>, const D: usize>(
     let mut not_pop_constr = builder.mul_extension(not_constr, pop_constr);
     not_pop_constr = builder.mul_extension(lv.op.not_pop, not_pop_constr);
     yield_constr.constraint(builder, not_pop_constr);
+
+    // Manually check lv.op.m_op_32bytes.
+    // Both are kernel-only.
+    let constr = builder.mul_extension(is_not_kernel_mode, lv.op.m_op_32bytes);
+    yield_constr.constraint(builder, constr);
+
+    // Check the MSTORE_32BYTES and MLOAD-32BYTES opcodes.
+    let opcode_high_three = opcode_high_bits_circuit(builder, lv, 3);
+    let mstore_32bytes_opcode =
+        builder.constant_extension(F::Extension::from_canonical_usize(0xc0_usize));
+    let mload_32bytes_opcode =
+        builder.constant_extension(F::Extension::from_canonical_usize(0xf8_usize));
+    let mstore_32bytes_constr = builder.sub_extension(opcode_high_three, mstore_32bytes_opcode);
+    let mload_32bytes_constr = builder.sub_extension(opcode, mload_32bytes_opcode);
+    let constr = builder.mul_extension(mstore_32bytes_constr, mload_32bytes_constr);
+    let constr = builder.mul_extension(constr, lv.op.m_op_32bytes);
+    yield_constr.constraint(builder, constr);
 }

evm/src/cpu/gas.rs

@@ -33,8 +33,7 @@ const SIMPLE_OPCODES: OpsColumnsView<Option<u32>> = OpsColumnsView {
     push: G_VERYLOW,
     dup_swap: G_VERYLOW,
     context_op: KERNEL_ONLY_INSTR,
-    mstore_32bytes: KERNEL_ONLY_INSTR,
+    m_op_32bytes: KERNEL_ONLY_INSTR,
-    mload_32bytes: KERNEL_ONLY_INSTR,
     exit_kernel: None,
     m_op_general: KERNEL_ONLY_INSTR,
     syscall: None,

evm/src/cpu/stack.rs

@@ -34,8 +34,7 @@ pub(crate) const MIGHT_OVERFLOW: OpsColumnsView<bool> = OpsColumnsView {
     push: true,
     dup_swap: true,
     context_op: false,
-    mload_32bytes: false,
+    m_op_32bytes: false,
-    mstore_32bytes: false,
     exit_kernel: true, // Doesn't directly push, but the syscall it's returning from might.
     m_op_general: false,
     syscall: false,
@@ -138,12 +137,7 @@ pub(crate) const STACK_BEHAVIORS: OpsColumnsView<Option<StackBehavior>> = OpsCol
     }),
     dup_swap: None,
     context_op: None,
-    mload_32bytes: Some(StackBehavior {
+    m_op_32bytes: Some(StackBehavior {
-        num_pops: 4,
-        pushes: true,
-        disable_other_channels: false,
-    }),
-    mstore_32bytes: Some(StackBehavior {
         num_pops: 4,
         pushes: true,
         disable_other_channels: false,

evm/src/witness/transition.rs

@@ -179,8 +179,7 @@ fn fill_op_flag<F: Field>(op: Operation, row: &mut CpuColumnsView<F>) {
         Operation::Jump | Operation::Jumpi => &mut flags.jumps,
         Operation::Pc | Operation::Push(0) => &mut flags.pc_push0,
         Operation::GetContext | Operation::SetContext => &mut flags.context_op,
-        Operation::Mload32Bytes => &mut flags.mload_32bytes,
+        Operation::Mload32Bytes | Operation::Mstore32Bytes(_) => &mut flags.m_op_32bytes,
-        Operation::Mstore32Bytes(_) => &mut flags.mstore_32bytes,
         Operation::ExitKernel => &mut flags.exit_kernel,
         Operation::MloadGeneral | Operation::MstoreGeneral => &mut flags.m_op_general,
     } = F::ONE;
@@ -211,8 +210,7 @@ const fn get_op_special_length(op: Operation) -> Option<usize> {
         Operation::Jump => JUMP_OP,
         Operation::Jumpi => JUMPI_OP,
         Operation::GetContext | Operation::SetContext => None,
-        Operation::Mload32Bytes => STACK_BEHAVIORS.mload_32bytes,
+        Operation::Mload32Bytes | Operation::Mstore32Bytes(_) => STACK_BEHAVIORS.m_op_32bytes,
-        Operation::Mstore32Bytes(_) => STACK_BEHAVIORS.mstore_32bytes,
         Operation::ExitKernel => STACK_BEHAVIORS.exit_kernel,
         Operation::MloadGeneral | Operation::MstoreGeneral => STACK_BEHAVIORS.m_op_general,
     };
@@ -251,8 +249,7 @@ const fn might_overflow_op(op: Operation) -> bool {
         Operation::Jump | Operation::Jumpi => MIGHT_OVERFLOW.jumps,
         Operation::Pc | Operation::Push(0) => MIGHT_OVERFLOW.pc_push0,
         Operation::GetContext | Operation::SetContext => MIGHT_OVERFLOW.context_op,
-        Operation::Mload32Bytes => MIGHT_OVERFLOW.mload_32bytes,
+        Operation::Mload32Bytes | Operation::Mstore32Bytes(_) => MIGHT_OVERFLOW.m_op_32bytes,
-        Operation::Mstore32Bytes(_) => MIGHT_OVERFLOW.mstore_32bytes,
         Operation::ExitKernel => MIGHT_OVERFLOW.exit_kernel,
         Operation::MloadGeneral | Operation::MstoreGeneral => MIGHT_OVERFLOW.m_op_general,
     }