Skip to content

Commit 2b222e9

Browse files
MaillewMaillew
committed
chore: debugging; works for shift!=0, source >12
1 parent 84bb711 commit 2b222e9

File tree

2 files changed

+157
-117
lines changed

2 files changed

+157
-117
lines changed

‎extensions/memcpy/circuit/src/iteration.rs‎

Lines changed: 129 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -104,9 +104,10 @@ impl<AB: InteractionBuilder> Air<AB> for MemcpyIterAir {
104104

105105
let timestamp: AB::Var = local.timestamp;
106106
let mut timestamp_delta: AB::Expr = AB::Expr::ZERO;
107-
let mut timestamp_pp = |timestamp_increase_value: AB::Var| {
107+
let mut timestamp_pp = |timestamp_increase_value: AB::Expr| {
108+
let timestamp_increase_clone = timestamp_increase_value.clone();
108109
timestamp_delta += timestamp_increase_value.into();
109-
timestamp + timestamp_delta.clone() - timestamp_increase_value.clone()
110+
timestamp + timestamp_delta.clone() - timestamp_increase_clone
110111
};
111112

112113
let shift = local
@@ -127,10 +128,6 @@ impl<AB: InteractionBuilder> Air<AB> for MemcpyIterAir {
127128
let is_shift_two = local.shift[1];
128129
let is_shift_three = local.shift[2];
129130

130-
let is_source_0 = local.is_source_0_4_8[0];
131-
let is_source_4 = local.is_source_0_4_8[1];
132-
let is_source_8 = local.is_source_0_4_8[2];
133-
let is_source_small = or::<AB::Expr>(is_source_0, or::<AB::Expr>(is_source_4, is_source_8));
134131
let is_end =
135132
(local.is_boundary + AB::Expr::ONE) * local.is_boundary * (AB::F::TWO).inverse();
136133
let is_not_start = (local.is_boundary + AB::Expr::ONE)
@@ -263,7 +260,7 @@ impl<AB: InteractionBuilder> Air<AB> for MemcpyIterAir {
263260
// since is_shift_non_zero degree is 2, we need to keep the degree of the condition to 1
264261
builder
265262
.when(not::<AB::Expr>(prev.is_valid_not_start) - not::<AB::Expr>(prev.is_valid))
266-
.assert_eq(local.timestamp, prev.timestamp + is_shift_non_zero.clone());
263+
.assert_eq(local.timestamp, prev.timestamp + AB::Expr::ONE);
267264

268265
// if prev.is_valid_not_start and local.is_valid_not_start, then timestamp=prev_timestamp+8
269266
// prev.is_valid_not_start is the opposite of previous condition
@@ -288,12 +285,12 @@ impl<AB: InteractionBuilder> Air<AB> for MemcpyIterAir {
288285
*/
289286

290287
// local_source_0_4_8
291-
eprintln!("is_source_small: {:?}", is_source_small);
292-
eprintln!("is_shift_non_zero: {:?}", is_shift_non_zero);
293-
eprintln!(
294-
"current timestamp: {:?}",
295-
local.timestamp * AB::Expr::from_canonical_u32(1)
296-
);
288+
// eprintln!("is_source_small: {:?}", is_source_small);
289+
// eprintln!("is_shift_non_zero: {:?}", is_shift_non_zero);
290+
// eprintln!(
291+
// "current timestamp: {:?}",
292+
// local.timestamp * AB::Expr::from_canonical_u32(1)
293+
// );
297294

298295
self.memcpy_bus
299296
.send(
@@ -312,29 +309,14 @@ impl<AB: InteractionBuilder> Air<AB> for MemcpyIterAir {
312309
let read_data = [local.data_1, local.data_2, local.data_3, local.data_4];
313310

314311
read_data.iter().enumerate().for_each(|(idx, data)| {
312+
// is valid read of entire 16 block chunk?
315313
let is_valid_read = if idx == 3 {
316-
local.is_shift_non_zero_or_not_start
314+
// will always be a valid read
315+
AB::Expr::ONE * (local.is_shift_non_zero_or_not_start)
317316
} else {
318-
local.is_valid_not_start
317+
// if idx < 3, its not an entire block read, if its the first block
318+
AB::Expr::ONE * (local.is_valid_not_start)
319319
};
320-
321-
/*
322-
7 values go:
323-
address sspace
324-
pointer to address
325-
data (4)
326-
timestamp
327-
data chosen is wrong lol
328-
*/
329-
// memory bridge data is tracing_read
330-
// based off of the unshifted data values
331-
332-
// tracing_write writes the corectly shifted values
333-
334-
eprintln!(
335-
"memory bridge read_data: {:?}",
336-
data.clone().map(|x| x * (AB::Expr::from_canonical_u32(1)))
337-
);
338320
self.memory_bridge
339321
.read(
340322
MemoryAddress::new(
@@ -346,22 +328,76 @@ impl<AB: InteractionBuilder> Air<AB> for MemcpyIterAir {
346328
&local.read_aux[idx],
347329
)
348330
.eval(builder, is_valid_read.clone());
349-
});
350-
eprintln!(
351-
"memory bridge data write_data: {:?}",
352-
write_data
353-
.iter()
354-
.map(|x| x.clone().map(|y| y * (AB::Expr::from_canonical_u32(1))))
355-
.collect::<Vec<_>>()
356-
);
357-
// Write final data to registers
358-
write_data_pairs.iter().for_each(|(prev_data, next_data)| {
359331
eprintln!(
360-
"prev_data: {:?}, next_data: {:?}",
361-
prev_data.map(|x| x * (AB::Expr::from_canonical_u32(1))),
362-
next_data.map(|x| x * (AB::Expr::from_canonical_u32(1))),
332+
"local.source: {:?}, data: {:?}, local.source - AB::Expr::from_canonical_usize(16 - idx * 4): {:?}",
333+
local.source
334+
data.clone().map(|x| x * (AB::Expr::from_canonical_u32(1))),
335+
local.source - AB::Expr::from_canonical_usize(16 - idx * 4) * AB::Expr::from_canonical_u32(1)
363336
);
364337
});
338+
/*
339+
7 values go:
340+
address sspace
341+
pointer to address
342+
data (4)
343+
timestamp
344+
345+
rn timestamp is wrong...
346+
*/
347+
// memory bridge data is tracing_read
348+
// based off of the unshifted data values
349+
350+
// tracing_write writes the corectly shifted values
351+
352+
// eprintln!(
353+
// "memory bridge read_data: {:?}",
354+
// data.clone().map(|x| x * (AB::Expr::from_canonical_u32(1)))
355+
// );
356+
357+
//read timestamp is off?
358+
// seems fine? since we use the read_aux variable to store the timestamp
359+
// error seems to be off by one for timestamp again, or off by 17 or 21??
360+
// timestamps are inconsistent everywhere...
361+
362+
/*
363+
heres the plan:
364+
identify everywhere where timeststamps are used; where theyre written to (tracing_read) etc.
365+
walk through each component, and take notes about how timestamp is incrementing in each portion
366+
*/
367+
368+
/*
369+
off by one:
370+
between memcpyiter Air and itself
371+
372+
off by 16:
373+
memcpyiter air and itself
374+
off by 17:
375+
between memoryDummyAir and accessAdapterAir
376+
377+
off by 21:
378+
memcpyiterAir and itself
379+
memory dummy and access adapter air
380+
381+
bruh, seems that everything is wrong... WHATS WRONG WITH THE TIMESTAMPS MAN
382+
*/
383+
384+
// eprintln!(
385+
// "memory bridge data write_data: {:?}",
386+
// write_data
387+
// .iter()
388+
// .map(|x| x.clone().map(|y| y * (AB::Expr::from_canonical_u32(1))))
389+
// .collect::<Vec<_>>()
390+
// );
391+
// Write final data to registers
392+
// write_data_pairs.iter().for_each(|(prev_data, next_data)| {
393+
// eprintln!(
394+
// "prev_data: {:?}, next_data: {:?}",
395+
// prev_data.map(|x| x * (AB::Expr::from_canonical_u32(1))),
396+
// next_data.map(|x| x * (AB::Expr::from_canonical_u32(1))),
397+
// );
398+
// });
399+
400+
// is timestamping off, is it pointer is off?
365401
write_data.iter().enumerate().for_each(|(idx, data)| {
366402
self.memory_bridge
367403
.write(
@@ -370,10 +406,15 @@ impl<AB: InteractionBuilder> Air<AB> for MemcpyIterAir {
370406
local.dest - AB::Expr::from_canonical_usize(16 - idx * 4),
371407
),
372408
data.clone(),
373-
timestamp_pp(local.is_valid_not_start),
409+
timestamp_pp(AB::Expr::ONE * (local.is_valid_not_start)),
374410
&local.write_aux[idx],
375411
)
376412
.eval(builder, local.is_valid_not_start);
413+
// eprintln!(
414+
// "Eval: write_data: {:?}, timestamp_pp: {:?}",
415+
// data.clone().map(|x| x * (AB::Expr::from_canonical_u32(1))),
416+
// timestamp_pp(AB::Expr::ZERO * (local.is_valid_not_start))
417+
// );
377418
});
378419

379420
// Range check len
@@ -548,7 +589,7 @@ where
548589

549590
let head = if shift == 0 { 0 } else { 4 - shift as u32 };
550591
let effective_len = len.saturating_sub(head);
551-
let num_iters = (effective_len / 16) as usize; // floor((len - head)/16)
592+
let num_iters = (effective_len / 16);
552593

553594
// eprintln!(
554595
// "PREFLIGHT: len={}, shift={}, effective_len={}, num_iters={}, allocated_rows={}",
@@ -561,7 +602,7 @@ where
561602
let record: MemcpyIterRecordMut<'_> =
562603
state.ctx.alloc(MultiRowLayout::new(MemcpyIterMetadata {
563604
//allocating based on number of rows needed
564-
num_rows: num_iters + 1,
605+
num_rows: num_iters as usize + 1,
565606
})); // is this too big then??
566607

567608
// Store the original values in the record
@@ -587,31 +628,28 @@ where
587628
// &mut record.var[0].read_aux[2].prev_timestamp,
588629
// );
589630
source = source.saturating_sub(12 * (shift != 0) as u32);
631+
// we have saturating sub, which isnt a perfect sub,
632+
// 0, 4, 20
633+
// source is tOO SMALL, SO READING SAME DATA TWICE??
590634
if shift != 0 {
591-
// if source >= 4 {
592635
record.var[0].data[3] = tracing_read(
593636
state.memory,
594637
RV32_MEMORY_AS,
595-
source - 4 * (source >= 4) as u32, // correct seed for mixing
638+
source - 4 * (source >= 4) as u32,
596639
&mut record.var[0].read_aux[3].prev_timestamp,
597640
);
598-
// eprintln!("record.var[0].data[3]: {:?}", record.var[0].data[3]);
599-
// } else {
600-
// record.var[0].data[3] = tracing_read(
601-
// state.memory,
602-
// RV32_MEMORY_AS,
603-
// source,
604-
// &mut record.var[0].read_aux[3].prev_timestamp,
605-
// );
606-
// }
607641
} else {
608642
record.var[0].data[3] = tracing_read(
609643
state.memory,
610644
RV32_MEMORY_AS,
611-
source,
645+
source - 4 * (source >= 4) as u32, // what happens when we read same memory twice? is it bc not constraining properly? since its the same piece of memory; this error will still happen, if source < 4? since no "previous" word
612646
&mut record.var[0].read_aux[3].prev_timestamp,
613647
);
614648
}
649+
eprintln!(
650+
"record.var[0].read_aux[3].prev_timestamp: {:?}",
651+
record.var[0].read_aux[3].prev_timestamp
652+
);
615653

616654
// Fill record.var for the rest of the rows of iteration trace
617655
let mut idx = 1;
@@ -638,7 +676,13 @@ where
638676
record.var[idx].data[i][j - shift as usize]
639677
}
640678
});
641-
eprintln!("execute write_data: {:?}", write_data);
679+
eprintln!(
680+
"src {:?}, dest {:?}, idx {:?}",
681+
source + 4 * i as u32,
682+
dest + 4 * i as u32,
683+
idx
684+
);
685+
eprintln!("execute read_data: {:?}", record.var[idx].data[i]);
642686
write_data
643687
});
644688
eprintln!("record.var[idx].data: {:?}", record.var[idx].data);
@@ -708,6 +752,10 @@ where
708752
}
709753
}
710754

755+
// dummy air is wrong?
756+
// initial timestamp is 18, but the tracing read is filing it in as 1?
757+
// this is bc in the testing framework, it will write things into memory to "set up" the initial state
758+
711759
/*
712760
- generate_proving_ctx is what creates trace fill
713761
- row major matrix, so stored in a vector, row by row
@@ -755,11 +803,8 @@ impl<F: PrimeField32> TraceFiller<F> for MemcpyIterFiller {
755803
sizes
756804
);
757805

758-
chunks
759-
.par_iter_mut()
760-
.zip(sizes.par_iter())
761-
.enumerate()
762-
.for_each(|(_row_idx, (chunk, &num_rows))| {
806+
chunks.iter_mut().zip(sizes.iter()).enumerate().for_each(
807+
|(_row_idx, (chunk, &num_rows))| {
763808
let record: MemcpyIterRecordMut = unsafe {
764809
get_record_from_slice(
765810
chunk,
@@ -782,9 +827,7 @@ impl<F: PrimeField32> TraceFiller<F> for MemcpyIterFiller {
782827

783828
// Calculate the timestamp for the last memory access
784829
// 4 reads + 4 writes per iteration + (shift != 0) read for the loop header
785-
let timestamp = record.inner.from_timestamp
786-
+ ((num_rows - 1) << 3) as u32
787-
+ (record.inner.shift != 0) as u32;
830+
let timestamp = record.inner.from_timestamp + ((num_rows - 1) << 3) as u32 + 1;
788831
let mut timestamp_delta: u32 = 0;
789832
let mut get_timestamp = |is_access: bool| {
790833
if is_access {
@@ -800,9 +843,13 @@ impl<F: PrimeField32> TraceFiller<F> for MemcpyIterFiller {
800843
// record.inner.source + ((num_rows - 1) << 4) as u32
801844
// );
802845

803-
let mut dest = record.inner.dest + ((num_rows - 1) << 4) as u32; // got rid of -1 here???
846+
let mut dest = record.inner.dest + ((num_rows - 1) << 4) as u32;
804847
let mut source = (record.inner.source + ((num_rows - 1) << 4) as u32)
805848
.saturating_sub(12 * (record.inner.shift != 0) as u32);
849+
eprintln!(
850+
"record.inner.source: {:?}, num_rows: {:?}, source: {:?}",
851+
record.inner.source, num_rows, source
852+
);
806853
let mut len =
807854
record.inner.len - ((num_rows - 1) << 4) as u32 - record.inner.shift as u32;
808855

@@ -835,17 +882,12 @@ impl<F: PrimeField32> TraceFiller<F> for MemcpyIterFiller {
835882
cols.write_aux.iter_mut().rev().for_each(|aux_col| {
836883
mem_helper.fill_zero(aux_col.as_mut());
837884
});
838-
839-
if record.inner.shift == 0 {
840-
mem_helper.fill_zero(cols.read_aux[3].as_mut());
841-
} else {
842-
mem_helper.fill(
843-
var.read_aux[3].prev_timestamp,
844-
get_timestamp(true),
845-
cols.read_aux[3].as_mut(),
846-
);
847-
}
848-
cols.read_aux[..2].iter_mut().rev().for_each(|aux_col| {
885+
mem_helper.fill(
886+
var.read_aux[3].prev_timestamp,
887+
get_timestamp(true),
888+
cols.read_aux[3].as_mut(),
889+
);
890+
cols.read_aux[..3].iter_mut().rev().for_each(|aux_col| {
849891
mem_helper.fill_zero(aux_col.as_mut());
850892
});
851893

@@ -873,7 +915,7 @@ impl<F: PrimeField32> TraceFiller<F> for MemcpyIterFiller {
873915
.for_each(|(aux_record, aux_col)| {
874916
mem_helper.fill(
875917
aux_record.prev_timestamp,
876-
get_timestamp(true),
918+
get_timestamp(true), // BUG was HERE. given current timestamp, need to read from memory at an earlier timestamp, cant read form the current one
877919
aux_col.as_mut(),
878920
);
879921
});
@@ -907,11 +949,6 @@ impl<F: PrimeField32> TraceFiller<F> for MemcpyIterFiller {
907949
cols.is_source_0_4_8 =
908950
[source == 0, source == 4, source == 8].map(F::from_bool);
909951
dest = dest.saturating_sub(16);
910-
911-
if source < 16 {
912-
eprintln!("source: {:?}", source);
913-
eprintln!("cols.is_boundary: {:?}", cols.is_boundary);
914-
}
915952
source = source.saturating_sub(16);
916953
len += 16;
917954

@@ -935,7 +972,8 @@ impl<F: PrimeField32> TraceFiller<F> for MemcpyIterFiller {
935972
// cols.read_aux.map(|x| x.get_base().timestamp_lt_aux.lower_decomp.iter().map(|x| x.as_canonical_u32()).collect::<Vec<_>>()).to_vec());
936973
// }
937974
});
938-
});
975+
},
976+
);
939977

940978
// chunks.iter().enumerate().for_each(|(row_idx, chunk)| {
941979
// let mut prv_data = [0; 4];
@@ -1138,13 +1176,9 @@ unsafe fn execute_e12_impl<F: PrimeField32, CTX: ExecutionCtxTrait>(
11381176
// Read the previous data from memory if shift != 0
11391177
// - 12 * (shift != 0) as u32 is affecting the write_data lol
11401178
let mut prev_data: [u8; 4] = if shift != 0 {
1141-
if source >= 4 {
1142-
exec_state.vm_read::<u8, 4>(RV32_MEMORY_AS, source - 4)
1143-
} else {
1144-
[0; 4]
1145-
}
1179+
exec_state.vm_read::<u8, 4>(RV32_MEMORY_AS, source - 4 * (source >= 4) as u32)
11461180
} else {
1147-
[0; 4] // unused when shift == 0
1181+
exec_state.vm_read::<u8, 4>(RV32_MEMORY_AS, source as u32)
11481182
};
11491183

11501184
eprintln!("num_iters: {:?}", num_iters);

0 commit comments

Comments
 (0)