Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: abi transformation tests #284

Merged
merged 18 commits into from
Aug 30, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
4c38c07
fix(codegen): ensure callee results are pushed on stack in correct order
bitwalker Aug 28, 2024
77a63e4
fix(codegen): ensure global initializers are set during rodata init
bitwalker Aug 28, 2024
6a6f774
fix(sdk): force all allocations to be minimally word-aligned
bitwalker Aug 28, 2024
7b52f31
fix(sdk): ensure we pass a miden-native ptr value to get_inputs
bitwalker Aug 28, 2024
affb200
fix(frontend-wasm): incorrect types applied to certain primops
bitwalker Aug 28, 2024
a2d6bc7
fix(frontend-wasm): do not apply redundant casts
bitwalker Aug 28, 2024
89cb0d6
fix(debugger): infinite loop in breakpoint id computation
bitwalker Aug 28, 2024
fc9ff3a
chore(codegen): be consistent about the way in which we push to stack
bitwalker Aug 28, 2024
cb52312
fix(codegen): incorrect handling of multi-result instructions
bitwalker Aug 28, 2024
9d091c0
fix(codegen): incorrect lowering of global.{load,iadd,symbol}
bitwalker Aug 28, 2024
fe8aa70
fix(codegen): make sure we always drop unused instruction results
bitwalker Aug 28, 2024
e77f235
chore(codegen): clippy suggested some improvements
bitwalker Aug 28, 2024
e6a3e95
fix(codegen): incorrect order of elements for word-oriented loads/stores
bitwalker Aug 29, 2024
c010bae
fix: swap the lo and mid parts in the most shifted case in `load_dw`
greenhat Aug 29, 2024
7747dcd
test: re-enable get_inputs test
bitwalker Aug 29, 2024
ee46fe3
chore: update expect tests due to codegen changes
bitwalker Aug 29, 2024
65365a0
fix(codegen): broken return via pointer transformation
bitwalker Aug 30, 2024
ba72ef6
chore: update expect output for sdk account/wallet tests
bitwalker Aug 30, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions codegen/masm/src/codegen/emit/mem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ use midenc_hir::{self as hir, Felt, FieldElement, SourceSpan, StructType, Type};
use super::OpEmitter;
use crate::masm::{NativePtr, Op};

pub(crate) const PAGE_SIZE: u32 = 64 * 1024;

/// Allocation
impl<'a> OpEmitter<'a> {
/// Allocate a procedure-local memory slot of sufficient size to store a value
Expand Down
2 changes: 1 addition & 1 deletion codegen/masm/src/codegen/emit/primop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ impl<'a> OpEmitter<'a> {
}
}

for result in signature.results.iter() {
for result in signature.results.iter().rev() {
self.stack.push(result.ty.clone());
}

Expand Down
1 change: 0 additions & 1 deletion codegen/masm/src/codegen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ mod opt;
mod scheduler;
mod stack;

pub(crate) use self::emit::mem::PAGE_SIZE;
pub use self::{
emitter::FunctionEmitter,
scheduler::Scheduler,
Expand Down
113 changes: 53 additions & 60 deletions codegen/masm/src/masm/program.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
};
use miden_core::crypto::hash::Rpo256;
use midenc_hir::{
self as hir, diagnostics::Report, DataSegmentTable, Felt, FieldElement, FunctionIdent, Ident,
SourceSpan,
self as hir, diagnostics::Report, DataSegmentTable, Felt, FieldElement, FunctionIdent,
GlobalVariableTable, Ident, SourceSpan,
};
use midenc_hir_analysis::GlobalVariableAnalysis;
use midenc_session::{Emit, Session};
Expand Down Expand Up @@ -40,29 +40,6 @@
heap_base: u32,
}
impl Program {
/// Create a new [Program] initialized from a [DataSegmentTable], a set of [Module]s, and an
/// optional entrypoint function.
///
/// A `main.masm` module will be generated which invokes the given entrypoint on startup, after
/// initializing the global heap of the root context, based on the provided data segment table.
///
/// You should generally prefer to use [Program::from_hir], but this constructor allows you to
/// manually produce a MASM program from its constituent parts.
pub fn new<M>(entrypoint: FunctionIdent, segments: &DataSegmentTable, modules: M) -> Self
where
M: IntoIterator<Item = Box<Module>>,
{
use crate::codegen::PAGE_SIZE;

let library = Library::new(segments, modules);
Self {
library,
entrypoint,
// By default, we assume the first two pages are reserved for shadow stack and globals
heap_base: 2 * PAGE_SIZE,
}
}

/// Create a new [Program] initialized from an [hir::Program].
///
/// The resulting [Program] will have the following:
Expand Down Expand Up @@ -381,29 +358,6 @@
Self::default()
}

/// Create a new [Library] initialized from a [DataSegmentTable] and a set of [Module]s.
///
/// You should generally prefer to use [Library::from_hir], but this constructor allows you to
/// manually produce a MASM program from its constituent parts.
pub fn new<M>(segments: &DataSegmentTable, modules: M) -> Self
where
M: IntoIterator<Item = Box<Module>>,
{
let mut module_tree = ModuleTree::default();
for module in modules {
module_tree.insert(module);
}
let modules = Modules::Open(module_tree);
let rodata = compute_rodata(segments);
Self {
modules,
libraries: vec![],
kernel: None,
rodata,
stack_pointer: None,
}
}

/// Create a new [Library] initialized from an [hir::Program].
///
/// The resulting [Library] will have the following:
Expand All @@ -422,7 +376,11 @@
} else {
None
};
let rodata = compute_rodata(program.segments());
let rodata = compute_rodata(
globals.layout().global_table_offset(),
program.globals(),
program.segments(),
);
Self {
modules: Modules::default(),
libraries: vec![],
Expand Down Expand Up @@ -597,18 +555,53 @@
///
/// This consists of the data itself, as well as a content digest, which will be used to place
/// that data in the advice map when the program starts.
fn compute_rodata(segments: &DataSegmentTable) -> Vec<Rodata> {
let mut rodatas = Vec::with_capacity(segments.iter().count());

for segment in segments.iter() {
fn compute_rodata(
global_table_offset: u32,
globals: &GlobalVariableTable,
segments: &DataSegmentTable,
) -> Vec<Rodata> {
let mut rodatas = Vec::with_capacity(segments.iter().count() + 1);

// Convert global variable initializers to a data segment, and place it at the computed
// global table offset in linear memory.
let extra = if globals.len() > 0 {

Check failure on line 567 in codegen/masm/src/masm/program.rs

View workflow job for this annotation

GitHub Actions / clippy

length comparison to zero
let size = globals.size_in_bytes();
let offset = global_table_offset;
let mut data = Vec::<u8>::with_capacity(size);
data.resize(size, 0);

Check failure on line 571 in codegen/masm/src/masm/program.rs

View workflow job for this annotation

GitHub Actions / clippy

slow zero-filling initialization
for gv in globals.iter() {
if let Some(init) = gv.initializer() {
let offset = unsafe { globals.offset_of(gv.id()) } as usize;
let init = globals.get_constant(init);
let init_bytes = init.as_slice();
assert!(offset + init_bytes.len() <= data.len());
let dst = &mut data[offset..(offset + init_bytes.len())];
dst.copy_from_slice(init_bytes);
}
}
// Don't bother emitting anything for zeroed segments
if segment.is_zeroed() {
continue;
if data.iter().any(|&b| b != 0) {
Some((size as u32, offset, Arc::new(midenc_hir::ConstantData::from(data))))
} else {
None
}
let size = segment.size();
let offset = segment.offset();
} else {
None
};

// Process all segments, ignoring zeroed segments (as Miden's memory is always zeroed)
for (size, offset, segment_data) in segments
.iter()
.filter_map(|segment| {
if segment.is_zeroed() {
None
} else {
Some((segment.size(), segment.offset(), segment.init()))
}
})
.chain(extra)
{
let base = NativePtr::from_ptr(offset);
let segment_data = segment.init();

// TODO(pauls): Do we ever have a need for data segments which are not aligned
// to an word boundary? If so, we need to implement that
Expand Down Expand Up @@ -636,13 +629,13 @@
// are mixed together, so that the data is preserved, and
// the commitment is correct
let mut iter = segment_data.as_slice().iter().copied().array_chunks::<4>();
elements.extend(iter.by_ref().map(|bytes| Felt::new(u32::from_be_bytes(bytes) as u64)));
elements.extend(iter.by_ref().map(|bytes| Felt::new(u32::from_le_bytes(bytes) as u64)));
if let Some(remainder) = iter.into_remainder() {
let mut chunk = [0u8; 4];
for (i, byte) in remainder.into_iter().enumerate() {
chunk[i] = byte;
}
elements.push(Felt::new(u32::from_be_bytes(chunk) as u64));
elements.push(Felt::new(u32::from_le_bytes(chunk) as u64));
}
elements.resize(num_elements + padding, Felt::ZERO);
let digest = Rpo256::hash_elements(&elements);
Expand Down
4 changes: 2 additions & 2 deletions codegen/masm/src/packaging/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,13 +168,13 @@ impl Rodata {
let data = self.data.as_slice();
let mut felts = Vec::with_capacity(data.len() / 4);
let mut iter = data.iter().copied().array_chunks::<4>();
felts.extend(iter.by_ref().map(|bytes| Felt::new(u32::from_be_bytes(bytes) as u64)));
felts.extend(iter.by_ref().map(|bytes| Felt::new(u32::from_le_bytes(bytes) as u64)));
if let Some(remainder) = iter.into_remainder() {
let mut chunk = [0u8; 4];
for (i, byte) in remainder.into_iter().enumerate() {
chunk[i] = byte;
}
felts.push(Felt::new(u32::from_be_bytes(chunk) as u64));
felts.push(Felt::new(u32::from_le_bytes(chunk) as u64));
}

let padding = (self.size_in_words() * 4).abs_diff(felts.len());
Expand Down
14 changes: 10 additions & 4 deletions frontend-wasm/src/code_translator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,15 +259,21 @@ pub fn translate_operator(
/******************************* Unary Operators *************************************/
Operator::I32Clz | Operator::I64Clz => {
let val = state.pop1();
state.push1(builder.ins().clz(val, span));
let count = builder.ins().clz(val, span);
// To ensure we match the Wasm semantics, treat the output of clz as an i32
state.push1(builder.ins().bitcast(count, Type::I32, span));
}
Operator::I32Ctz | Operator::I64Ctz => {
let val = state.pop1();
state.push1(builder.ins().ctz(val, span));
let count = builder.ins().ctz(val, span);
// To ensure we match the Wasm semantics, treat the output of ctz as an i32
state.push1(builder.ins().bitcast(count, Type::I32, span));
}
Operator::I32Popcnt | Operator::I64Popcnt => {
let val = state.pop1();
state.push1(builder.ins().popcnt(val, span));
let count = builder.ins().popcnt(val, span);
// To ensure we match the Wasm semantics, treat the output of popcnt as an i32
state.push1(builder.ins().bitcast(count, Type::I32, span));
}
Operator::I32Extend8S | Operator::I32Extend16S => {
let val = state.pop1();
Expand Down Expand Up @@ -749,7 +755,7 @@ fn translate_br_if(
state: &mut FuncTranslationState,
span: SourceSpan,
) -> WasmResult<()> {
let cond = state.pop1();
let cond = state.pop1_bitcasted(Type::I32, builder, span);
let (br_destination, inputs) = translate_br_if_args(relative_depth, state);
let next_block = builder.create_block();
let then_dest = br_destination;
Expand Down
40 changes: 32 additions & 8 deletions frontend-wasm/src/module/func_translation_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,11 @@ impl FuncTranslationState {
span: SourceSpan,
) -> Value {
let val = self.stack.pop().expect("attempted to pop a value from an empty stack");
builder.ins().cast(val, ty.clone(), span)
if builder.data_flow_graph().value_type(val) != &ty {
builder.ins().cast(val, ty, span)
} else {
val
}
}

/// Pop one value and bitcast it to the specified type.
Expand All @@ -290,7 +294,11 @@ impl FuncTranslationState {
span: SourceSpan,
) -> Value {
let val = self.stack.pop().expect("attempted to pop a value from an empty stack");
builder.ins().bitcast(val, ty.clone(), span)
if builder.data_flow_graph().value_type(val) != &ty {
builder.ins().bitcast(val, ty, span)
} else {
val
}
}

/// Peek at the top of the stack without popping it.
Expand All @@ -314,9 +322,17 @@ impl FuncTranslationState {
) -> (Value, Value) {
let v2 = self.stack.pop().unwrap();
let v1 = self.stack.pop().unwrap();
let v1_casted = builder.ins().cast(v1, ty.clone(), span);
let v2_casted = builder.ins().cast(v2, ty, span);
(v1_casted, v2_casted)
let v1 = if builder.data_flow_graph().value_type(v1) != &ty {
builder.ins().cast(v1, ty.clone(), span)
} else {
v1
};
let v2 = if builder.data_flow_graph().value_type(v2) != &ty {
builder.ins().cast(v2, ty, span)
} else {
v2
};
(v1, v2)
}

/// Pop two values. Bitcast them to the specified type. Return them in the order they were
Expand All @@ -329,9 +345,17 @@ impl FuncTranslationState {
) -> (Value, Value) {
let v2 = self.stack.pop().unwrap();
let v1 = self.stack.pop().unwrap();
let v1_casted = builder.ins().bitcast(v1, ty.clone(), span);
let v2_casted = builder.ins().bitcast(v2, ty, span);
(v1_casted, v2_casted)
let v1 = if builder.data_flow_graph().value_type(v1) != &ty {
builder.ins().bitcast(v1, ty.clone(), span)
} else {
v1
};
let v2 = if builder.data_flow_graph().value_type(v2) != &ty {
builder.ins().bitcast(v2, ty, span)
} else {
v2
};
(v1, v2)
}

/// Pop three values. Return them in the order they were pushed.
Expand Down
4 changes: 2 additions & 2 deletions hir-analysis/src/data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ impl Analysis for GlobalVariableAnalysis<Program> {
) -> AnalysisResult<Self> {
let mut layout = GlobalVariableLayout {
global_table_offset: core::cmp::max(
program.reserved_memory_bytes(),
program.reserved_memory_bytes().next_multiple_of(32),
program.segments().next_available_offset(),
),
..GlobalVariableLayout::default()
Expand Down Expand Up @@ -73,7 +73,7 @@ impl Analysis for GlobalVariableAnalysis<Module> {
) -> AnalysisResult<Self> {
let mut layout = GlobalVariableLayout {
global_table_offset: core::cmp::max(
module.reserved_memory_bytes(),
module.reserved_memory_bytes().next_multiple_of(32),
module.segments().next_available_offset(),
),
..GlobalVariableLayout::default()
Expand Down
1 change: 1 addition & 0 deletions midenc-debug/src/ui/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ impl State {
.any(|bp| bp.id == candidate)
{
candidate = next;
next = candidate.wrapping_add(1);
continue;
}
self.next_breakpoint_id = next;
Expand Down
17 changes: 16 additions & 1 deletion sdk/alloc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ use core::{
#[cfg(target_family = "wasm")]
const PAGE_SIZE: usize = 2usize.pow(16);

/// We require all allocations to be minimally word-aligned, i.e. 32 byte alignment
const MIN_ALIGN: usize = 32;

/// The linear memory heap must not spill over into the region reserved for procedure
/// locals, which begins at 2^30 in Miden's address space.
const HEAP_END: *mut u8 = (2usize.pow(30) / 4) as *mut u8;
Expand Down Expand Up @@ -71,7 +74,19 @@ impl BumpAlloc {

unsafe impl GlobalAlloc for BumpAlloc {
unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
let layout = layout.pad_to_align();
// Force allocations to be at minimally word-aligned. This is wasteful of memory, but
// we don't need to be particularly conservative with memory anyway, as most, if not all,
// Miden programs will be relatively short-lived. This makes interop at the Rust/Miden
// call boundary less expensive, as we can typically pass pointers directly to Miden,
// whereas without this alignment guarantee, we would have to set up temporary buffers for
// Miden code to write to, and then copy out of that buffer to whatever Rust type, e.g.
// `Vec`, we actually want.
//
// NOTE: This cannot fail, because we're always meeting minimum alignment requirements
let layout = layout
.align_to(core::cmp::max(layout.align(), MIN_ALIGN))
.unwrap()
.pad_to_align();
let size = layout.size();
let align = layout.align();

Expand Down
10 changes: 9 additions & 1 deletion sdk/base-sys/src/bindings/tx/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,14 @@ pub fn get_inputs() -> Vec<Felt> {
const MAX_INPUTS: usize = 256;
let mut inputs: Vec<Felt> = Vec::with_capacity(MAX_INPUTS);
let num_inputs = unsafe {
// Ensure the pointer is a valid Miden pointer
//
// NOTE: This relies on the fact that BumpAlloc makes all allocations
// minimally word-aligned. Each word consists of 4 elements of 4 bytes,
// so to get a Miden address from a Rust address, we divide by 16 to get
// the address in words (dividing by 4 gets us an address in elements,
// and by 4 again we get the word address).
let ptr = (inputs.as_mut_ptr() as usize) / 16;
// The MASM for this function is here:
// https://github.com/0xPolygonMiden/miden-base/blob/3cbe8d59dcf4ccc9c380b7c8417ac6178fc6b86a/miden-lib/asm/miden/note.masm#L69-L102
// #! Writes the inputs of the currently execute note into memory starting at the specified
Expand All @@ -30,7 +38,7 @@ pub fn get_inputs() -> Vec<Felt> {
// #! - dest_ptr is the memory address to write the inputs.
// Compiler generated adapter code at call site will drop the returned dest_ptr
// and return the number of inputs
extern_note_get_inputs(inputs.as_mut_ptr())
extern_note_get_inputs(ptr as *mut Felt)
};
unsafe {
inputs.set_len(num_inputs);
Expand Down
Loading