Skip to content

Commit 1ba442a

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.
1 parent 6598baf commit 1ba442a

File tree

3 files changed

+80
-68
lines changed

3 files changed

+80
-68
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 # []

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

Lines changed: 41 additions & 30 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:?}"),
@@ -111,28 +111,35 @@ fn load_dw() {
111111
let config = proptest::test_runner::Config::with_cases(10);
112112
let res = TestRunner::new(config).run(&any::<u64>(), move |value| {
113113
// Write `value` to the start of the 17th page (1 page after the 16 pages reserved for the
114-
// Rust stack)
114+
// Rust stack). Felts must be written in little endian order.
115115
let value_felts = value.to_felts();
116116
let initializers = [Initializer::MemoryFelts {
117117
addr: write_to / 4,
118-
felts: Cow::Borrowed(value_felts.as_slice()),
118+
felts: Cow::Borrowed(&[value_felts[1], value_felts[0]]),
119119
}];
120120

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})");
130-
let stored = trace.read_from_rust_memory::<u64>(write_to).ok_or_else(|| {
129+
130+
prop_assert_eq!(lo, value & 0xffffffff);
131+
prop_assert_eq!(hi, value >> 32);
132+
133+
let mut 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 \
133136
read from that address failed"
134137
))
135138
})?;
139+
140+
// read_from_rust_memory() still reads in big-endian limbs.
141+
stored = ((stored >> 32) & 0xffffffff) | (stored << 32);
142+
136143
prop_assert_eq!(
137144
stored,
138145
value,
@@ -145,14 +152,14 @@ fn load_dw() {
145152
Ok(())
146153
})?;
147154

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

150157
Ok(())
151158
});
152159

153160
match res {
154-
Err(TestError::Fail(_, value)) => {
155-
panic!("Found minimal(shrinked) failing case: {value:?}");
161+
Err(TestError::Fail(reason, value)) => {
162+
panic!("FAILURE: {}\nMinimal failing case: {value:?}", reason.message());
156163
}
157164
Ok(_) => (),
158165
_ => panic!("Unexpected test result: {res:?}"),
@@ -214,14 +221,14 @@ fn load_u8() {
214221
Ok(())
215222
})?;
216223

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

219226
Ok(())
220227
});
221228

222229
match res {
223-
Err(TestError::Fail(_, value)) => {
224-
panic!("Found minimal(shrinked) failing case: {value:?}");
230+
Err(TestError::Fail(reason, value)) => {
231+
panic!("FAILURE: {}\nMinimal failing case: {value:?}", reason.message());
225232
}
226233
Ok(_) => (),
227234
_ => panic!("Unexpected test result: {res:?}"),
@@ -283,14 +290,14 @@ fn load_u16() {
283290
Ok(())
284291
})?;
285292

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

288295
Ok(())
289296
});
290297

291298
match res {
292-
Err(TestError::Fail(_, value)) => {
293-
panic!("Found minimal(shrinked) failing case: {value:?}");
299+
Err(TestError::Fail(reason, value)) => {
300+
panic!("FAILURE: {}\nMinimal failing case: {value:?}", reason.message());
294301
}
295302
Ok(_) => (),
296303
_ => panic!("Unexpected test result: {res:?}"),
@@ -358,14 +365,14 @@ fn load_bool() {
358365
},
359366
)?;
360367

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

363370
Ok(())
364371
});
365372

366373
match res {
367-
Err(TestError::Fail(_, value)) => {
368-
panic!("Found minimal(shrinked) failing case: {value:?}");
374+
Err(TestError::Fail(reason, value)) => {
375+
panic!("FAILURE: {}\nMinimal failing case: {value:?}", reason.message());
369376
}
370377
Ok(_) => (),
371378
_ => panic!("Unexpected test result: {res:?}"),
@@ -502,8 +509,8 @@ fn store_u16() {
502509
);
503510

504511
match res {
505-
Err(TestError::Fail(_, value)) => {
506-
panic!("Found minimal(shrinked) failing case: {value:?}");
512+
Err(TestError::Fail(reason, value)) => {
513+
panic!("FAILURE: {}\nMinimal failing case: {value:?}", reason.message());
507514
}
508515
Ok(_) => (),
509516
_ => panic!("Unexpected test result: {res:?}"),
@@ -724,8 +731,8 @@ fn store_u8() {
724731
);
725732

726733
match res {
727-
Err(TestError::Fail(_, value)) => {
728-
panic!("Found minimal(shrinked) failing case: {value:?}");
734+
Err(TestError::Fail(reason, value)) => {
735+
panic!("FAILURE: {}\nMinimal failing case: {value:?}", reason.message());
729736
}
730737
Ok(_) => (),
731738
_ => panic!("Unexpected test result: {res:?}"),
@@ -826,10 +833,8 @@ fn store_unaligned_u64() {
826833
// Use the start of the 17th page (1 page after the 16 pages reserved for the Rust stack)
827834
let write_to = 17 * 2u32.pow(16);
828835

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;
836+
// Value which in turn will be little-endian bytes [ AA BB CC DD EE FF AB CD ] at addr.
837+
let write_val = 0xcdabffee_ddccbbaa_u64;
833838

834839
// Generate a `test` module with `main` function that stores to a u32 offset.
835840
let signature = Signature::new(
@@ -917,6 +922,12 @@ fn store_unaligned_u64() {
917922
assert_eq!(output, 1);
918923
};
919924

925+
// Overwrite 01 02 03 04 05 06 07 08-11 12 13 14 15 16 17 18
926+
// with bytes aa bb cc dd ee ff ab cd at offset 0:
927+
// Expect aa bb cc dd ee ff ab cd 11 12 13 14 15 16 17 18
928+
// or 0xccbbaa01, 0xabffeedd, 0x141312cd, 0x18171615
929+
run_test(0, 0xddccbbaa, 0xcdabffee, 0x14131211, 0x18171615);
930+
920931
// Overwrite 01 02 03 04 05 06 07 08-11 12 13 14 15 16 17 18
921932
// with bytes aa bb cc dd ee ff ab cd at offset 1:
922933
// 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)