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

feat: don't simplify SSA instructions when creating them from a string #6948

Merged
merged 3 commits into from
Jan 6, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
26 changes: 20 additions & 6 deletions compiler/noirc_evaluator/src/ssa/function_builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ pub(crate) struct FunctionBuilder {
finished_functions: Vec<Function>,
call_stack: CallStackId,
error_types: BTreeMap<ErrorSelector, HirType>,

/// Whether instructions are simplified as soon as they are inserted into this builder.
/// This is true by default unless changed to false after constructing a builder.
pub(crate) simplify: bool,
}

impl FunctionBuilder {
Expand All @@ -56,6 +60,7 @@ impl FunctionBuilder {
finished_functions: Vec::new(),
call_stack: CallStackId::root(),
error_types: BTreeMap::default(),
simplify: true,
}
}

Expand Down Expand Up @@ -172,12 +177,21 @@ impl FunctionBuilder {
ctrl_typevars: Option<Vec<Type>>,
) -> InsertInstructionResult {
let block = self.current_block();
self.current_function.dfg.insert_instruction_and_results(
instruction,
block,
ctrl_typevars,
self.call_stack,
)
if self.simplify {
self.current_function.dfg.insert_instruction_and_results(
instruction,
block,
ctrl_typevars,
self.call_stack,
)
} else {
self.current_function.dfg.insert_instruction_and_results_without_simplification(
instruction,
block,
ctrl_typevars,
self.call_stack,
)
}
}

/// Switch to inserting instructions in the given block.
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -730,7 +730,7 @@ mod tests {
return v2
}
"#;
let ssa = Ssa::from_str(src).unwrap();
let ssa = Ssa::from_str_simplifying(src).unwrap();
TomAFrench marked this conversation as resolved.
Show resolved Hide resolved

let expected = r#"
brillig(inline) fn main f0 {
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1045,7 +1045,7 @@ mod test {
return
}
";
let ssa = Ssa::from_str(src).unwrap();
let ssa = Ssa::from_str_simplifying(src).unwrap();

let expected = "
acir(inline) fn main f0 {
Expand Down
1 change: 1 addition & 0 deletions compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1081,6 +1081,7 @@ mod test {
v23 = mul v20, Field 6
v24 = mul v21, v16
v25 = add v23, v24
enable_side_effects v0
enable_side_effects v3
v26 = cast v3 as Field
v27 = cast v0 as Field
Expand Down
13 changes: 7 additions & 6 deletions compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,13 @@ impl Context {
.requires_ctrl_typevars()
.then(|| vecmap(old_results, |result| old_function.dfg.type_of_value(*result)));

let new_results = new_function.dfg.insert_instruction_and_results(
instruction,
new_block_id,
ctrl_typevars,
new_call_stack,
);
let new_results =
new_function.dfg.insert_instruction_and_results_without_simplification(
instruction,
new_block_id,
ctrl_typevars,
new_call_stack,
);

assert_eq!(old_results.len(), new_results.len());
for (old_result, new_result) in old_results.iter().zip(new_results.results().iter())
Expand Down
11 changes: 6 additions & 5 deletions compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ use super::{
};

impl ParsedSsa {
pub(crate) fn into_ssa(self) -> Result<Ssa, SsaError> {
Translator::translate(self)
pub(crate) fn into_ssa(self, simplify: bool) -> Result<Ssa, SsaError> {
Translator::translate(self, simplify)
}
}

Expand All @@ -41,8 +41,8 @@ struct Translator {
}

impl Translator {
fn translate(mut parsed_ssa: ParsedSsa) -> Result<Ssa, SsaError> {
let mut translator = Self::new(&mut parsed_ssa)?;
fn translate(mut parsed_ssa: ParsedSsa, simplify: bool) -> Result<Ssa, SsaError> {
let mut translator = Self::new(&mut parsed_ssa, simplify)?;

// Note that the `new` call above removed the main function,
// so all we are left with are non-main functions.
Expand All @@ -53,12 +53,13 @@ impl Translator {
Ok(translator.finish())
}

fn new(parsed_ssa: &mut ParsedSsa) -> Result<Self, SsaError> {
fn new(parsed_ssa: &mut ParsedSsa, simplify: bool) -> Result<Self, SsaError> {
// A FunctionBuilder must be created with a main Function, so here wer remove it
// from the parsed SSA to avoid adding it twice later on.
let main_function = parsed_ssa.functions.remove(0);
let main_id = FunctionId::test_new(0);
let mut builder = FunctionBuilder::new(main_function.external_name.clone(), main_id);
builder.simplify = simplify;
builder.set_runtime(main_function.runtime_type);

// Map function names to their IDs so calls can be resolved
Expand Down
18 changes: 13 additions & 5 deletions compiler/noirc_evaluator/src/ssa/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,24 @@ mod token;

impl Ssa {
/// Creates an Ssa object from the given string.
///
/// Note that the resulting Ssa might not be exactly the same as the given string.
/// This is because, internally, the Ssa is built using a `FunctionBuilder`, so
/// some instructions might be simplified while they are inserted.
pub(crate) fn from_str(src: &str) -> Result<Ssa, SsaErrorWithSource> {
Self::from_str_impl(src, false)
}

/// Creates an Ssa object from the given string but trying to simplify
/// each parsed instruction as it's inserted into the final SSA.
pub(crate) fn from_str_simplifying(src: &str) -> Result<Ssa, SsaErrorWithSource> {
Self::from_str_impl(src, true)
}

fn from_str_impl(src: &str, simplify: bool) -> Result<Ssa, SsaErrorWithSource> {
let mut parser =
Parser::new(src).map_err(|err| SsaErrorWithSource::parse_error(err, src))?;
let parsed_ssa =
parser.parse_ssa().map_err(|err| SsaErrorWithSource::parse_error(err, src))?;
parsed_ssa.into_ssa().map_err(|error| SsaErrorWithSource { src: src.to_string(), error })
parsed_ssa
.into_ssa(simplify)
.map_err(|error| SsaErrorWithSource { src: src.to_string(), error })
}
}

Expand Down
12 changes: 12 additions & 0 deletions compiler/noirc_evaluator/src/ssa/parser/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -502,3 +502,15 @@ fn test_function_type() {
";
assert_ssa_roundtrip(src);
}

#[test]
fn test_does_not_simplify() {
let src = "
acir(inline) fn main f0 {
b0():
v2 = add Field 1, Field 2
return v2
}
";
assert_ssa_roundtrip(src);
}
Loading