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

chore: deduplicate ReturnConstant warning #5109

Merged
merged 1 commit into from
May 28, 2024
Merged
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
75 changes: 31 additions & 44 deletions compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@
#[tracing::instrument(level = "trace", skip_all)]
pub(crate) fn into_acir(self, brillig: &Brillig) -> Result<Artifacts, RuntimeError> {
let mut acirs = Vec::new();
// TODO: can we parallelise this?

Check warning on line 287 in compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (parallelise)
let mut shared_context = SharedContext::default();
for function in self.functions.values() {
let context = Context::new(&mut shared_context);
Expand Down Expand Up @@ -1731,29 +1731,44 @@
) -> Result<Vec<SsaReport>, RuntimeError> {
let (return_values, call_stack) = match terminator {
TerminatorInstruction::Return { return_values, call_stack } => {
(return_values, call_stack)
(return_values, call_stack.clone())
}
// TODO(https://github.com/noir-lang/noir/issues/4616): Enable recursion on foldable/non-inlined ACIR functions
_ => unreachable!("ICE: Program must have a singular return"),
};

// The return value may or may not be an array reference. Calling `flatten_value_list`
// will expand the array if there is one.
let return_acir_vars = self.flatten_value_list(return_values, dfg)?;
let mut warnings = Vec::new();
for (acir_var, is_databus) in return_acir_vars {
if self.acir_context.is_constant(&acir_var) {
warnings.push(SsaReport::Warning(InternalWarning::ReturnConstant {
call_stack: call_stack.clone(),
}));
}
if !is_databus {
// We do not return value for the data bus.
self.acir_context.return_var(acir_var)?;
} else {
self.check_array_is_initialized(self.data_bus.return_data.unwrap(), dfg)?;
let mut has_constant_return = false;
for value_id in return_values {
let is_databus = self
.data_bus
.return_data
.map_or(false, |return_databus| dfg[*value_id] == dfg[return_databus]);
let value = self.convert_value(*value_id, dfg);

// `value` may or may not be an array reference. Calling `flatten` will expand the array if there is one.
let acir_vars = self.acir_context.flatten(value)?;
for (acir_var, _) in acir_vars {
has_constant_return |= self.acir_context.is_constant(&acir_var);
if is_databus {
// We do not return value for the data bus.
self.check_array_is_initialized(
self.data_bus.return_data.expect(
"`is_databus == true` implies `data_bus.return_data` is `Some`",
),
dfg,
)?;
} else {
self.acir_context.return_var(acir_var)?;
}
}
}

let warnings = if has_constant_return {
vec![SsaReport::Warning(InternalWarning::ReturnConstant { call_stack })]
} else {
Vec::new()
};

Ok(warnings)
}

Expand Down Expand Up @@ -2678,34 +2693,6 @@
}
}

/// Maps an ssa value list, for which some values may be references to arrays, by inlining
/// the `AcirVar`s corresponding to the contents of each array into the list of `AcirVar`s
/// that correspond to other values.
fn flatten_value_list(
&mut self,
arguments: &[ValueId],
dfg: &DataFlowGraph,
) -> Result<Vec<(AcirVar, bool)>, InternalError> {
let mut acir_vars = Vec::with_capacity(arguments.len());
for value_id in arguments {
let is_databus = if let Some(return_databus) = self.data_bus.return_data {
dfg[*value_id] == dfg[return_databus]
} else {
false
};
let value = self.convert_value(*value_id, dfg);
acir_vars.append(
&mut self
.acir_context
.flatten(value)?
.iter()
.map(|(var, _)| (*var, is_databus))
.collect(),
);
}
Ok(acir_vars)
}

/// Convert a Vec<AcirVar> into a Vec<AcirValue> using the given result ids.
/// If the type of a result id is an array, several acir vars are collected into
/// a single AcirValue::Array of the same length.
Expand Down
Loading