chore: General Sys phantom (#1867)

Author: nyunyunyunyu

crates/vm/src/arch/extensions.rs

@@ -17,7 +17,8 @@ use openvm_circuit_primitives::{
 };
 use openvm_circuit_primitives_derive::{Chip, ChipUsageGetter};
 use openvm_instructions::{
-    program::Program, LocalOpcode, PhantomDiscriminant, PublishOpcode, SystemOpcode, VmOpcode,
+    program::Program, LocalOpcode, PhantomDiscriminant, PublishOpcode, SysPhantom, SysPhantom::Nop,
+    SystemOpcode, VmOpcode,
 };
 use openvm_stark_backend::{
     config::{Domain, StarkGenericConfig},
@@ -49,7 +50,9 @@ use crate::{
             MemoryController, MemoryImage, BOUNDARY_AIR_OFFSET, MERKLE_AIR_OFFSET,
         },
         native_adapter::{NativeAdapterAir, NativeAdapterStep},
-        phantom::PhantomChip,
+        phantom::{
+            CycleEndPhantomExecutor, CycleStartPhantomExecutor, NopPhantomExecutor, PhantomChip,
+        },
         poseidon2::Poseidon2PeripheryChip,
         program::{ProgramBus, ProgramChip},
         public_values::{
@@ -640,7 +643,22 @@ impl<F: PrimeField32> SystemComplex<F> {
             inventory.add_periphery_chip(chip);
         }
         let phantom_opcode = SystemOpcode::PHANTOM.global_opcode();
-        let phantom_chip = PhantomChip::new(execution_bus, program_bus, SystemOpcode::CLASS_OFFSET);
+        let mut phantom_chip =
+            PhantomChip::new(execution_bus, program_bus, SystemOpcode::CLASS_OFFSET);
+        // Use NopPhantomExecutor so the discriminant is set but `DebugPanic` is handled specially.
+        phantom_chip.add_sub_executor(
+            NopPhantomExecutor,
+            PhantomDiscriminant(SysPhantom::DebugPanic as u16),
+        );
+        phantom_chip.add_sub_executor(NopPhantomExecutor, PhantomDiscriminant(Nop as u16));
+        phantom_chip.add_sub_executor(
+            CycleStartPhantomExecutor,
+            PhantomDiscriminant(SysPhantom::CtStart as u16),
+        );
+        phantom_chip.add_sub_executor(
+            CycleEndPhantomExecutor,
+            PhantomDiscriminant(SysPhantom::CtEnd as u16),
+        );
         inventory
             .add_executor(RefCell::new(phantom_chip), [phantom_opcode])
             .unwrap();

crates/vm/src/arch/interpreter.rs

@@ -7,10 +7,7 @@ use std::{
 use itertools::Itertools;
 use openvm_circuit_primitives_derive::AlignedBytesBorrow;
 use openvm_instructions::{
-    exe::VmExe,
-    instruction::Instruction,
-    program::{Program, DEFAULT_PC_STEP},
-    LocalOpcode, SysPhantom, SystemOpcode,
+    exe::VmExe, instruction::Instruction, program::Program, LocalOpcode, SystemOpcode,
 };
 use openvm_stark_backend::p3_field::{Field, PrimeField32};
 use rand::{rngs::StdRng, SeedableRng};
@@ -291,21 +288,6 @@ unsafe fn terminate_execute_e12_impl<F: PrimeField32, CTX: E1ExecutionCtx>(
     vm_state.exit_code = Ok(Some(pre_compute.exit_code));
 }
 
-unsafe fn debug_panic_execute_e12_impl<F: PrimeField32, CTX: E1ExecutionCtx>(
-    _pre_compute: &[u8],
-    vm_state: &mut VmSegmentState<F, CTX>,
-) {
-    vm_state.exit_code = Err(ExecutionError::Fail { pc: vm_state.pc });
-}
-
-unsafe fn nop_execute_e12_impl<F: PrimeField32, CTX: E1ExecutionCtx>(
-    _pre_compute: &[u8],
-    vm_state: &mut VmSegmentState<F, CTX>,
-) {
-    vm_state.pc += DEFAULT_PC_STEP;
-    vm_state.instret += 1;
-}
-
 fn get_pre_compute_max_size<F: PrimeField32, E: InsExecutorE1<F>, P>(
     program: &Program<F>,
     chip_complex: &VmChipComplex<F, E, P>,
@@ -361,15 +343,8 @@ fn get_e2_pre_compute_max_size<F: PrimeField32, E: InsExecutorE2<F>, P>(
 }
 
 fn system_opcode_pre_compute_size<F: PrimeField32>(inst: &Instruction<F>) -> Option<usize> {
-    let discriminant = SysPhantom::from_repr(inst.c.as_canonical_u32() as u16);
     if inst.opcode == SystemOpcode::TERMINATE.global_opcode() {
         return Some(size_of::<TerminatePreCompute>());
-    } else if inst.opcode == SystemOpcode::PHANTOM.global_opcode() && discriminant.is_some() {
-        let discriminant = discriminant.unwrap();
-        let ret = match discriminant {
-            SysPhantom::DebugPanic | SysPhantom::Nop | SysPhantom::CtStart | SysPhantom::CtEnd => 0,
-        };
-        return Some(ret);
     }
     None
 }
@@ -482,16 +457,7 @@ fn get_system_opcode_handler<F: PrimeField32, Ctx: E1ExecutionCtx>(
     inst: &Instruction<F>,
     buf: &mut [u8],
 ) -> Option<ExecuteFunc<F, Ctx>> {
-    let discriminant = SysPhantom::from_repr(inst.c.as_canonical_u32() as u16);
-    if inst.opcode == SystemOpcode::PHANTOM.global_opcode() && discriminant.is_some() {
-        let discriminant = discriminant.unwrap();
-        return Some(match discriminant {
-            SysPhantom::Nop => nop_execute_e12_impl,
-            SysPhantom::DebugPanic => debug_panic_execute_e12_impl,
-            SysPhantom::CtStart => nop_execute_e12_impl,
-            SysPhantom::CtEnd => nop_execute_e12_impl,
-        });
-    } else if inst.opcode == SystemOpcode::TERMINATE.global_opcode() {
+    if inst.opcode == SystemOpcode::TERMINATE.global_opcode() {
         let pre_compute: &mut TerminatePreCompute = buf.borrow_mut();
         pre_compute.exit_code = inst.c.as_canonical_u32();
         return Some(terminate_execute_e12_impl);

crates/vm/src/system/phantom/execution.rs

@@ -70,7 +70,7 @@ unsafe fn execute_e12_impl<F: PrimeField32, CTX: E1ExecutionCtx>(
     vm_state: &mut VmSegmentState<F, CTX>,
 ) {
     let sub_executor = &*pre_compute.sub_executor;
-    execute_impl(
+    if let Err(e) = execute_impl(
         PhantomStateMut {
             pc: &mut vm_state.pc,
             memory: &mut vm_state.memory,
@@ -79,8 +79,10 @@ unsafe fn execute_e12_impl<F: PrimeField32, CTX: E1ExecutionCtx>(
         },
         &pre_compute.operands,
         sub_executor.as_ref(),
-    )
-    .unwrap();
+    ) {
+        vm_state.exit_code = Err(e);
+        return;
+    }
     vm_state.pc += DEFAULT_PC_STEP;
     vm_state.instret += 1;
 }
@@ -120,23 +122,26 @@ where
     let discriminant = PhantomDiscriminant(c as u16);
     // If not a system phantom sub-instruction (which is handled in
     // ExecutionSegment), look for a phantom sub-executor to handle it.
-    if SysPhantom::from_repr(discriminant.0).is_none() {
-        sub_executor
-            .phantom_execute(
-                state.memory,
-                state.streams,
-                state.rng,
-                discriminant,
-                a,
-                b,
-                (c >> 16) as u16,
-            )
-            .map_err(|e| ExecutionError::Phantom {
-                pc: *state.pc,
-                discriminant,
-                inner: e,
-            })?;
+    if let Some(discr) = SysPhantom::from_repr(discriminant.0) {
+        if discr == SysPhantom::DebugPanic {
+            return Err(ExecutionError::Fail { pc: *state.pc });
+        }
     }
+    sub_executor
+        .phantom_execute(
+            state.memory,
+            state.streams,
+            state.rng,
+            discriminant,
+            a,
+            b,
+            (c >> 16) as u16,
+        )
+        .map_err(|e| ExecutionError::Phantom {
+            pc: *state.pc,
+            discriminant,
+            inner: e,
+        })?;
 
     Ok(())
 }

crates/vm/src/system/phantom/mod.rs

@@ -31,6 +31,7 @@ use crate::{
         PcIncOrSet, PhantomSubExecutor, Streams,
     },
     system::{
+        memory::online::GuestMemory,
         phantom::execution::{execute_impl, PhantomOperands, PhantomStateMut},
         program::ProgramBus,
     },
@@ -211,3 +212,57 @@ where
         AirProofInput::simple(trace, vec![])
     }
 }
+
+pub struct NopPhantomExecutor;
+pub struct CycleStartPhantomExecutor;
+pub struct CycleEndPhantomExecutor;
+
+impl<F> PhantomSubExecutor<F> for NopPhantomExecutor {
+    #[inline(always)]
+    fn phantom_execute(
+        &self,
+        _memory: &GuestMemory,
+        _streams: &mut Streams<F>,
+        _rng: &mut StdRng,
+        _discriminant: PhantomDiscriminant,
+        _a: u32,
+        _b: u32,
+        _c_upper: u16,
+    ) -> eyre::Result<()> {
+        Ok(())
+    }
+}
+
+impl<F> PhantomSubExecutor<F> for CycleStartPhantomExecutor {
+    #[inline(always)]
+    fn phantom_execute(
+        &self,
+        _memory: &GuestMemory,
+        _streams: &mut Streams<F>,
+        _rng: &mut StdRng,
+        _discriminant: PhantomDiscriminant,
+        _a: u32,
+        _b: u32,
+        _c_upper: u16,
+    ) -> eyre::Result<()> {
+        // TODO: implement cycle tracker for E1/E2
+        Ok(())
+    }
+}
+
+impl<F> PhantomSubExecutor<F> for CycleEndPhantomExecutor {
+    #[inline(always)]
+    fn phantom_execute(
+        &self,
+        _memory: &GuestMemory,
+        _streams: &mut Streams<F>,
+        _rng: &mut StdRng,
+        _discriminant: PhantomDiscriminant,
+        _a: u32,
+        _b: u32,
+        _c_upper: u16,
+    ) -> eyre::Result<()> {
+        // TODO: implement cycle tracker for E1/E2
+        Ok(())
+    }
+}