From 756cb7f250096965ff436f9ccc18049f812ec614 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Wed, 14 Aug 2024 10:13:36 +0000 Subject: [PATCH 01/10] feat: Use typed labels --- .../src/brillig/brillig_gen.rs | 7 ++- .../src/brillig/brillig_gen/brillig_block.rs | 12 ++-- .../src/brillig/brillig_gen/brillig_fn.rs | 7 +-- .../brillig/brillig_gen/brillig_slice_ops.rs | 5 +- .../noirc_evaluator/src/brillig/brillig_ir.rs | 18 +++--- .../src/brillig/brillig_ir/artifact.rs | 57 ++++++++++++++--- .../brillig_ir/codegen_control_flow.rs | 14 ++--- .../src/brillig/brillig_ir/entry_point.rs | 27 ++++---- .../src/brillig/brillig_ir/instructions.rs | 61 ++++++++++--------- .../noirc_evaluator/src/brillig/mod.rs | 15 +++-- .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 4 +- 11 files changed, 135 insertions(+), 92 deletions(-) diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen.rs index ee61a9d13d3..540c6546ad0 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen.rs @@ -9,7 +9,10 @@ mod variable_liveness; use acvm::FieldElement; use self::{brillig_block::BrilligBlock, brillig_fn::FunctionContext}; -use super::brillig_ir::{artifact::BrilligArtifact, BrilligContext}; +use super::brillig_ir::{ + artifact::{BrilligArtifact, Label}, + BrilligContext, +}; use crate::ssa::ir::function::Function; /// Converting an SSA function into Brillig bytecode. @@ -21,7 +24,7 @@ pub(crate) fn convert_ssa_function( let mut function_context = FunctionContext::new(func); - brillig_context.enter_context(FunctionContext::function_id_to_function_label(func.id())); + brillig_context.enter_context(Label::Function(func.id())); for block in function_context.blocks.clone() { BrilligBlock::compile(&mut function_context, &mut brillig_context, block, &func.dfg); diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index 76ab613ed1f..5748d51b75a 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -1,3 +1,4 @@ +use crate::brillig::brillig_ir::artifact::Label; use crate::brillig::brillig_ir::brillig_variable::{ type_to_heap_value_type, BrilligArray, BrilligVariable, BrilligVector, SingleAddrVariable, }; @@ -93,7 +94,7 @@ impl<'block> BrilligBlock<'block> { /// This uses the current functions's function ID and the block ID /// Making the assumption that the block ID passed in belongs to this /// function. - fn create_block_label_for_current_function(&self, block_id: BasicBlockId) -> String { + fn create_block_label_for_current_function(&self, block_id: BasicBlockId) -> Label { Self::create_block_label(self.function_context.function_id, block_id) } /// Creates a unique label for a block using the function Id and the block ID. @@ -102,8 +103,8 @@ impl<'block> BrilligBlock<'block> { /// for us to create a unique label across functions and blocks. /// /// This is so that during linking there are no duplicates or labels being overwritten. - fn create_block_label(function_id: FunctionId, block_id: BasicBlockId) -> String { - format!("{function_id}-{block_id}") + fn create_block_label(function_id: FunctionId, block_id: BasicBlockId) -> Label { + Label::Block(function_id, block_id) } /// Converts an SSA terminator instruction into the necessary opcodes. @@ -761,9 +762,6 @@ impl<'block> BrilligBlock<'block> { .flat_map(|argument_id| self.convert_ssa_value(*argument_id, dfg).extract_registers()) .collect(); - // Create label for the function that will be called - let label_of_function_to_call = FunctionContext::function_id_to_function_label(func_id); - let variables_to_save = self.variables.get_available_variables(self.function_context); let saved_registers = self @@ -774,7 +772,7 @@ impl<'block> BrilligBlock<'block> { self.variables.dump_constants(); // Call instruction, which will interpret above registers 0..num args - self.brillig_context.add_external_call_instruction(label_of_function_to_call); + self.brillig_context.add_external_call_instruction(func_id); // Important: resolve after pre_call_save_registers_prep_args // This ensures we don't save the results to registers unnecessarily. diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_fn.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_fn.rs index f0752c80c46..c1abad17a8f 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_fn.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_fn.rs @@ -2,7 +2,7 @@ use iter_extended::vecmap; use crate::{ brillig::brillig_ir::{ - artifact::{BrilligParameter, Label}, + artifact::BrilligParameter, brillig_variable::{get_bit_size_from_ssa_type, BrilligVariable}, }, ssa::ir::{ @@ -44,11 +44,6 @@ impl FunctionContext { } } - /// Creates a function label from a given SSA function id. - pub(crate) fn function_id_to_function_label(function_id: FunctionId) -> Label { - function_id.to_string() - } - pub(crate) fn ssa_type_to_parameter(typ: &Type) -> BrilligParameter { match typ { Type::Numeric(_) | Type::Reference(_) => { diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_slice_ops.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_slice_ops.rs index 55679432b1f..e96797ec89a 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_slice_ops.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_slice_ops.rs @@ -371,7 +371,7 @@ mod tests { use crate::brillig::brillig_gen::brillig_block::BrilligBlock; use crate::brillig::brillig_gen::brillig_block_variables::BlockVariables; use crate::brillig::brillig_gen::brillig_fn::FunctionContext; - use crate::brillig::brillig_ir::artifact::BrilligParameter; + use crate::brillig::brillig_ir::artifact::{BrilligParameter, Label}; use crate::brillig::brillig_ir::brillig_variable::{ BrilligArray, BrilligVariable, BrilligVector, SingleAddrVariable, }; @@ -389,7 +389,8 @@ mod tests { builder.set_runtime(RuntimeType::Brillig); let ssa = builder.finish(); - let brillig_context = create_context(); + let mut brillig_context = create_context(ssa.main_id); + brillig_context.enter_context(Label::Block(ssa.main_id, Id::test_new(0))); let function_context = FunctionContext::new(ssa.main()); (ssa, function_context, brillig_context) diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir.rs index 21f8722c116..34832fc9df8 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir.rs @@ -23,6 +23,7 @@ mod codegen_stack; mod entry_point; mod instructions; +use artifact::Label; pub(crate) use instructions::BrilligBinaryOp; use self::{ @@ -84,9 +85,9 @@ pub(crate) struct BrilligContext { registers: BrilligRegistersContext, /// Context label, must be unique with respect to the function /// being linked. - context_label: String, + context_label: Label, /// Section label, used to separate sections of code - section_label: usize, + current_section: usize, /// Stores the next available section next_section: usize, /// IR printer @@ -99,8 +100,8 @@ impl BrilligContext { BrilligContext { obj: BrilligArtifact::default(), registers: BrilligRegistersContext::new(), - context_label: String::default(), - section_label: 0, + context_label: Label::EntryPoint, + current_section: 0, next_section: 1, debug_show: DebugShow::new(enable_debug_trace), } @@ -135,8 +136,9 @@ pub(crate) mod tests { use acvm::{BlackBoxFunctionSolver, BlackBoxResolutionError, FieldElement}; use crate::brillig::brillig_ir::{BrilligBinaryOp, BrilligContext}; + use crate::ssa::ir::function::FunctionId; - use super::artifact::{BrilligParameter, GeneratedBrillig}; + use super::artifact::{BrilligParameter, GeneratedBrillig, Label}; use super::{BrilligOpcode, ReservedRegisters}; pub(crate) struct DummyBlackBoxSolver; @@ -195,9 +197,9 @@ pub(crate) mod tests { } } - pub(crate) fn create_context() -> BrilligContext { + pub(crate) fn create_context(id: FunctionId) -> BrilligContext { let mut context = BrilligContext::new(true); - context.enter_context("test"); + context.enter_context(Label::Function(id)); context } @@ -208,7 +210,7 @@ pub(crate) mod tests { ) -> GeneratedBrillig { let artifact = context.artifact(); let mut entry_point_artifact = - BrilligContext::new_entry_point_artifact(arguments, returns, "test".to_string()); + BrilligContext::new_entry_point_artifact(arguments, returns, FunctionId::test_new(0)); entry_point_artifact.link_with(&artifact); entry_point_artifact.finish() } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs index 2d0bdb5955c..3f4a50e479b 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs @@ -1,7 +1,7 @@ use acvm::acir::brillig::Opcode as BrilligOpcode; use std::collections::{BTreeMap, HashMap}; -use crate::ssa::ir::dfg::CallStack; +use crate::ssa::ir::{basic_block::BasicBlockId, dfg::CallStack, function::FunctionId}; /// Represents a parameter or a return value of an entry point function. #[derive(Debug, Clone, Eq, PartialEq, Hash, PartialOrd, Ord)] @@ -57,7 +57,47 @@ pub(crate) type OpcodeLocation = usize; /// /// It is assumed that an entity will keep a map /// of labels to Opcode locations. -pub(crate) type Label = String; +#[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)] +pub(crate) enum Label { + /// Used to mark the start of functions, to make them callable. + Function(FunctionId), + /// Used to mark the start of blocks, to make them jumpable. + Block(FunctionId, BasicBlockId), + /// Used to mark sections inside blocks, to implement intra-block jumps. + BlockSection(FunctionId, BasicBlockId, usize), + /// Used to mark the start of the entrypoint code. + EntryPoint, + /// Used to mark the start of a section in the entrypoint code, to implement jumps in the entrypoint. + EntryPointSection(usize), +} + +impl Label { + pub(crate) fn can_add_section(&self) -> bool { + matches!(self, Label::Block(_, _) | Label::EntryPoint) + } + + pub(crate) fn add_section(&self, section: usize) -> Label { + match self { + Label::Block(func_id, block_id) => Label::BlockSection(*func_id, *block_id, section), + Label::EntryPoint => Label::EntryPointSection(section), + _ => unreachable!("cannot add section to label {self}"), + } + } +} + +impl std::fmt::Display for Label { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + match self { + Label::Function(id) => write!(f, "Function({})", id), + Label::Block(func_id, block_id) => write!(f, "Block({}, {})", func_id, block_id), + Label::BlockSection(func_id, block_id, section_id) => { + write!(f, "BlockSection({}, {}, {})", func_id, block_id, section_id) + } + Label::EntryPoint => write!(f, "EntryPoint"), + Label::EntryPointSection(section_id) => write!(f, "EntryPointSection({})", section_id), + } + } +} /// Pointer to a unresolved Jump instruction in /// the bytecode. pub(crate) type JumpInstructionPosition = OpcodeLocation; @@ -86,7 +126,7 @@ impl BrilligArtifact { /// Gets the first unresolved function call of this artifact. pub(crate) fn first_unresolved_function_call(&self) -> Option