Skip to content

Commit

Permalink
remove type from Value::Global and cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
vezenovm committed Jan 8, 2025
1 parent 328004e commit 40c50fc
Show file tree
Hide file tree
Showing 11 changed files with 24 additions and 24 deletions.
2 changes: 1 addition & 1 deletion compiler/noirc_evaluator/src/acir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1892,7 +1892,7 @@ impl<'a> Context<'a> {
Value::Instruction { .. } | Value::Param { .. } => {
unreachable!("ICE: Should have been in cache {value_id} {value:?}")
}
Value::Global(_) => {
Value::Global => {
unreachable!("ICE: all globals should have been inlined");
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -649,7 +649,7 @@ impl<'block> BrilligBlock<'block> {
Value::Instruction { .. }
| Value::Param { .. }
| Value::NumericConstant { .. }
| Value::Global(_) => {
| Value::Global => {
unreachable!("unsupported function call type {:?}", dfg[*func])
}
},
Expand Down Expand Up @@ -1560,7 +1560,7 @@ impl<'block> BrilligBlock<'block> {
let value = &dfg[value_id];

match value {
Value::Global(_) => {
Value::Global => {
unreachable!("ICE: all globals should have been inlined");
}
Value::Param { .. } | Value::Instruction { .. } => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ pub(crate) fn collect_variables_of_value(
Value::Instruction { .. }
| Value::Param { .. }
| Value::NumericConstant { .. }
| Value::Global(_) => Some(value_id),
| Value::Global => Some(value_id),
// Functions are not variables in a defunctionalized SSA. Only constant function values should appear.
Value::ForeignFunction(_) | Value::Function(_) | Value::Intrinsic(..) => None,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ impl DependencyContext {
Value::Instruction { .. }
| Value::NumericConstant { .. }
| Value::Param { .. }
| Value::Global(_) => {
| Value::Global => {
panic!(
"calling non-function value with ID {func_id} in function {}",
function.name()
Expand Down Expand Up @@ -620,7 +620,7 @@ impl Context {
Value::Instruction { .. }
| Value::NumericConstant { .. }
| Value::Param { .. }
| Value::Global(_) => {
| Value::Global => {
panic!("At the point we are running disconnect there shouldn't be any other values as arguments")
}
}
Expand Down
6 changes: 3 additions & 3 deletions compiler/noirc_evaluator/src/ssa/ir/dfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,8 +325,8 @@ impl DataFlowGraph {
id
}

pub(crate) fn make_global(&mut self, typ: Type) -> ValueId {
self.values.insert(Value::Global(typ))
pub(crate) fn make_global(&mut self) -> ValueId {
self.values.insert(Value::Global)
}

/// Gets or creates a ValueId for the given FunctionId.
Expand Down Expand Up @@ -595,7 +595,7 @@ impl DataFlowGraph {
},
// TODO: Make this true and handle instruction simplifications with globals.
// Currently all globals are inlined as a temporary measure so this is fine to have as false.
Value::Global(_) => false,
Value::Global => false,
_ => true,
}
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_evaluator/src/ssa/ir/printer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ fn value(dfg: &DataFlowGraph, id: ValueId) -> String {
Value::Intrinsic(intrinsic) => intrinsic.to_string(),
Value::ForeignFunction(function) => function.clone(),
Value::Param { .. } | Value::Instruction { .. } => id.to_string(),
Value::Global(_) => {
Value::Global => {
format!("@{id}")
}
}
Expand Down
5 changes: 3 additions & 2 deletions compiler/noirc_evaluator/src/ssa/ir/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ pub(crate) enum Value {
/// other than generating different backend operations and being only accessible through Brillig.
ForeignFunction(String),

Global(Type),
Global,
}

impl Value {
Expand All @@ -77,7 +77,8 @@ impl Value {
Value::Function { .. } | Value::Intrinsic { .. } | Value::ForeignFunction { .. } => {
Cow::Owned(Type::Function)
}
Value::Global(typ) => Cow::Borrowed(typ),
// Value::Global(typ) => Cow::Borrowed(typ),
Value::Global => unreachable!("Global does not have a type associated with it. Fetch the actual value from the global context"),
}
}
}
6 changes: 3 additions & 3 deletions compiler/noirc_evaluator/src/ssa/opt/inlining.rs
Original file line number Diff line number Diff line change
Expand Up @@ -382,8 +382,8 @@ impl<'global> InlineContext<'global> {
let mut context = PerFunctionContext::new(&mut self, entry_point);
context.inlining_entry = true;

for (_, value) in ssa.globals.dfg.values_iter() {
context.context.builder.current_function.dfg.make_global(value.get_type().into_owned());
for _ in ssa.globals.dfg.values_iter() {
context.context.builder.current_function.dfg.make_global();
}

// The entry block is already inserted so we have to add it to context.blocks and add
Expand Down Expand Up @@ -488,7 +488,7 @@ impl<'function, 'global> PerFunctionContext<'function, 'global> {
Value::ForeignFunction(function) => {
self.context.builder.import_foreign_function(function)
}
Value::Global(_) => {
Value::Global => {
// TODO: Inlining the global into the function is only a temporary measure
// until Brillig gen with globals is working end to end
let new_id = match &self.context.globals.dfg[id] {
Expand Down
6 changes: 3 additions & 3 deletions compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ impl Context {
let new_function_id = self.new_ids.function_ids[&old_function.id()];
let new_function = &mut self.functions[new_function_id];

for (_, value) in globals.dfg.values_iter() {
new_function.dfg.make_global(value.get_type().into_owned());
for _ in globals.dfg.values_iter() {
new_function.dfg.make_global();
}

let mut reachable_blocks = PostOrder::with_function(old_function).into_vec();
Expand Down Expand Up @@ -196,7 +196,7 @@ impl IdMaps {
}
Value::Intrinsic(intrinsic) => new_function.dfg.import_intrinsic(*intrinsic),
Value::ForeignFunction(name) => new_function.dfg.import_foreign_function(name),
Value::Global(_) => {
Value::Global => {
// Globals are computed at compile-time and thus are expected to be remain normalized
// between SSA passes
old_value
Expand Down
8 changes: 4 additions & 4 deletions compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,8 @@ impl<'a> FunctionContext<'a> {
let mut builder = FunctionBuilder::new(function_name, function_id);
builder.set_runtime(runtime);

for (_, value) in shared_context.globals_builder.dfg.values_iter() {
builder.current_function.dfg.make_global(value.get_type().into_owned());
for _ in shared_context.globals_builder.dfg.values_iter() {
builder.current_function.dfg.make_global();
}

let definitions = HashMap::default();
Expand All @@ -139,8 +139,8 @@ impl<'a> FunctionContext<'a> {
self.builder.new_function(func.name.clone(), id, func.inline_type);
}

for (_, value) in self.shared_context.globals_builder.dfg.values_iter() {
self.builder.current_function.dfg.make_global(value.get_type().into_owned());
for _ in self.shared_context.globals_builder.dfg.values_iter() {
self.builder.current_function.dfg.make_global();
}

self.add_parameters_to_scope(&func.parameters);
Expand Down
3 changes: 1 addition & 2 deletions compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use serde_with::serde_as;

use crate::ssa::ir::{
function::{Function, FunctionId},
instruction::Instruction,
map::AtomicCounter,
printer::display_instruction,
value::Value,
Expand Down Expand Up @@ -118,7 +117,7 @@ impl Display for Ssa {
Value::Instruction { instruction, .. } => {
display_instruction(&self.globals.dfg, *instruction, f)?;
}
Value::Global(_) => {
Value::Global => {
panic!("Value::Global should only be in the function dfg");
}
_ => panic!("Expected only numeric constant or instruction"),
Expand Down

0 comments on commit 40c50fc

Please sign in to comment.