Skip to content

Commit 0b9f6f2

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 47bb21f commit 0b9f6f2

File tree

4 files changed

+143
-57
lines changed

4 files changed

+143
-57
lines changed

codegen/masm/intrinsics/mem.masm

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ export.realign_dw # [chunk_hi, chunk_mid, chunk_lo, offset]
288288
swap.1 # [x_hi, x_lo]
289289
end
290290

291-
# Shift a double-word (64-bit, in two 32-bit chunks) value by the given offset.
291+
# Shift two 32-bit words by the given offset.
292292
#
293293
# Returns three 32-bit chunks [chunk_lo, chunk_mid, chunk_hi]
294294
export.offset_dw # [value_hi, value_lo, offset]
@@ -308,7 +308,7 @@ export.offset_dw # [value_hi, value_lo, offset]
308308
u32shr # [ chunk_lo, chunk_mid, chunk_hi]
309309
end
310310

311-
# Load a machine double-word (64-bit value, in two 32-bit chunks) to the operand stack
311+
# Load two 32-bit words to the operand stack
312312
export.load_dw # [addr, offset]
313313
# check for alignment and offset validity
314314
dup.1 eq.0 # [offset == 0, addr, offset]
@@ -434,7 +434,7 @@ export.store_sw # [addr, offset, value]
434434
end
435435
end
436436

437-
# Store a 64-bit value, i.e. two 32-bit machine words from the given native pointer tuple.
437+
# Store two 32-bit words to the given native pointer tuple.
438438
#
439439
# A native pointer tuple consists of an element address where the data begins, and a byte offset,
440440
# which is the offset of the first byte, in the 32-bit representation of that element.
@@ -536,7 +536,7 @@ end
536536
# Write `count` copies of `value` to memory, starting at `dst`.
537537
#
538538
# * `dst` is expected to be an address in byte-addressable space, _not_ an element address.
539-
# * `value` must be a 64-bit value or smaller
539+
# * `value` must be a two 32-bit words.
540540
export.memset_dw # [size, dst, count, value_hi, value_lo]
541541
# prepare to loop until `count` iterations have been performed
542542
push.0 # [i, dst, size, count, value_hi, value_lo]

codegen/masm/src/emit/mem.rs

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ impl OpEmitter<'_> {
6262
);
6363
match &ty {
6464
Type::I128 => self.load_quad_word(None, span),
65-
Type::I64 | Type::U64 => self.load_double_word(None, span),
65+
Type::I64 | Type::U64 => self.load_double_word_int(None, span),
6666
Type::Felt => self.load_felt(None, span),
6767
Type::I32 | Type::U32 => self.load_word(None, span),
6868
ty @ (Type::I16 | Type::U16 | Type::U8 | Type::I8 | Type::I1) => {
@@ -87,7 +87,7 @@ impl OpEmitter<'_> {
8787
let ptr = NativePtr::from_ptr(addr);
8888
match &ty {
8989
Type::I128 => self.load_quad_word(Some(ptr), span),
90-
Type::I64 | Type::U64 => self.load_double_word(Some(ptr), span),
90+
Type::I64 | Type::U64 => self.load_double_word_int(Some(ptr), span),
9191
Type::Felt => self.load_felt(Some(ptr), span),
9292
Type::I32 | Type::U32 => self.load_word(Some(ptr), span),
9393
Type::I16 | Type::U16 | Type::U8 | Type::I8 | Type::I1 => {
@@ -170,13 +170,17 @@ impl OpEmitter<'_> {
170170
}
171171
}
172172

173-
/// Load a pair of machine words (32-bit elements) to the operand stack
174-
fn load_double_word(&mut self, ptr: Option<NativePtr>, span: SourceSpan) {
173+
/// Load a 64-bit word from the given address.
174+
fn load_double_word_int(&mut self, ptr: Option<NativePtr>, span: SourceSpan) {
175175
if let Some(imm) = ptr {
176-
return self.load_double_word_imm(imm, span);
176+
self.load_double_word_imm(imm, span);
177+
} else {
178+
self.raw_exec("intrinsics::mem::load_dw", span);
177179
}
178180

179-
self.raw_exec("intrinsics::mem::load_dw", span);
181+
// The mem::intrinsic loads two 32-bit words with the first at the top of the stack. Swap
182+
// them to make a big-endian-limbed stack value.
183+
self.emit(masm::Instruction::Swap1, span);
180184
}
181185

182186
/// Load a sub-word value (u8, u16, etc.) from memory
@@ -538,7 +542,7 @@ impl OpEmitter<'_> {
538542
);
539543
match value_ty {
540544
Type::I128 => self.store_quad_word(None, span),
541-
Type::I64 | Type::U64 => self.store_double_word(None, span),
545+
Type::I64 | Type::U64 => self.store_double_word_int(None, span),
542546
Type::Felt => self.store_felt(None, span),
543547
Type::I32 | Type::U32 => self.store_word(None, span),
544548
ref ty if ty.size_in_bytes() <= 4 => self.store_small(ty, None, span),
@@ -566,7 +570,7 @@ impl OpEmitter<'_> {
566570
let ptr = NativePtr::from_ptr(addr);
567571
match value_ty {
568572
Type::I128 => self.store_quad_word(Some(ptr), span),
569-
Type::I64 | Type::U64 => self.store_double_word(Some(ptr), span),
573+
Type::I64 | Type::U64 => self.store_double_word_int(Some(ptr), span),
570574
Type::Felt => self.store_felt(Some(ptr), span),
571575
Type::I32 | Type::U32 => self.store_word(Some(ptr), span),
572576
ref ty if ty.size_in_bytes() <= 4 => self.store_small(ty, Some(ptr), span),
@@ -853,13 +857,18 @@ impl OpEmitter<'_> {
853857
}
854858
}
855859

856-
/// Store a pair of machine words (32-bit elements) to the operand stack
857-
fn store_double_word(&mut self, ptr: Option<NativePtr>, span: SourceSpan) {
860+
/// Store a 64-bit word to the operand stack
861+
fn store_double_word_int(&mut self, ptr: Option<NativePtr>, span: SourceSpan) {
862+
// The mem::intrinsic stores two 32-bit words in stack order. Swap them (the 3rd and 4th
863+
// params) first to make a little-endian-limbed memory value.
864+
self.emit(masm::Instruction::MovUp2, span);
865+
self.emit(masm::Instruction::MovDn3, span);
866+
858867
if let Some(imm) = ptr {
859-
return self.store_double_word_imm(imm, span);
868+
self.store_double_word_imm(imm, span);
869+
} else {
870+
self.raw_exec("intrinsics::mem::store_dw", span);
860871
}
861-
862-
self.raw_exec("intrinsics::mem::store_dw", span);
863872
}
864873

865874
fn store_double_word_imm(&mut self, ptr: NativePtr, span: SourceSpan) {

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

Lines changed: 108 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:?}"),
@@ -821,15 +828,80 @@ fn store_unaligned_u32() {
821828
run_test(3, 0xaa332211, 0x88ddccbb);
822829
}
823830

831+
#[test]
832+
fn load_unaligned_u64() {
833+
// Use the start of the 17th page (1 page after the 16 pages reserved for the Rust stack)
834+
let write_to = 17 * 2u32.pow(16);
835+
836+
// Generate a `test` module with `main` function that loads from `write_to` + a passed offset.
837+
let signature = Signature::new([AbiParam::new(Type::U32)], [AbiParam::new(Type::U64)]);
838+
839+
// Compile once outside the test loop
840+
let (package, context) = compile_test_module(signature, |builder| {
841+
let block = builder.current_block();
842+
843+
// Get the offset, add it to the base address and load the 64bit value there.
844+
let offs = block.borrow().arguments()[0] as ValueRef;
845+
let base_addr = builder.u32(write_to, SourceSpan::default());
846+
let read_addr = builder.add(base_addr, offs, SourceSpan::default()).unwrap();
847+
let ptr = builder
848+
.inttoptr(read_addr, Type::from(PointerType::new(Type::U64)), SourceSpan::default())
849+
.unwrap();
850+
let loaded = builder.load(ptr, SourceSpan::default()).unwrap();
851+
852+
// Return the value so we can assert that the output of execution matches
853+
builder.ret(Some(loaded), SourceSpan::default()).unwrap();
854+
});
855+
856+
let run_test = |offs: u32, expected: u64| {
857+
// Initialise memory with some known bytes.
858+
let initializers = [Initializer::MemoryBytes {
859+
addr: write_to,
860+
bytes: &[
861+
0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16,
862+
0x17, 0x18,
863+
],
864+
}];
865+
866+
let output = eval_package::<u64, _, _>(
867+
&package,
868+
initializers,
869+
&[Felt::new(offs as u64)],
870+
context.session(),
871+
|trace| {
872+
//
873+
let stack = trace.outputs();
874+
let hi: u64 = stack.get_stack_item(0).unwrap().into();
875+
let lo: u64 = stack.get_stack_item(1).unwrap().into();
876+
877+
eprintln!("hi limb = 0x{hi:08x}");
878+
eprintln!("lo limb = 0x{lo:08x}");
879+
880+
Ok(())
881+
},
882+
)
883+
.unwrap();
884+
885+
assert_eq!(output, expected);
886+
};
887+
888+
run_test(0, 0x0807060504030201_u64);
889+
run_test(1, 0x1108070605040302_u64);
890+
run_test(2, 0x1211080706050403_u64);
891+
run_test(3, 0x1312110807060504_u64);
892+
run_test(4, 0x1413121108070605_u64);
893+
run_test(5, 0x1514131211080706_u64);
894+
run_test(6, 0x1615141312110807_u64);
895+
run_test(7, 0x1716151413121108_u64);
896+
}
897+
824898
#[test]
825899
fn store_unaligned_u64() {
826900
// Use the start of the 17th page (1 page after the 16 pages reserved for the Rust stack)
827901
let write_to = 17 * 2u32.pow(16);
828902

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

834906
// Generate a `test` module with `main` function that stores to a u32 offset.
835907
let signature = Signature::new(
@@ -917,6 +989,12 @@ fn store_unaligned_u64() {
917989
assert_eq!(output, 1);
918990
};
919991

992+
// Overwrite 01 02 03 04 05 06 07 08-11 12 13 14 15 16 17 18
993+
// with bytes aa bb cc dd ee ff ab cd at offset 0:
994+
// Expect aa bb cc dd ee ff ab cd 11 12 13 14 15 16 17 18
995+
// or 0xccbbaa01, 0xabffeedd, 0x141312cd, 0x18171615
996+
run_test(0, 0xddccbbaa, 0xcdabffee, 0x14131211, 0x18171615);
997+
920998
// Overwrite 01 02 03 04 05 06 07 08-11 12 13 14 15 16 17 18
921999
// with bytes aa bb cc dd ee ff ab cd at offset 1:
9221000
// Expect 01 aa bb cc dd ee ff ab cd 12 13 14 15 16 17 18

0 commit comments

Comments
 (0)