From 3195c205df35fc9bb4d1a86413a0c570e9113ce7 Mon Sep 17 00:00:00 2001 From: Hamy Ratoanina Date: Fri, 8 Dec 2023 17:57:45 -0500 Subject: [PATCH] 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 | 9 +++- evm/src/cpu/columns/ops.rs | 6 +-- evm/src/cpu/contextops.rs | 3 +- evm/src/cpu/cpu_stark.rs | 18 +++++++- evm/src/cpu/decode.rs | 83 ++++++++++++++++++++++++++--------- evm/src/cpu/gas.rs | 3 +- evm/src/cpu/stack.rs | 10 +---- evm/src/witness/transition.rs | 9 ++-- 8 files changed, 94 insertions(+), 47 deletions(-) diff --git a/evm/src/cpu/byte_unpacking.rs b/evm/src/cpu/byte_unpacking.rs index be3ed59d..aed3bc4e 100644 --- a/evm/src/cpu/byte_unpacking.rs +++ b/evm/src/cpu/byte_unpacking.rs @@ -13,7 +13,9 @@ pub(crate) fn eval_packed( nv: &CpuColumnsView

, yield_constr: &mut ConstraintConsumer

, ) { - 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, const D: usize>( nv: &CpuColumnsView>, yield_constr: &mut RecursiveConstraintConsumer, ) { - 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. diff --git a/evm/src/cpu/columns/ops.rs b/evm/src/cpu/columns/ops.rs index 73f501d7..250c67af 100644 --- a/evm/src/cpu/columns/ops.rs +++ b/evm/src/cpu/columns/ops.rs @@ -34,10 +34,8 @@ pub(crate) struct OpsColumnsView { pub dup_swap: T, /// Combines GET_CONTEXT and SET_CONTEXT flags. pub context_op: T, - /// Flag for MSTORE_32BYTES. - pub mstore_32bytes: T, - /// Flag for MLOAD_32BYTES. - pub mload_32bytes: T, + /// Combines MSTORE_32BYTES and MLOAD_32BYTES. + pub m_op_32bytes: T, /// Flag for EXIT_KERNEL. pub exit_kernel: T, /// Combines MSTORE_GENERAL and MLOAD_GENERAL flags. diff --git a/evm/src/cpu/contextops.rs b/evm/src/cpu/contextops.rs index 9f62ade0..77eb3be9 100644 --- a/evm/src/cpu/contextops.rs +++ b/evm/src/cpu/contextops.rs @@ -30,8 +30,7 @@ const KEEPS_CONTEXT: OpsColumnsView = OpsColumnsView { push: true, dup_swap: true, context_op: false, - mstore_32bytes: true, - mload_32bytes: true, + m_op_32bytes: true, exit_kernel: true, m_op_general: true, syscall: true, diff --git a/evm/src/cpu/cpu_stark.rs b/evm/src/cpu/cpu_stark.rs index 5eaa0c3f..b045b7e0 100644 --- a/evm/src/cpu/cpu_stark.rs +++ b/evm/src/cpu/cpu_stark.rs @@ -125,8 +125,15 @@ pub(crate) fn ctl_data_byte_packing() -> Vec> { } /// 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() -> Filter { - 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() -> Vec> { } /// 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() -> Filter { - 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`. diff --git a/evm/src/cpu/decode.rs b/evm/src/cpu/decode.rs index 90e49ec0..4e6dbfbd 100644 --- a/evm/src/cpu/decode.rs +++ b/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 - (0x60, 5, false, COL_MAP.op.push), // 0x60-0x7f - (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 - (0xf8, 0, true, COL_MAP.op.mload_32bytes), + (0x56, 1, false, COL_MAP.op.jumps), // 0x56-0x57 + (0x60, 5, false, COL_MAP.op.push), // 0x60-0x7f + (0x80, 5, false, COL_MAP.op.dup_swap), // 0x80-0x9f + (0xf6, 1, true, COL_MAP.op.context_op), //0xf6-0xf7 (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( 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 - .opcode_bits - .into_iter() - .enumerate() - .map(|(i, bit)| bit * P::Scalar::from_canonical_u64(1 << i)) - .sum(); + let opcode = opcode_high_bits(8); 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( * (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, const D: usize>( + builder: &mut plonky2::plonk::circuit_builder::CircuitBuilder, + lv: &CpuColumnsView>, + num_high_bits: usize, +) -> ExtensionTarget { + 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, const D: usize>( } // Manually check lv.op.m_op_constr - let opcode = lv - .opcode_bits - .into_iter() - .rev() - .fold(builder.zero_extension(), |cumul, bit| { - builder.mul_const_add_extension(F::TWO, cumul, bit) - }); + let opcode = opcode_high_bits_circuit(builder, lv, 8); 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, 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); } diff --git a/evm/src/cpu/gas.rs b/evm/src/cpu/gas.rs index 33ef9364..af86fb47 100644 --- a/evm/src/cpu/gas.rs +++ b/evm/src/cpu/gas.rs @@ -33,8 +33,7 @@ const SIMPLE_OPCODES: OpsColumnsView> = OpsColumnsView { push: G_VERYLOW, dup_swap: G_VERYLOW, context_op: KERNEL_ONLY_INSTR, - mstore_32bytes: KERNEL_ONLY_INSTR, - mload_32bytes: KERNEL_ONLY_INSTR, + m_op_32bytes: KERNEL_ONLY_INSTR, exit_kernel: None, m_op_general: KERNEL_ONLY_INSTR, syscall: None, diff --git a/evm/src/cpu/stack.rs b/evm/src/cpu/stack.rs index 62c0dd30..b834482e 100644 --- a/evm/src/cpu/stack.rs +++ b/evm/src/cpu/stack.rs @@ -34,8 +34,7 @@ pub(crate) const MIGHT_OVERFLOW: OpsColumnsView = OpsColumnsView { push: true, dup_swap: true, context_op: false, - mload_32bytes: false, - mstore_32bytes: false, + m_op_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> = OpsCol }), dup_swap: None, context_op: None, - mload_32bytes: Some(StackBehavior { - num_pops: 4, - pushes: true, - disable_other_channels: false, - }), - mstore_32bytes: Some(StackBehavior { + m_op_32bytes: Some(StackBehavior { num_pops: 4, pushes: true, disable_other_channels: false, diff --git a/evm/src/witness/transition.rs b/evm/src/witness/transition.rs index 6cdc5f6a..6f720a3b 100644 --- a/evm/src/witness/transition.rs +++ b/evm/src/witness/transition.rs @@ -179,8 +179,7 @@ fn fill_op_flag(op: Operation, row: &mut CpuColumnsView) { 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::Mstore32Bytes(_) => &mut flags.mstore_32bytes, + Operation::Mload32Bytes | Operation::Mstore32Bytes(_) => &mut flags.m_op_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 { Operation::Jump => JUMP_OP, Operation::Jumpi => JUMPI_OP, Operation::GetContext | Operation::SetContext => None, - Operation::Mload32Bytes => STACK_BEHAVIORS.mload_32bytes, - Operation::Mstore32Bytes(_) => STACK_BEHAVIORS.mstore_32bytes, + Operation::Mload32Bytes | Operation::Mstore32Bytes(_) => STACK_BEHAVIORS.m_op_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::Mstore32Bytes(_) => MIGHT_OVERFLOW.mstore_32bytes, + Operation::Mload32Bytes | Operation::Mstore32Bytes(_) => MIGHT_OVERFLOW.m_op_32bytes, Operation::ExitKernel => MIGHT_OVERFLOW.exit_kernel, Operation::MloadGeneral | Operation::MstoreGeneral => MIGHT_OVERFLOW.m_op_general, }