Merge pull request #1181 from topos-protocol/combine_logic_flags

Combine logic flags

evm/src/cpu/columns/ops.rs

@@ -22,12 +22,9 @@ pub struct OpsColumnsView<T: Copy> {
     pub submod: T,
     pub lt: T,
     pub gt: T,
-    pub eq: T,     // Note: This column must be 0 when is_cpu_cycle = 0.
-    pub iszero: T, // Note: This column must be 0 when is_cpu_cycle = 0.
-    // TODO: combine AND, OR, and XOR into one flag
-    pub and: T,
-    pub or: T,
-    pub xor: T,
+    pub eq: T,       // Note: This column must be 0 when is_cpu_cycle = 0.
+    pub iszero: T,   // Note: This column must be 0 when is_cpu_cycle = 0.
+    pub logic_op: T, // Combines AND, OR and XOR flags.
     pub not: T,
     pub byte: T,
     // TODO: combine SHL and SHR into one flag

evm/src/cpu/control_flow.rs

@@ -8,7 +8,7 @@ use crate::constraint_consumer::{ConstraintConsumer, RecursiveConstraintConsumer
 use crate::cpu::columns::{CpuColumnsView, COL_MAP};
 use crate::cpu::kernel::aggregator::KERNEL;
 
-const NATIVE_INSTRUCTIONS: [usize; 32] = [
+const NATIVE_INSTRUCTIONS: [usize; 30] = [
     COL_MAP.op.add,
     COL_MAP.op.mul,
     COL_MAP.op.sub,
@@ -23,9 +23,7 @@ const NATIVE_INSTRUCTIONS: [usize; 32] = [
     COL_MAP.op.gt,
     COL_MAP.op.eq,
     COL_MAP.op.iszero,
-    COL_MAP.op.and,
-    COL_MAP.op.or,
-    COL_MAP.op.xor,
+    COL_MAP.op.logic_op,
     COL_MAP.op.not,
     COL_MAP.op.shl,
     COL_MAP.op.shr,

evm/src/cpu/cpu_stark.rs

@@ -83,11 +83,14 @@ fn ctl_data_ternops<F: Field>(ops: &[usize], is_shift: bool) -> Vec<Column<F>> {
 }
 
 pub fn ctl_data_logic<F: Field>() -> Vec<Column<F>> {
-    ctl_data_binops(&[COL_MAP.op.and, COL_MAP.op.or, COL_MAP.op.xor])
+    // Instead of taking single columns, we reconstruct the entire opcode value directly.
+    let mut res = vec![Column::le_bits(COL_MAP.opcode_bits)];
+    res.extend(ctl_data_binops(&[]));
+    res
 }
 
 pub fn ctl_filter_logic<F: Field>() -> Column<F> {
-    Column::sum([COL_MAP.op.and, COL_MAP.op.or, COL_MAP.op.xor])
+    Column::single(COL_MAP.op.logic_op)
 }
 
 pub fn ctl_arithmetic_base_rows<F: Field>() -> TableWithColumns<F> {

evm/src/cpu/decode.rs

@@ -22,7 +22,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); 37] = [
+const OPCODES: [(u8, usize, bool, usize); 34] = [
     // (start index of block, number of top bits to check (log2), kernel-only, flag column)
     (0x01, 0, false, COL_MAP.op.add),
     (0x02, 0, false, COL_MAP.op.mul),
@@ -38,9 +38,7 @@ const OPCODES: [(u8, usize, bool, usize); 37] = [
     (0x11, 0, false, COL_MAP.op.gt),
     (0x14, 0, false, COL_MAP.op.eq),
     (0x15, 0, false, COL_MAP.op.iszero),
-    (0x16, 0, false, COL_MAP.op.and),
-    (0x17, 0, false, COL_MAP.op.or),
-    (0x18, 0, false, COL_MAP.op.xor),
+    // AND, OR and XOR flags are handled partly manually here, and partly through the Logic table CTL.
     (0x19, 0, false, COL_MAP.op.not),
     (0x1a, 0, false, COL_MAP.op.byte),
     (0x1b, 0, false, COL_MAP.op.shl),
@@ -144,11 +142,21 @@ pub fn eval_packed_generic<P: PackedField>(
         let flag = lv[flag_col];
         yield_constr.constraint(cycle_filter * flag * (flag - P::ONES));
     }
+    // Manually check the logic_op flag combining AND, OR and XOR.
+    // TODO: This would go away once cycle_filter is replaced by the sum
+    // of all CPU opcode flags.
+    let flag = lv.op.logic_op;
+    yield_constr.constraint(cycle_filter * flag * (flag - P::ONES));
+
     // Now check that they sum to 0 or 1.
+    // Includes the logic_op flag encompassing AND, OR and XOR opcodes.
+    // TODO: This would go away once cycle_filter is replaced by the sum
+    // of all CPU opcode flags.
     let flag_sum: P = OPCODES
         .into_iter()
         .map(|(_, _, _, flag_col)| lv[flag_col])
-        .sum::<P>();
+        .sum::<P>()
+        + lv.op.logic_op;
     yield_constr.constraint(cycle_filter * flag_sum * (flag_sum - P::ONES));
 
     // Finally, classify all opcodes, together with the kernel flag, into blocks
@@ -212,9 +220,20 @@ pub fn eval_ext_circuit<F: RichField + Extendable<D>, const D: usize>(
         let constr = builder.mul_extension(cycle_filter, constr);
         yield_constr.constraint(builder, constr);
     }
+    // Manually check the logic_op flag combining AND, OR and XOR.
+    // TODO: This would go away once cycle_filter is replaced by the sum
+    // of all CPU opcode flags.
+    let flag = lv.op.logic_op;
+    let constr = builder.mul_sub_extension(flag, flag, flag);
+    let constr = builder.mul_extension(cycle_filter, constr);
+    yield_constr.constraint(builder, constr);
+
     // Now check that they sum to 0 or 1.
+    // Includes the logic_op flag encompassing AND, OR and XOR opcodes.
+    // TODO: This would go away once cycle_filter is replaced by the sum
+    // of all CPU opcode flags.
     {
-        let mut flag_sum = builder.zero_extension();
+        let mut flag_sum = lv.op.logic_op;
         for (_, _, _, flag_col) in OPCODES {
             let flag = lv[flag_col];
             flag_sum = builder.add_extension(flag_sum, flag);

evm/src/cpu/gas.rs

@@ -33,9 +33,7 @@ const SIMPLE_OPCODES: OpsColumnsView<Option<u32>> = OpsColumnsView {
     gt: G_VERYLOW,
     eq: G_VERYLOW,
     iszero: G_VERYLOW,
-    and: G_VERYLOW,
-    or: G_VERYLOW,
-    xor: G_VERYLOW,
+    logic_op: G_VERYLOW,
     not: G_VERYLOW,
     byte: G_VERYLOW,
     shl: G_VERYLOW,

evm/src/cpu/stack.rs

@@ -55,9 +55,7 @@ const STACK_BEHAVIORS: OpsColumnsView<Option<StackBehavior>> = OpsColumnsView {
     gt: BASIC_BINARY_OP,
     eq: BASIC_BINARY_OP,
     iszero: BASIC_UNARY_OP,
-    and: BASIC_BINARY_OP,
-    or: BASIC_BINARY_OP,
-    xor: BASIC_BINARY_OP,
+    logic_op: BASIC_BINARY_OP,
     not: BASIC_UNARY_OP,
     byte: BASIC_BINARY_OP,
     shl: Some(StackBehavior {

evm/src/keccak_sponge/keccak_sponge_stark.rs

@@ -103,9 +103,7 @@ pub(crate) fn ctl_looking_logic<F: Field>(i: usize) -> Vec<Column<F>> {
     let cols = KECCAK_SPONGE_COL_MAP;
 
     let mut res = vec![
-        Column::zero(), // is_and
-        Column::zero(), // is_or
-        Column::one(),  // is_xor
+        Column::constant(F::from_canonical_u8(0x18)), // is_xor
     ];
 
     // Input 0 contains some of the sponge's original rate chunks. If this is the last CTL, we won't

evm/src/logic.rs

@@ -52,11 +52,15 @@ pub(crate) mod columns {
 }
 
 pub fn ctl_data<F: Field>() -> Vec<Column<F>> {
-    let mut res = vec![
-        Column::single(columns::IS_AND),
-        Column::single(columns::IS_OR),
-        Column::single(columns::IS_XOR),
-    ];
+    // We scale each filter flag with the associated opcode value.
+    // If a logic operation is happening on the CPU side, the CTL
+    // will enforce that the reconstructed opcode value from the
+    // opcode bits matches.
+    let mut res = vec![Column::linear_combination([
+        (columns::IS_AND, F::from_canonical_u8(0x16)),
+        (columns::IS_OR, F::from_canonical_u8(0x17)),
+        (columns::IS_XOR, F::from_canonical_u8(0x18)),
+    ])];
     res.extend(columns::limb_bit_cols_for_input(columns::INPUT0).map(Column::le_bits));
     res.extend(columns::limb_bit_cols_for_input(columns::INPUT1).map(Column::le_bits));
     res.extend(columns::RESULT.map(Column::single));

evm/src/witness/transition.rs

@@ -160,9 +160,7 @@ fn fill_op_flag<F: Field>(op: Operation, row: &mut CpuColumnsView<F>) {
         Operation::Not => &mut flags.not,
         Operation::Syscall(_, _, _) => &mut flags.syscall,
         Operation::Eq => &mut flags.eq,
-        Operation::BinaryLogic(logic::Op::And) => &mut flags.and,
-        Operation::BinaryLogic(logic::Op::Or) => &mut flags.or,
-        Operation::BinaryLogic(logic::Op::Xor) => &mut flags.xor,
+        Operation::BinaryLogic(_) => &mut flags.logic_op,
         Operation::BinaryArithmetic(arithmetic::BinaryOperator::Add) => &mut flags.add,
         Operation::BinaryArithmetic(arithmetic::BinaryOperator::Mul) => &mut flags.mul,
         Operation::BinaryArithmetic(arithmetic::BinaryOperator::Sub) => &mut flags.sub,