Skip to content

Commit 0475f18

Browse files
committed
Change the memory representation of 64-bit, dual-limbed values to be little-endian.
In memory the low (least significant) 32-bit limb is now loaded or stored from/to `addr` and the high 32-bit limb from/to `addr+1`. On the stack the limb order is still big-endian. For testing the `ToMidenRepr` array of felts are now little-endian. The `FromMidenRepr` is still a big-endian array of felts, as per the stack.
1 parent fae010b commit 0475f18

File tree

5 files changed

+78
-72
lines changed

5 files changed

+78
-72
lines changed

codegen/masm/intrinsics/mem.masm

Lines changed: 30 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -317,22 +317,21 @@ export.load_dw # [addr, offset]
317317
# if the pointer is naturally aligned..
318318
if.true
319319
# drop offset
320-
swap.1 drop # [addr]
321-
# load second element
322-
dup.0 push.1 u32overflowing_add assertz # [addr + 1, addr]
323-
mem_load # [e1, addr]
324-
# load first element
325-
swap.1 mem_load # [e0, e1]
320+
swap.1 drop # [addr]
321+
322+
dup.0 mem_load # [lo, addr]
323+
swap.1 push.1 u32overflowing_add assertz # [addr + 1, lo]
324+
mem_load # [hi, lo]
326325
else
327326
# unaligned; an unaligned double-word spans three elements
328327
#
329328
# convert offset from bytes to bits
330329
swap.1 push.8 u32wrapping_mul swap.1 # [addr, bit_offset]
331330

332331
# load the three elements containing the double-word on the stack
333-
dup.0 push.2 u32overflowing_add assertz mem_load # [e2, addr, bit_offset]
334-
dup.1 push.1 add mem_load # [e1, e2, addr, bit_offset]
335-
movup.2 mem_load # [e0, e1, e2, bit_offset]
332+
dup.0 mem_load # [lo, addr, bit_offset]
333+
dup.1 push.1 u32overflowing_add assertz mem_load # [mid, lo, addr, bit_offset]
334+
dup.2 push.2 u32overflowing_add assertz mem_load # [hi, mid, lo, addr, bit_offset]
336335

337336
# re-align it, and we're done
338337
exec.realign_dw
@@ -447,8 +446,8 @@ export.store_dw # [addr, offset, value_hi, value_lo]
447446
if.true # [addr, offset, value_hi, value_lo]
448447
# drop byte offset
449448
swap.1 drop # [addr, value_hi, value_lo]
450-
swap.1 dup.1 # [addr, value_hi, addr, value_lo]
451-
mem_store # [addr, value_lo]
449+
movup.2 dup.1 # [addr, value_lo, addr, value_hi]
450+
mem_store # [addr, value_hi]
452451
push.1 u32overflowing_add assertz # [addr + 1, value_lo]
453452
mem_store
454453
else # [addr, offset, value_hi, value_lo]
@@ -459,34 +458,37 @@ export.store_dw # [addr, offset, value_hi, value_lo]
459458

460459
movdn.3 # [bit_offset, value_hi, value_lo, addr]
461460
dup.0 movdn.4 # [bit_offset, value_hi, value_lo, addr, bit_offset]
462-
movdn.2 # [value_hi, value_lo, bit_offset, addr, bit_offset]
463-
exec.offset_dw # [chunk_lo, chunk_mid, chunk_hi, addr, bit_offset]
461+
movdn.2 swap.1 # [value_lo, value_hi, bit_offset, addr, bit_offset]
462+
463+
# NOTE: we call offset_dw with args in reverse order for which it is documented, but the
464+
# hi/mid/lo distinction is arbirtrary.
465+
exec.offset_dw # [chunk_hi, chunk_mid, chunk_lo, addr, bit_offset]
464466

465467
# compute the bit shift
466-
push.32 movup.5 sub # [32 - bit_offset, lo, mid, hi, addr]
468+
push.32 movup.5 sub # [32 - bit_offset, hi, mid, lo, addr]
467469

468470
# compute the masks
469-
push.4294967295 swap.1 u32shr # [mask_hi, lo, mid, hi, addr]
470-
dup.0 u32not # [mask_lo, mask_hi, lo, mid, hi, addr]
471+
push.4294967295 swap.1 u32shr # [mask_lo, hi, mid, lo, addr]
472+
dup.0 u32not # [mask_hi, mask_lo, hi, mid, lo, addr]
471473

472474
# combine the bits of the lo and hi chunks with their corresponding elements, so that we
473475
# do not overwrite memory that is out of bounds of the actual value being stored.
474476
#
475-
# 1. load e2 (lo)
477+
# 1. load e2 (hi)
476478
dup.5 push.2 u32overflowing_add assertz mem_load
477-
# 2. mask e2 (lo)
478-
u32and # [e2_masked, mask_hi, lo, mid, hi, addr]
479-
# 3. load e0 (hi)
480-
dup.5 mem_load # [e0, e2_masked, mask_hi, lo, mid, hi, addr]
481-
# 4. mask e0 (hi)
482-
movup.2 u32and # [e0_masked, e2_masked, lo, mid, hi, addr]
483-
# 5. combine e2 & lo
484-
movdn.2 u32or # [e2', e0_masked, mid, hi, addr]
479+
# 2. mask e2 (hi)
480+
u32and # [e2_masked, mask_lo, hi, mid, lo, addr]
481+
# 3. load e0 (lo)
482+
dup.5 mem_load # [e0, e2_masked, mask_lo, hi, mid, lo, addr]
483+
# 4. mask e0 (lo)
484+
movup.2 u32and # [e0_masked, e2_masked, hi, mid, lo, addr]
485+
# 5. combine e2 & hi
486+
movdn.2 u32or # [e2', e0_masked, mid, lo, addr]
485487
# 6. store e2
486-
dup.4 add.2 mem_store # [e0_masked, mid, hi, addr]
488+
dup.4 add.2 mem_store # [e0_masked, mid, lo, addr]
487489
# 7. store e1 (mid)
488-
swap.1 dup.3 add.1 mem_store # [e0_masked, hi, addr]
489-
# 8. combine e0 & hi
490+
swap.1 dup.3 add.1 mem_store # [e0_masked, lo, addr]
491+
# 8. combine e0 & lo
490492
u32or # [e0', addr]
491493
# 9. store e0
492494
swap.1 mem_store # []

midenc-debug/src/exec/trace.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -193,13 +193,13 @@ impl ExecutionTrace {
193193
2 => {
194194
let lo = self.read_memory_element_in_context(ptr.addr, ctx, clk)?;
195195
let hi = self.read_memory_element_in_context(ptr.addr + 1, ctx, clk)?;
196-
Some(T::from_felts(&[lo, hi]))
196+
Some(T::from_felts(&[hi, lo]))
197197
}
198198
3 => {
199199
let lo_l = self.read_memory_element_in_context(ptr.addr, ctx, clk)?;
200200
let lo_h = self.read_memory_element_in_context(ptr.addr + 1, ctx, clk)?;
201201
let hi_l = self.read_memory_element_in_context(ptr.addr + 2, ctx, clk)?;
202-
Some(T::from_felts(&[lo_l, lo_h, hi_l]))
202+
Some(T::from_felts(&[hi_l, lo_h, lo_l]))
203203
}
204204
n => {
205205
assert_ne!(n, 0);

midenc-debug/src/felt.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ pub trait ToMidenRepr {
6868
/// Push this value on the given operand stack using [Self::to_felts] representation
6969
fn push_to_operand_stack(&self, stack: &mut Vec<RawFelt>) {
7070
let felts = self.to_felts();
71-
for felt in felts.into_iter().rev() {
71+
for felt in felts.into_iter() {
7272
stack.push(felt);
7373
}
7474
}
@@ -400,7 +400,7 @@ impl ToMidenRepr for u64 {
400400
let bytes = self.to_be_bytes();
401401
let hi = u32::from_be_bytes([bytes[0], bytes[1], bytes[2], bytes[3]]);
402402
let lo = u32::from_be_bytes([bytes[4], bytes[5], bytes[6], bytes[7]]);
403-
smallvec![RawFelt::new(hi as u64), RawFelt::new(lo as u64)]
403+
smallvec![RawFelt::new(lo as u64), RawFelt::new(hi as u64)]
404404
}
405405
}
406406

@@ -466,9 +466,7 @@ impl ToMidenRepr for u128 {
466466
let lo_l =
467467
RawFelt::new(u32::from_be_bytes([bytes[12], bytes[13], bytes[14], bytes[15]]) as u64);
468468

469-
// The 64-bit limbs are little endian, (lo, hi), but the 32-bit limbs of those 64-bit
470-
// values are big endian, (lo_h, lo_l) and (hi_h, hi_l).
471-
smallvec![lo_h, lo_l, hi_h, hi_l]
469+
smallvec![hi_l, hi_h, lo_l, lo_h]
472470
}
473471
}
474472

tests/integration/src/codegen/intrinsics/mem.rs

Lines changed: 34 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -70,14 +70,14 @@ fn load_sw() {
7070
Ok(())
7171
})?;
7272

73-
prop_assert_eq!(output, value);
73+
prop_assert_eq!(output, value, "expected 0x{:x}; found 0x{:x}", value, output,);
7474

7575
Ok(())
7676
});
7777

7878
match res {
79-
Err(TestError::Fail(_, value)) => {
80-
panic!("Found minimal(shrinked) failing case: {value:?}");
79+
Err(TestError::Fail(reason, value)) => {
80+
panic!("FAILURE: {}\nMinimal failing case: {value:?}", reason.message());
8181
}
8282
Ok(_) => (),
8383
_ => panic!("Unexpected test result: {res:?}"),
@@ -121,12 +121,15 @@ fn load_dw() {
121121
let args = [Felt::new(write_to as u64)];
122122
let output =
123123
eval_package::<u64, _, _>(&package, initializers, &args, context.session(), |trace| {
124-
let hi =
125-
trace.read_memory_element(write_to / 4).unwrap_or_default().as_int() as u32;
126-
let lo = trace.read_memory_element((write_to / 4) + 1).unwrap_or_default().as_int()
127-
as u32;
124+
let lo = trace.read_memory_element(write_to / 4).unwrap_or_default().as_int();
125+
let hi = trace.read_memory_element((write_to / 4) + 1).unwrap_or_default().as_int();
126+
128127
log::trace!(target: "executor", "hi = {hi} ({hi:0x})");
129128
log::trace!(target: "executor", "lo = {lo} ({lo:0x})");
129+
130+
prop_assert_eq!(lo, value & 0xffffffff);
131+
prop_assert_eq!(hi, value >> 32);
132+
130133
let stored = trace.read_from_rust_memory::<u64>(write_to).ok_or_else(|| {
131134
TestCaseError::fail(format!(
132135
"expected {value} to have been written to byte address {write_to}, but \
@@ -145,14 +148,14 @@ fn load_dw() {
145148
Ok(())
146149
})?;
147150

148-
prop_assert_eq!(output, value);
151+
prop_assert_eq!(output, value, "expected 0x{:x}; found 0x{:x}", value, output,);
149152

150153
Ok(())
151154
});
152155

153156
match res {
154-
Err(TestError::Fail(_, value)) => {
155-
panic!("Found minimal(shrinked) failing case: {value:?}");
157+
Err(TestError::Fail(reason, value)) => {
158+
panic!("FAILURE: {}\nMinimal failing case: {value:?}", reason.message());
156159
}
157160
Ok(_) => (),
158161
_ => panic!("Unexpected test result: {res:?}"),
@@ -214,14 +217,14 @@ fn load_u8() {
214217
Ok(())
215218
})?;
216219

217-
prop_assert_eq!(output, value);
220+
prop_assert_eq!(output, value, "expected 0x{:x}; found 0x{:x}", value, output,);
218221

219222
Ok(())
220223
});
221224

222225
match res {
223-
Err(TestError::Fail(_, value)) => {
224-
panic!("Found minimal(shrinked) failing case: {value:?}");
226+
Err(TestError::Fail(reason, value)) => {
227+
panic!("FAILURE: {}\nMinimal failing case: {value:?}", reason.message());
225228
}
226229
Ok(_) => (),
227230
_ => panic!("Unexpected test result: {res:?}"),
@@ -283,14 +286,14 @@ fn load_u16() {
283286
Ok(())
284287
})?;
285288

286-
prop_assert_eq!(output, value);
289+
prop_assert_eq!(output, value, "expected 0x{:x}; found 0x{:x}", value, output,);
287290

288291
Ok(())
289292
});
290293

291294
match res {
292-
Err(TestError::Fail(_, value)) => {
293-
panic!("Found minimal(shrinked) failing case: {value:?}");
295+
Err(TestError::Fail(reason, value)) => {
296+
panic!("FAILURE: {}\nMinimal failing case: {value:?}", reason.message());
294297
}
295298
Ok(_) => (),
296299
_ => panic!("Unexpected test result: {res:?}"),
@@ -358,14 +361,14 @@ fn load_bool() {
358361
},
359362
)?;
360363

361-
prop_assert_eq!(output, value);
364+
prop_assert_eq!(output, value, "expected {}; found {}", output, value);
362365

363366
Ok(())
364367
});
365368

366369
match res {
367-
Err(TestError::Fail(_, value)) => {
368-
panic!("Found minimal(shrinked) failing case: {value:?}");
370+
Err(TestError::Fail(reason, value)) => {
371+
panic!("FAILURE: {}\nMinimal failing case: {value:?}", reason.message());
369372
}
370373
Ok(_) => (),
371374
_ => panic!("Unexpected test result: {res:?}"),
@@ -502,8 +505,8 @@ fn store_u16() {
502505
);
503506

504507
match res {
505-
Err(TestError::Fail(_, value)) => {
506-
panic!("Found minimal(shrinked) failing case: {value:?}");
508+
Err(TestError::Fail(reason, value)) => {
509+
panic!("FAILURE: {}\nMinimal failing case: {value:?}", reason.message());
507510
}
508511
Ok(_) => (),
509512
_ => panic!("Unexpected test result: {res:?}"),
@@ -724,8 +727,8 @@ fn store_u8() {
724727
);
725728

726729
match res {
727-
Err(TestError::Fail(_, value)) => {
728-
panic!("Found minimal(shrinked) failing case: {value:?}");
730+
Err(TestError::Fail(reason, value)) => {
731+
panic!("FAILURE: {}\nMinimal failing case: {value:?}", reason.message());
729732
}
730733
Ok(_) => (),
731734
_ => panic!("Unexpected test result: {res:?}"),
@@ -826,10 +829,8 @@ fn store_unaligned_u64() {
826829
// Use the start of the 17th page (1 page after the 16 pages reserved for the Rust stack)
827830
let write_to = 17 * 2u32.pow(16);
828831

829-
// STORE_DW writes the high 32bit word to address and low 32bit word to address+1.
830-
// So a .store() of 0xddccbbaa_cdabffee writes 0xddccbbaa to addr and 0xcdabffee to addr+1.
831-
// Which in turn will be little-endian bytes [ AA BB CC DD EE FF AB CD ] at addr.
832-
let write_val = 0xddccbbaa_cdabffee_u64;
832+
// Value which in turn will be little-endian bytes [ AA BB CC DD EE FF AB CD ] at addr.
833+
let write_val = 0xcdabffee_ddccbbaa_u64;
833834

834835
// Generate a `test` module with `main` function that stores to a u32 offset.
835836
let signature = Signature::new(
@@ -917,6 +918,12 @@ fn store_unaligned_u64() {
917918
assert_eq!(output, 1);
918919
};
919920

921+
// Overwrite 01 02 03 04 05 06 07 08-11 12 13 14 15 16 17 18
922+
// with bytes aa bb cc dd ee ff ab cd at offset 0:
923+
// Expect aa bb cc dd ee ff ab cd 11 12 13 14 15 16 17 18
924+
// or 0xccbbaa01, 0xabffeedd, 0x141312cd, 0x18171615
925+
run_test(0, 0xddccbbaa, 0xcdabffee, 0x14131211, 0x18171615);
926+
920927
// Overwrite 01 02 03 04 05 06 07 08-11 12 13 14 15 16 17 18
921928
// with bytes aa bb cc dd ee ff ab cd at offset 1:
922929
// Expect 01 aa bb cc dd ee ff ab cd 12 13 14 15 16 17 18

tests/integration/src/rust_masm_tests/instructions.rs

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -87,13 +87,6 @@ macro_rules! test_wide_bin_op {
8787
let rs_out = a $op b;
8888
dbg!(&rs_out);
8989

90-
// Moves the little-endian 32bit limbs [A, B, C, D] to [D, C, B, A].
91-
let rs_out = ((rs_out >> 32) & 0xffffffff_00000000_00000000)
92-
| ((rs_out & 0xffffffff_00000000_00000000) << 32)
93-
| ((rs_out & 0xffffffff_00000000) >> 32)
94-
| ((rs_out & 0xffffffff) << 32);
95-
let rs_out_bytes = rs_out.to_le_bytes();
96-
9790
// Write the operation result to 20 * PAGE_SIZE.
9891
let out_addr = 20u32 * 65536;
9992

@@ -104,11 +97,14 @@ macro_rules! test_wide_bin_op {
10497
dbg!(&args);
10598

10699
eval_package::<Felt, _, _>(&package, None, &args, &test.session, |trace| {
107-
let vm_out: [u8; 16] =
100+
let vm_out_bytes: [u8; 16] =
108101
trace.read_from_rust_memory(out_addr).expect("output was not written");
109-
dbg!(&vm_out);
102+
dbg!(&vm_out_bytes);
103+
104+
let rs_out_bytes = rs_out.to_le_bytes();
110105
dbg!(&rs_out_bytes);
111-
prop_assert_eq!(&rs_out_bytes, &vm_out, "VM output mismatch");
106+
107+
prop_assert_eq!(&rs_out_bytes, &vm_out_bytes, "VM output mismatch");
112108
Ok(())
113109
})?;
114110

@@ -253,6 +249,9 @@ test_int_op!(add, +, i8, 0..=i8::MAX/2, 0..=i8::MAX/2);
253249
// const WK1234: i128 = 79228162551157825753847955460000;
254250
// const WC1234: i128 = 7922816255115782575384795546000;
255251
//
252+
// const WK1234H: i128 = 0x00001000_00002000_00003000_00004000;
253+
// const WC1234H: i128 = 0x00000100_00000200_00000300_00000400;
254+
//
256255
// test_wide_bin_op!(xxx, x, i128, i128, WK1234..=WK1234, WC1234..=WC1234);
257256

258257
test_wide_bin_op!(add, +, u128, u128, 0..=u128::MAX/2, 0..=u128::MAX/2);

0 commit comments

Comments
 (0)