Skip to content

Commit 90b629c

Browse files
GunaDDGunaDD
committed
chore: do jpw's todo by fixing the test config to not resize the number of cells in the register AS since now we have a separate gen_distinct_register_pointers
1 parent 3432514 commit 90b629c

19 files changed

Lines changed: 89 additions & 132 deletions

File tree

‎crates/vm/src/arch/testing/cpu.rs‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,7 @@ use openvm_circuit_primitives::{
88
Chip,
99
};
1010
use openvm_cpu_backend::{CpuBackend, CpuDevice, CpuProverError};
11-
use openvm_instructions::{
12-
instruction::Instruction,
13-
riscv::{RV64_REGISTER_AS, RV64_REGISTER_NUM_LIMBS},
14-
DEFERRAL_AS,
15-
};
11+
use openvm_instructions::{instruction::Instruction, riscv::RV64_REGISTER_NUM_LIMBS, DEFERRAL_AS};
1612
use openvm_poseidon2_air::Poseidon2SubAir;
1713
use openvm_stark_backend::{
1814
interaction::{LookupBus, PermutationCheckBus},
@@ -367,9 +363,6 @@ impl<F: VmField> VmChipTestBuilder<F> {
367363
impl<F: VmField> Default for VmChipTestBuilder<F> {
368364
fn default() -> Self {
369365
let mut mem_config = MemoryConfig::default();
370-
// TODO[jpw]: this is because old tests use `gen_pointer` on address space 1; this can be
371-
// removed when tests are updated.
372-
mem_config.addr_spaces[RV64_REGISTER_AS as usize].num_cells = 1 << 31;
373366
mem_config.addr_spaces[DEFERRAL_AS as usize].num_cells = 0;
374367
Self::from_config(mem_config)
375368
}

‎crates/vm/src/arch/testing/cuda.rs‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ use openvm_cuda_common::{
2525
common::get_device,
2626
stream::{device_synchronize, CudaStream, GpuDeviceCtx, StreamGuard},
2727
};
28-
use openvm_instructions::{program::PC_BITS, riscv::RV64_REGISTER_AS};
28+
use openvm_instructions::program::PC_BITS;
2929
use openvm_poseidon2_air::{Poseidon2Config, Poseidon2SubAir};
3030
use openvm_stark_backend::{
3131
interaction::{LookupBus, PermutationCheckBus},
@@ -55,7 +55,7 @@ use crate::{
5555
},
5656
to_byte_ptr_bits, Arena, DenseRecordArena, ExecutionBridge, ExecutionBus, ExecutionState,
5757
MatrixRecordArena, MemoryConfig, PreflightExecutor, Streams, VmStateMut, BLOCK_FE_WIDTH,
58-
MEMORY_BLOCK_BYTES, U16_CELL_SIZE,
58+
MEMORY_BLOCK_BYTES,
5959
},
6060
system::{
6161
cuda::poseidon2::Poseidon2PeripheryChipGPU,
@@ -269,11 +269,7 @@ pub struct GpuChipTestBuilder {
269269

270270
impl Default for GpuChipTestBuilder {
271271
fn default() -> Self {
272-
let mut mem_config = MemoryConfig::default();
273-
// Increasing the size of the register AS for testing since the `gen_register_pointer`
274-
// function creates wider registers.
275-
mem_config.addr_spaces[RV64_REGISTER_AS as usize].num_cells = (1 << 16) / U16_CELL_SIZE;
276-
Self::new(mem_config, default_var_range_checker_bus())
272+
Self::new(MemoryConfig::default(), default_var_range_checker_bus())
277273
}
278274
}
279275

‎crates/vm/src/arch/testing/memory/mod.rs‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use air::{MemoryDummyAir, MemoryDummyChip};
22
use rand::Rng;
33

44
use crate::{
5-
arch::{MemoryCellType, VmField, BLOCK_FE_WIDTH, U16_CELL_SIZE},
5+
arch::{MemoryCellType, VmField, BLOCK_FE_WIDTH, NUM_RV64_REGISTERS, U16_CELL_SIZE},
66
system::memory::{
77
offline_checker::pack_u8_block_value, online::TracingMemory, MemoryController,
88
},
@@ -157,6 +157,24 @@ pub fn gen_register_pointer<R>(rng: &mut R, len: usize) -> usize
157157
where
158158
R: Rng + ?Sized,
159159
{
160-
const MAX_REGISTER: usize = 1 << 16;
161-
rng.random_range(0..MAX_REGISTER - len) / len * len
160+
rng.random_range(0..NUM_RV64_REGISTERS * size_of::<u64>() - len) / len * len
161+
}
162+
163+
/// Generates `N` pairwise-distinct `len`-aligned register pointers. Use this when a test writes
164+
/// several register operands independently: the register file only has 32 slots, so independent
165+
/// draws collide often enough to corrupt the expected values.
166+
pub fn gen_distinct_register_pointers<R, const N: usize>(rng: &mut R, len: usize) -> [usize; N]
167+
where
168+
R: Rng + ?Sized,
169+
{
170+
let mut ptrs = [0usize; N];
171+
for i in 0..N {
172+
ptrs[i] = loop {
173+
let ptr = gen_register_pointer(rng, len);
174+
if !ptrs[..i].contains(&ptr) {
175+
break ptr;
176+
}
177+
};
178+
}
179+
ptrs
162180
}

‎extensions/algebra/circuit/src/fp2_chip/tests.rs‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use num_traits::Zero;
88
use openvm_algebra_transpiler::Fp2Opcode;
99
use openvm_circuit::arch::{
1010
testing::{
11-
memory::{gen_pointer, gen_register_pointer},
11+
memory::{gen_distinct_register_pointers, gen_pointer},
1212
TestBuilder, TestChipHarness, VmChipTestBuilder,
1313
},
1414
Arena, PreflightExecutor, MEMORY_BLOCK_BYTES,
@@ -151,9 +151,7 @@ fn set_and_execute_fp2<const BLOCKS: usize, const NUM_LIMBS: usize, RA>(
151151
let ptr_as = RV64_REGISTER_AS as usize;
152152
let data_as = RV64_MEMORY_AS as usize;
153153

154-
let rs1_ptr = gen_register_pointer(rng, RV64_REGISTER_NUM_LIMBS);
155-
let rs2_ptr = gen_register_pointer(rng, RV64_REGISTER_NUM_LIMBS);
156-
let rd_ptr = gen_register_pointer(rng, RV64_REGISTER_NUM_LIMBS);
154+
let [rs1_ptr, rs2_ptr, rd_ptr] = gen_distinct_register_pointers(rng, RV64_REGISTER_NUM_LIMBS);
157155

158156
let a_base_addr = gen_pointer(rng, RV64_REGISTER_NUM_LIMBS) as u32;
159157
let b_base_addr = gen_pointer(rng, RV64_REGISTER_NUM_LIMBS) as u32;

‎extensions/deferral/circuit/src/call/tests.rs‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use std::{array::from_fn, sync::Arc};
33
use openvm_circuit::arch::{
44
deferral::{DeferralState, InputMapVal},
55
testing::{
6-
memory::{gen_pointer, gen_register_pointer},
6+
memory::{gen_distinct_register_pointers, gen_pointer},
77
TestBuilder, TestChipHarness, VmChipTestBuilder, BITWISE_OP_LOOKUP_BUS,
88
},
99
Arena, MatrixRecordArena, MemoryConfig, PreflightExecutor, BLOCK_FE_WIDTH, MEMORY_BLOCK_BYTES,
@@ -109,7 +109,6 @@ struct CudaHarnessBundle {
109109

110110
fn test_memory_config() -> MemoryConfig {
111111
let mut config = MemoryConfig::default();
112-
config.addr_spaces[RV64_REGISTER_AS as usize].num_cells = 1 << config.pointer_max_bits;
113112
config.addr_spaces[DEFERRAL_AS as usize].num_cells = 1 << 20;
114113
config
115114
}
@@ -152,8 +151,7 @@ fn set_and_execute_call<RA, E>(
152151
RA: Arena,
153152
E: PreflightExecutor<F, RA>,
154153
{
155-
let rd = gen_register_pointer(rng, MEMORY_BLOCK_BYTES);
156-
let rs = gen_register_pointer(rng, MEMORY_BLOCK_BYTES);
154+
let [rd, rs] = gen_distinct_register_pointers(rng, MEMORY_BLOCK_BYTES);
157155
let output_ptr = gen_pointer(rng, MEMORY_BLOCK_BYTES);
158156
let input_ptr = gen_pointer(rng, MEMORY_BLOCK_BYTES);
159157
let deferral_idx = rng.random_range(0..num_deferrals);

‎extensions/deferral/circuit/src/output/tests.rs‎

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,10 @@ use std::sync::Arc;
33
use openvm_circuit::arch::{
44
deferral::{DeferralResult, DeferralState},
55
testing::{
6-
memory::{gen_pointer, gen_register_pointer},
6+
memory::{gen_distinct_register_pointers, gen_pointer},
77
TestBuilder, TestChipHarness, VmChipTestBuilder, BITWISE_OP_LOOKUP_BUS,
88
},
9-
to_byte_ptr_bits, Arena, MatrixRecordArena, MemoryConfig, PreflightExecutor,
10-
MEMORY_BLOCK_BYTES,
9+
Arena, MatrixRecordArena, MemoryConfig, PreflightExecutor, MEMORY_BLOCK_BYTES,
1110
};
1211
use openvm_circuit_primitives::bitwise_op_lookup::{
1312
BitwiseOperationLookupAir, BitwiseOperationLookupBus, BitwiseOperationLookupChip,
@@ -101,18 +100,10 @@ struct CudaHarnessBundle {
101100

102101
fn test_memory_config() -> MemoryConfig {
103102
let mut config = MemoryConfig::default();
104-
config.addr_spaces[RV64_REGISTER_AS as usize].num_cells = 1 << config.pointer_max_bits;
105103
config.addr_spaces[DEFERRAL_AS as usize].num_cells = 1 << 20;
106104
config
107105
}
108106

109-
fn test_memory_config_cpu() -> MemoryConfig {
110-
let mut config = test_memory_config();
111-
config.addr_spaces[RV64_REGISTER_AS as usize].num_cells =
112-
1 << to_byte_ptr_bits(config.pointer_max_bits);
113-
config
114-
}
115-
116107
fn init_streams(tester: &mut impl TestBuilder<F>, num_deferrals: usize) {
117108
tester.streams_mut().deferrals = vec![DeferralState::new(vec![]); num_deferrals];
118109
}
@@ -158,8 +149,7 @@ fn set_and_execute_output<RA, E>(
158149
RA: Arena,
159150
E: PreflightExecutor<F, RA>,
160151
{
161-
let rd = gen_register_pointer(rng, MEMORY_BLOCK_BYTES);
162-
let rs = gen_register_pointer(rng, MEMORY_BLOCK_BYTES);
152+
let [rd, rs] = gen_distinct_register_pointers(rng, MEMORY_BLOCK_BYTES);
163153
let output_ptr = gen_pointer(rng, MEMORY_BLOCK_BYTES);
164154
let input_ptr = gen_pointer(rng, MEMORY_BLOCK_BYTES);
165155
let deferral_idx = rng.random_range(0..num_deferrals);
@@ -328,7 +318,7 @@ fn create_cuda_harness(tester: &GpuChipTestBuilder, num_deferrals: usize) -> Cud
328318
#[test]
329319
fn rand_deferral_output_test() {
330320
let mut rng = create_seeded_rng();
331-
let mut tester = VmChipTestBuilder::<F>::from_config(test_memory_config_cpu());
321+
let mut tester = VmChipTestBuilder::<F>::from_config(test_memory_config());
332322
let CpuHarnessBundle {
333323
mut harness,
334324
bitwise,
@@ -376,7 +366,7 @@ fn rand_deferral_output_test() {
376366
#[test]
377367
fn deferral_output_non_first_row_canonicity_aux_cleared_test() {
378368
let mut rng = create_seeded_rng();
379-
let mut tester = VmChipTestBuilder::<F>::from_config(test_memory_config_cpu());
369+
let mut tester = VmChipTestBuilder::<F>::from_config(test_memory_config());
380370
let CpuHarnessBundle {
381371
mut harness,
382372
bitwise,
@@ -391,8 +381,7 @@ fn deferral_output_non_first_row_canonicity_aux_cleared_test() {
391381
// columns the filler skips (the bug under test) retain this value.
392382
harness.arena.trace_buffer.fill(F::from_u32(0xdead));
393383

394-
let rd = gen_register_pointer(&mut rng, MEMORY_BLOCK_BYTES);
395-
let rs = gen_register_pointer(&mut rng, MEMORY_BLOCK_BYTES);
384+
let [rd, rs] = gen_distinct_register_pointers(&mut rng, MEMORY_BLOCK_BYTES);
396385
let output_ptr = gen_pointer(&mut rng, MEMORY_BLOCK_BYTES);
397386
let input_ptr = gen_pointer(&mut rng, MEMORY_BLOCK_BYTES);
398387
let deferral_idx = 0;

‎extensions/ecc/circuit/src/weierstrass_chip/tests.rs‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use num_bigint::BigUint;
77
use num_traits::{FromPrimitive, Num, Zero};
88
use openvm_circuit::arch::{
99
testing::{
10-
memory::{gen_pointer, gen_register_pointer},
10+
memory::{gen_distinct_register_pointers, gen_pointer},
1111
TestBuilder, TestChipHarness, VmChipTestBuilder,
1212
},
1313
Arena, MatrixRecordArena, PreflightExecutor, MEMORY_BLOCK_BYTES,
@@ -226,9 +226,8 @@ mod ec_addne_tests {
226226
let ptr_as = RV64_REGISTER_AS as usize;
227227
let data_as = RV64_MEMORY_AS as usize;
228228

229-
let rs1_ptr = gen_register_pointer(rng, RV64_REGISTER_NUM_LIMBS);
230-
let rs2_ptr = gen_register_pointer(rng, RV64_REGISTER_NUM_LIMBS);
231-
let rd_ptr = gen_register_pointer(rng, RV64_REGISTER_NUM_LIMBS);
229+
let [rs1_ptr, rs2_ptr, rd_ptr] =
230+
gen_distinct_register_pointers(rng, RV64_REGISTER_NUM_LIMBS);
232231

233232
let p1_base_addr = gen_pointer(rng, MEMORY_BLOCK_BYTES) as u64;
234233
let p2_base_addr = gen_pointer(rng, MEMORY_BLOCK_BYTES) as u64;
@@ -644,8 +643,7 @@ mod ec_double_tests {
644643
let ptr_as = RV64_REGISTER_AS as usize;
645644
let data_as = RV64_MEMORY_AS as usize;
646645

647-
let rs1_ptr = gen_register_pointer(rng, RV64_REGISTER_NUM_LIMBS);
648-
let rd_ptr = gen_register_pointer(rng, RV64_REGISTER_NUM_LIMBS);
646+
let [rs1_ptr, rd_ptr] = gen_distinct_register_pointers(rng, RV64_REGISTER_NUM_LIMBS);
649647

650648
let p1_base_addr = gen_pointer(rng, MEMORY_BLOCK_BYTES) as u64;
651649
let result_base_addr = gen_pointer(rng, MEMORY_BLOCK_BYTES) as u64;

‎extensions/keccak256/circuit/src/xorin/tests.rs‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -123,10 +123,8 @@ fn set_and_execute<RA: Arena, E: PreflightExecutor<F, RA>>(
123123
let mut rand_input_arr = [0u8; MAX_LEN];
124124
rand_input_arr.copy_from_slice(&rand_input);
125125

126-
use openvm_circuit::arch::testing::memory::{gen_pointer, gen_register_pointer};
127-
let rd = gen_register_pointer(rng, RV64_REGISTER_NUM_LIMBS);
128-
let rs1 = gen_register_pointer(rng, RV64_REGISTER_NUM_LIMBS);
129-
let rs2 = gen_register_pointer(rng, RV64_REGISTER_NUM_LIMBS);
126+
use openvm_circuit::arch::testing::memory::{gen_distinct_register_pointers, gen_pointer};
127+
let [rd, rs1, rs2] = gen_distinct_register_pointers(rng, RV64_REGISTER_NUM_LIMBS);
130128

131129
// Align buffer/input pointers to MEMORY_BLOCK_BYTES-byte blocks for memory bus compatibility
132130
let num_blocks = buffer_length.div_ceil(MEMORY_BLOCK_BYTES);
@@ -345,16 +343,15 @@ fn cuda_set_and_execute(
345343
rng: &mut StdRng,
346344
len: Option<usize>,
347345
) {
348-
use openvm_circuit::arch::testing::memory::{gen_pointer, gen_register_pointer};
346+
use openvm_circuit::arch::testing::memory::{gen_distinct_register_pointers, gen_pointer};
349347

350348
let len = len.unwrap_or_else(|| rng.random_range(1..=KECCAK_RATE_MEM_OPS) * MEMORY_BLOCK_BYTES);
351349
if len == 0 {
352350
return;
353351
}
354352

355-
let buffer_reg = gen_register_pointer(rng, RV64_REGISTER_NUM_LIMBS);
356-
let input_reg = gen_register_pointer(rng, RV64_REGISTER_NUM_LIMBS);
357-
let len_reg = gen_register_pointer(rng, RV64_REGISTER_NUM_LIMBS);
353+
let [buffer_reg, input_reg, len_reg] =
354+
gen_distinct_register_pointers(rng, RV64_REGISTER_NUM_LIMBS);
358355

359356
let buffer_ptr = gen_pointer(rng, len);
360357
let input_ptr = gen_pointer(rng, len);

‎extensions/riscv-adapters/src/test_utils.rs‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use openvm_circuit::arch::{
2-
testing::{memory::gen_register_pointer, TestBuilder},
2+
testing::{memory::gen_distinct_register_pointers, TestBuilder},
33
BLOCK_FE_WIDTH, U16_CELL_SIZE,
44
};
55
use openvm_instructions::{instruction::Instruction, VmOpcode};
@@ -168,9 +168,8 @@ pub fn rv64_rand_write_register_or_imm<const NUM_LIMBS: usize>(
168168
) -> (Instruction<BabyBear>, usize) {
169169
let rs2_is_imm = imm.is_some();
170170

171-
let rs1 = gen_register_pointer(rng, NUM_LIMBS);
172-
let rs2 = imm.unwrap_or_else(|| gen_register_pointer(rng, NUM_LIMBS));
173-
let rd = gen_register_pointer(rng, NUM_LIMBS);
171+
let [rs1, rs2_reg, rd] = gen_distinct_register_pointers(rng, NUM_LIMBS);
172+
let rs2 = imm.unwrap_or(rs2_reg);
174173

175174
tester.write::<NUM_LIMBS>(1, rs1, rs1_writes.map(BabyBear::from_u32));
176175
if !rs2_is_imm {

‎extensions/riscv/circuit/src/branch_eq/tests.rs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@ use std::{array, borrow::BorrowMut};
22

33
use openvm_circuit::{
44
arch::{
5-
testing::{memory::gen_register_pointer, TestBuilder, TestChipHarness, VmChipTestBuilder},
5+
testing::{
6+
memory::gen_distinct_register_pointers, TestBuilder, TestChipHarness, VmChipTestBuilder,
7+
},
68
Arena, ExecutionBridge, PreflightExecutor, BLOCK_FE_WIDTH,
79
},
810
system::memory::{offline_checker::MemoryBridge, SharedMemoryHelper},
@@ -108,8 +110,7 @@ fn set_and_execute<RA: Arena, E: PreflightExecutor<F, RA>>(
108110
});
109111

110112
let imm = imm.unwrap_or(rng.random_range((-ABS_MAX_IMM)..ABS_MAX_IMM));
111-
let rs1 = gen_register_pointer(rng, RV64_REGISTER_NUM_LIMBS);
112-
let rs2 = gen_register_pointer(rng, RV64_REGISTER_NUM_LIMBS);
113+
let [rs1, rs2] = gen_distinct_register_pointers(rng, RV64_REGISTER_NUM_LIMBS);
113114
tester.write_bytes::<RV64_REGISTER_NUM_LIMBS>(1, rs1, a.map(F::from_u8));
114115
tester.write_bytes::<RV64_REGISTER_NUM_LIMBS>(1, rs2, b.map(F::from_u8));
115116

0 commit comments

Comments
 (0)