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: basic wallet account compilation #187

Merged
merged 4 commits into from
May 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 21 additions & 4 deletions codegen/masm/src/codegen/scheduler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -558,12 +558,15 @@ impl<'a> BlockScheduler<'a> {

/// We are visiting a `Node::Result` during planning, and need to determine whether or
/// not to materialize the result at this point, or if we must defer to another result of
/// the same instruction which is visited later in planning.
/// the same instruction which is visited later in planning, OR if the result will be
/// materialized but unused due to the instruction having side effects.
///
/// The name of this function refers to the fact that we may have to "force" materialization
/// of an instruction when the Result node has no predecessors in the graph, but is either
/// live _after_ the current block, or its instruction has side effects that require it to
/// be materialized anyway.
/// be materialized anyway. However, we only force materialize in the latter case if there
/// are no direct dependents on the instruction itself, so as to avoid materializing the
/// same instruction multiple times (once for the result, once for the dependent).
fn maybe_force_materialize_inst_results(&mut self, result: hir::Value, result_node: NodeId) {
let inst_node = self.block_info.depgraph.unwrap_child(result_node);
let inst = inst_node.unwrap_inst();
Expand All @@ -578,17 +581,24 @@ impl<'a> BlockScheduler<'a> {

// We're the first result scheduled, whether used or not. If the result is
// not actually used, we may need to force materialization if the following
// two conditions hold:
// three conditions hold:
//
// * There are no results used
// * The instruction has side effects
// * There are no dependencies on the instruction itself present in the graph, not counting
// the edges produced by results of the instruction.
//
// However, if any of the results are used, we must materialize them now, since
// we are scheduled before any of the others.
let is_used = inst_info.results.iter().any(|v| v.is_used());
let has_side_effects = self.f.dfg.inst(inst).has_side_effects();
let has_dependent_insts = self
.block_info
.depgraph
.predecessors(inst_node)
.any(|p| p.dependent.is_instruction());

if is_used || has_side_effects {
if is_used || (has_side_effects && !has_dependent_insts) {
self.materialize_inst_results(inst_info);
}
}
Expand Down Expand Up @@ -620,6 +630,13 @@ impl<'a> BlockScheduler<'a> {
return;
}

// If the result belongs to an instruction which is a treegraph node, then
// we never materialize the results, because they must already have been
// materialized
if self.block_info.treegraph.is_root(inst_node) {
return;
}

// We're the first use of the referenced instruction, so materialize its
// results, and drop any that have no uses.
self.materialize_inst_results(inst_info);
Expand Down
11 changes: 10 additions & 1 deletion codegen/masm/src/masm/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,14 +141,23 @@ impl Function {
kind: ast::InvokeKind,
target: FunctionIdent,
) {
let path = LibraryPath::new(target.module.as_str()).expect("invalid procedure path");
let module_name_span = miden_assembly::SourceSpan::new(
target.module.span.start_index().0..target.module.span.end_index().0,
);
let module_id = ast::Ident::new_unchecked(miden_assembly::Span::new(
module_name_span,
Arc::from(target.module.as_str().to_string().into_boxed_str()),
));
let name_span = miden_assembly::SourceSpan::new(
target.function.span.start_index().0..target.function.span.end_index().0,
);
let id = ast::Ident::new_unchecked(miden_assembly::Span::new(
name_span,
Arc::from(target.function.as_str().to_string().into_boxed_str()),
));
let path = LibraryPath::new(target.module.as_str()).unwrap_or_else(|_| {
LibraryPath::new_from_components(LibraryNamespace::Anon, [module_id])
});
let name = ast::ProcedureName::new_unchecked(id);
self.register_invoked(kind, ast::InvocationTarget::AbsoluteProcedurePath { name, path });
}
Expand Down
2 changes: 1 addition & 1 deletion frontend-wasm/src/miden_abi/transform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ pub fn return_via_pointer(
// expected to be +8 from the first and so on. So we need to
// multiply the index by 8 so that the subsequent Rust code finds
// the values in the expected locations.
let imm = Immediate::I32(idx as i32 * 8);
let imm = Immediate::U32(idx as u32 * 8);
builder.ins().add_imm_checked(ptr_u32, imm, span)
};
let value_ty = builder.data_flow_graph().value_type(*value).clone();
Expand Down
4 changes: 2 additions & 2 deletions hir/src/pass/rewrite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,9 @@ where
if self.0.should_apply(&function, session) {
dirty = true;
self.0.apply(&mut function, analyses, session)?;
} else {
analyses.mark_all_preserved::<crate::Function>(&function.id);
analyses.invalidate::<crate::Function>(&function.id);
}

// Add the function back to the module
//
// We add it before the current position of the cursor
Expand Down
Loading