chore: sunday cleanup (#363)

Co-authored-by: John Guibas <[email protected]>

core/src/alu/add/mod.rs

@@ -47,7 +47,7 @@ impl<F: PrimeField> MachineAir<F> for AddChip {
         "Add".to_string()
     }
 
-    #[instrument(name = "generate add trace", skip_all)]
+    #[instrument(name = "generate add trace", level = "debug", skip_all)]
     fn generate_trace(
         &self,
         input: &ExecutionRecord,

core/src/alu/bitwise/mod.rs

@@ -49,7 +49,7 @@ impl<F: PrimeField> MachineAir<F> for BitwiseChip {
         "Bitwise".to_string()
     }
 
-    #[instrument(name = "generate bitwise trace", skip_all)]
+    #[instrument(name = "generate bitwise trace", level = "debug", skip_all)]
     fn generate_trace(
         &self,
         input: &ExecutionRecord,

core/src/alu/divrem/mod.rs

@@ -188,7 +188,7 @@ impl<F: PrimeField> MachineAir<F> for DivRemChip {
         "DivRem".to_string()
     }
 
-    #[instrument(name = "generate divrem trace", skip_all)]
+    #[instrument(name = "generate divrem trace", level = "debug", skip_all)]
     fn generate_trace(
         &self,
         input: &ExecutionRecord,

core/src/alu/lt/mod.rs

@@ -82,7 +82,7 @@ impl<F: PrimeField> MachineAir<F> for LtChip {
 
     fn generate_dependencies(&self, _input: &ExecutionRecord, _output: &mut ExecutionRecord) {}
 
-    #[instrument(name = "generate lt trace", skip_all)]
+    #[instrument(name = "generate lt trace", level = "debug", skip_all)]
     fn generate_trace(
         &self,
         input: &ExecutionRecord,

core/src/alu/mul/mod.rs

@@ -117,7 +117,7 @@ impl<F: PrimeField> MachineAir<F> for MulChip {
         "Mul".to_string()
     }
 
-    #[instrument(name = "generate mul trace", skip_all)]
+    #[instrument(name = "generate mul trace", level = "debug", skip_all)]
     fn generate_trace(
         &self,
         input: &ExecutionRecord,

core/src/alu/sll/mod.rs

@@ -97,7 +97,7 @@ impl<F: PrimeField> MachineAir<F> for ShiftLeft {
         "ShiftLeft".to_string()
     }
 
-    #[instrument(name = "generate sll trace", skip_all)]
+    #[instrument(name = "generate sll trace", level = "debug", skip_all)]
     fn generate_trace(
         &self,
         input: &ExecutionRecord,

core/src/alu/sr/mod.rs

@@ -129,7 +129,7 @@ impl<F: PrimeField> MachineAir<F> for ShiftRightChip {
         "ShiftRight".to_string()
     }
 
-    #[instrument(name = "generate sr trace", skip_all)]
+    #[instrument(name = "generate sr trace", level = "debug", skip_all)]
     fn generate_trace(
         &self,
         input: &ExecutionRecord,

core/src/alu/sub/mod.rs

@@ -47,7 +47,7 @@ impl<F: PrimeField> MachineAir<F> for SubChip {
         "Sub".to_string()
     }
 
-    #[instrument(name = "generate sub trace", skip_all)]
+    #[instrument(name = "generate sub trace", level = "debug", skip_all)]
     fn generate_trace(
         &self,
         input: &ExecutionRecord,

core/src/cpu/air/mod.rs

@@ -13,7 +13,7 @@ use crate::cpu::columns::OpcodeSelectorCols;
 use crate::cpu::columns::{CpuCols, NUM_CPU_COLS};
 use crate::cpu::CpuChip;
 use crate::memory::MemoryCols;
-use crate::runtime::{AccessPosition, Opcode};
+use crate::runtime::{MemoryAccessPosition, Opcode};
 
 impl<AB> Air<AB> for CpuChip
 where
@@ -50,7 +50,7 @@ where
         // If they are not immediates, read `b` and `c` from memory.
         builder.constraint_memory_access(
             local.shard,
-            local.clk + AB::F::from_canonical_u32(AccessPosition::B as u32),
+            local.clk + AB::F::from_canonical_u32(MemoryAccessPosition::B as u32),
             local.instruction.op_b[0],
             &local.op_b_access,
             AB::Expr::one() - local.selectors.imm_b,
@@ -61,7 +61,7 @@ where
 
         builder.constraint_memory_access(
             local.shard,
-            local.clk + AB::F::from_canonical_u32(AccessPosition::C as u32),
+            local.clk + AB::F::from_canonical_u32(MemoryAccessPosition::C as u32),
             local.instruction.op_c[0],
             &local.op_c_access,
             AB::Expr::one() - local.selectors.imm_c,
@@ -74,7 +74,7 @@ where
         // we are performing a branch or a store.
         builder.constraint_memory_access(
             local.shard,
-            local.clk + AB::F::from_canonical_u32(AccessPosition::A as u32),
+            local.clk + AB::F::from_canonical_u32(MemoryAccessPosition::A as u32),
             local.instruction.op_a[0],
             &local.op_a_access,
             AB::Expr::one() - local.selectors.is_noop - local.selectors.reg_0_write,
@@ -90,7 +90,7 @@ where
         let memory_columns = local.opcode_specific_columns.memory();
         builder.constraint_memory_access(
             local.shard,
-            local.clk + AB::F::from_canonical_u32(AccessPosition::Memory as u32),
+            local.clk + AB::F::from_canonical_u32(MemoryAccessPosition::Memory as u32),
             memory_columns.addr_aligned,
             &memory_columns.memory_access,
             is_memory_instruction.clone(),

core/src/cpu/event.rs

@@ -1,8 +1,7 @@
 use serde::{Deserialize, Serialize};
 
 use crate::runtime::Instruction;
-
-use super::memory::MemoryRecordEnum;
+use crate::runtime::MemoryRecordEnum;
 
 /// A standard format for describing CPU operations that need to be proven.
 #[derive(Debug, Copy, Clone, Serialize, Deserialize)]

core/src/cpu/mod.rs

@@ -1,11 +1,9 @@
 pub mod air;
 pub mod columns;
 pub mod event;
-pub mod memory;
 pub mod trace;
 
 pub use event::*;
-pub use memory::*;
 
 /// A chip that implements the CPU.
 #[derive(Default)]

core/src/cpu/trace.rs

@@ -4,10 +4,10 @@ use crate::air::MachineAir;
 use crate::alu::{self, AluEvent};
 use crate::bytes::{ByteLookupEvent, ByteOpcode};
 use crate::cpu::columns::CpuCols;
-use crate::cpu::memory::MemoryRecordEnum;
 use crate::disassembler::WORD_SIZE;
 use crate::field::event::FieldEvent;
 use crate::memory::MemoryCols;
+use crate::runtime::MemoryRecordEnum;
 use crate::runtime::{ExecutionRecord, Opcode};
 use hashbrown::HashMap;
 use p3_field::PrimeField;
@@ -25,7 +25,7 @@ impl<F: PrimeField> MachineAir<F> for CpuChip {
         "CPU".to_string()
     }
 
-    #[instrument(name = "generate CPU trace", skip_all)]
+    #[instrument(name = "generate cpu trace", level = "debug", skip_all)]
     fn generate_trace(
         &self,
         input: &ExecutionRecord,
@@ -72,7 +72,7 @@ impl<F: PrimeField> MachineAir<F> for CpuChip {
         trace
     }
 
-    #[instrument(name = "generate CPU dependencies", skip_all)]
+    #[instrument(name = "generate cpu dependencies", level = "debug", skip_all)]
     fn generate_dependencies(&self, input: &ExecutionRecord, output: &mut ExecutionRecord) {
         // Generate the trace rows for each event.
         let chunk_size = std::cmp::max(input.cpu_events.len() / num_cpus::get(), 1);

core/src/field/mod.rs

@@ -56,7 +56,7 @@ impl<F: PrimeField> MachineAir<F> for FieldLtuChip {
 
     fn generate_dependencies(&self, _input: &ExecutionRecord, _output: &mut ExecutionRecord) {}
 
-    #[instrument(name = "generate FieldLTU trace", skip_all)]
+    #[instrument(name = "generate field ltu trace", level = "debug", skip_all)]
     fn generate_trace(
         &self,
         input: &ExecutionRecord,

core/src/lib.rs

@@ -73,7 +73,7 @@ impl SP1Prover {
         let program = Program::from(elf);
         let mut runtime = Runtime::new(program);
         runtime.write_stdin_slice(&stdin.buffer.data);
-        tracing::info_span!("runtime.run(...)").in_scope(|| {
+        tracing::info_span!("execute").in_scope(|| {
             runtime.run();
         });
         let config = BabyBearBlake3::new();
@@ -126,7 +126,8 @@ impl SP1Verifier {
         let mut challenger = config.challenger();
         let machine = RiscvAir::machine(config);
         let (_, vk) = machine.setup(&Program::from(elf));
-        machine.verify(&vk, &proof.proof, &mut challenger)
+        tracing::info_span!("verify")
+            .in_scope(|| machine.verify(&vk, &proof.proof, &mut challenger))
     }
 
     /// Verify a proof generated by `SP1Prover` with a custom config.

core/src/memory/trace.rs

@@ -1,5 +1,5 @@
-use crate::cpu::{MemoryReadRecord, MemoryRecord, MemoryRecordEnum, MemoryWriteRecord};
 use crate::field::event::FieldEvent;
+use crate::runtime::{MemoryReadRecord, MemoryRecord, MemoryRecordEnum, MemoryWriteRecord};
 use p3_field::Field;
 
 use super::{MemoryAccessCols, MemoryReadCols, MemoryReadWriteCols, MemoryWriteCols};

core/src/runtime/instruction.rs

@@ -1,9 +1,7 @@
+use super::Opcode;
 use core::fmt::Debug;
-
 use serde::{Deserialize, Serialize};
 
-use super::Opcode;
-
 /// An instruction specifies an operation to execute and the operands.
 #[derive(Clone, Copy, Serialize, Deserialize)]
 pub struct Instruction {
@@ -16,6 +14,7 @@ pub struct Instruction {
 }
 
 impl Instruction {
+    /// Create a new instruction.
     pub fn new(opcode: Opcode, op_a: u32, op_b: u32, op_c: u32, imm_b: bool, imm_c: bool) -> Self {
         Self {
             opcode,
@@ -75,6 +74,7 @@ impl Instruction {
         )
     }
 
+    /// Returns if the instruction is a jump instruction.
     pub fn is_jump_instruction(&self) -> bool {
         matches!(self.opcode, Opcode::JAL | Opcode::JALR)
     }

core/src/runtime/io.rs

@@ -1,9 +1,8 @@
+use super::Runtime;
 use serde::de::DeserializeOwned;
 use serde::Serialize;
 use std::io::Read;
 
-use super::Runtime;
-
 impl Read for Runtime {
     fn read(&mut self, buf: &mut [u8]) -> std::io::Result<usize> {
         self.read_stdout_slice(buf);

core/src/cpu/memory.rs → core/src/runtime/memory.rs

@@ -1,18 +1,34 @@
 use serde::{Deserialize, Serialize};
 
-#[derive(Debug, Copy, Clone, Serialize, Deserialize)]
-pub enum MemoryRecordEnum {
-    Read(MemoryReadRecord),
-    Write(MemoryWriteRecord),
-}
-
+/// An record of a write to a memory address.
 #[derive(Debug, Copy, Clone, Default, Serialize, Deserialize)]
 pub struct MemoryRecord {
+    /// The value at the memory address.
     pub value: u32,
+
+    /// The shard in which the memory address was last written to.
     pub shard: u32,
+
+    /// The timestamp at which the memory address was last written to.
     pub timestamp: u32,
 }
 
+#[derive(Copy, Clone, Debug, PartialEq)]
+pub enum MemoryAccessPosition {
+    Memory = 0,
+    // Note that these AccessPositions mean that when when read/writing registers, they must be
+    // read/written in the following order: C, B, A.
+    C = 1,
+    B = 2,
+    A = 3,
+}
+
+#[derive(Debug, Copy, Clone, Serialize, Deserialize)]
+pub enum MemoryRecordEnum {
+    Read(MemoryReadRecord),
+    Write(MemoryWriteRecord),
+}
+
 #[derive(Debug, Copy, Clone, Default, Serialize, Deserialize)]
 #[non_exhaustive]
 pub struct MemoryReadRecord {