From 6735b28ddb96c55f85a4b6432c90b58157587f42 Mon Sep 17 00:00:00 2001 From: Lennart Van Hirtum Date: Tue, 14 Jan 2025 20:41:15 +0100 Subject: [PATCH] Fix #13, and the first domain of a module implicitly names the clock - See #7 This change was inspired such that the sus-float work can use the aclk IP port more easily. --- src/codegen/system_verilog.rs | 10 +++++--- src/codegen/vhdl.rs | 3 ++- src/dev_aid/lsp/hover_info.rs | 6 ++--- src/flattening/flatten.rs | 2 +- src/flattening/initialization.rs | 32 ++++++++++++++++++++----- src/flattening/mod.rs | 13 +++++----- src/flattening/typechecking.rs | 6 ++--- src/instantiation/concrete_typecheck.rs | 1 + src/instantiation/latency_count.rs | 2 +- src/to_string.rs | 4 ++-- src/typing/concrete_type.rs | 4 ++-- test.sus | 9 +++++++ 12 files changed, 64 insertions(+), 28 deletions(-) diff --git a/src/codegen/system_verilog.rs b/src/codegen/system_verilog.rs index 1beb673..c57b34b 100644 --- a/src/codegen/system_verilog.rs +++ b/src/codegen/system_verilog.rs @@ -131,9 +131,10 @@ impl<'g, 'out, Stream: std::fmt::Write> CodeGenerationContext<'g, 'out, Stream> let var_decl = typ_to_declaration(&w.typ, &to); + let clk_name = self.md.get_clock_name(); writeln!( self.program_text, - "/*latency*/ logic {var_decl}; always_ff @(posedge clk) begin {to} <= {from}; end" + "/*latency*/ logic {var_decl}; always_ff @(posedge {clk_name}) begin {to} <= {from}; end" ).unwrap(); } } @@ -281,6 +282,7 @@ impl<'g, 'out, Stream: std::fmt::Write> CodeGenerationContext<'g, 'out, Stream> } fn write_submodules(&mut self) { + let parent_clk_name = self.md.get_clock_name(); for (_id, sm) in &self.instance.submodules { let sm_md = &self.linker.modules[sm.module_uuid]; let sm_inst: &InstantiatedModule = sm @@ -293,8 +295,9 @@ impl<'g, 'out, Stream: std::fmt::Write> CodeGenerationContext<'g, 'out, Stream> self.program_text.write_str(&sm_inst.name).unwrap(); }; let sm_name = &sm.name; + let submodule_clk_name = sm_md.get_clock_name(); writeln!(self.program_text, " {sm_name}(").unwrap(); - write!(self.program_text, "\t.clk(clk)").unwrap(); + write!(self.program_text, "\t.{submodule_clk_name}({parent_clk_name})").unwrap(); for (port_id, iport) in sm_inst.interface_ports.iter_valids() { let port_name = wire_name_self_latency(&sm_inst.wires[iport.wire], self.use_latency); @@ -352,7 +355,8 @@ impl<'g, 'out, Stream: std::fmt::Write> CodeGenerationContext<'g, 'out, Stream> RealWireDataSource::Multiplexer { is_state, sources } => { let output_name = wire_name_self_latency(w, self.use_latency); let arrow_str = if is_state.is_some() { - writeln!(self.program_text, "always_ff @(posedge clk) begin").unwrap(); + let clk_name = self.md.get_clock_name(); + writeln!(self.program_text, "always_ff @(posedge {clk_name}) begin").unwrap(); "<=" } else { writeln!(self.program_text, "always_comb begin\n\t// Combinatorial wires are not defined when not valid. This is just so that the synthesis tool doesn't generate latches").unwrap(); diff --git a/src/codegen/vhdl.rs b/src/codegen/vhdl.rs index ccdb8c6..d69acbf 100644 --- a/src/codegen/vhdl.rs +++ b/src/codegen/vhdl.rs @@ -89,9 +89,10 @@ impl<'g, 'out, Stream: std::fmt::Write> CodeGenerationContext<'g, 'out, Stream> let mut it = self.instance.interface_ports.iter_valids().peekable(); let end = if it.peek().is_some() { ";" } else { "" }; + let clk_name = self.md.get_clock_name(); write!( self.program_text, - "{comment_text}entity {} is (\n{comment_text} port (\n clk : in std_logic{end}\n", + "{comment_text}entity {} is (\n{comment_text} port (\n {clk_name} : in std_logic{end}\n", instance_name ) .unwrap(); diff --git a/src/dev_aid/lsp/hover_info.rs b/src/dev_aid/lsp/hover_info.rs index cd20a0c..d80cc04 100644 --- a/src/dev_aid/lsp/hover_info.rs +++ b/src/dev_aid/lsp/hover_info.rs @@ -95,7 +95,7 @@ pub fn hover(info: LocationInfo, linker: &Linker, file_data: &FileData) -> Vec { let mut details_vec: Vec<&str> = Vec::with_capacity(5); - let domain_str = if md.is_multi_domain() { + let domain_str = if md.implicit_clk_domain { if let DomainType::Physical(ph) = decl.typ.domain { Some(DomainType::physical_to_string(ph, &md.domains)) } else { @@ -152,7 +152,7 @@ pub fn hover(info: LocationInfo, linker: &Linker, file_data: &FileData) -> Vec Vec details_vec.push(Cow::Borrowed("gen")), DomainType::Physical(ph) => { - if md.is_multi_domain() { + if md.implicit_clk_domain { details_vec .push(Cow::Owned(DomainType::physical_to_string(ph, &md.domains))) } diff --git a/src/flattening/flatten.rs b/src/flattening/flatten.rs index 3409568..3a53a88 100644 --- a/src/flattening/flatten.rs +++ b/src/flattening/flatten.rs @@ -546,7 +546,7 @@ impl<'l, 'errs> FlatteningContext<'l, 'errs> { fn alloc_submodule_instruction(&mut self, module_ref: GlobalReference, name: Option<(String, Span)>, documentation: Documentation) -> FlatID { let md = &self.globals[module_ref.id]; let local_interface_domains = md - .domain_names + .domains .map(|_| DomainType::Unknown(self.type_alloc.domain_variable_alloc.alloc())); self.instructions.alloc(Instruction::SubModule(SubModuleInstance{ diff --git a/src/flattening/initialization.rs b/src/flattening/initialization.rs index 7c3e2f9..178d906 100644 --- a/src/flattening/initialization.rs +++ b/src/flattening/initialization.rs @@ -22,11 +22,15 @@ struct InitializationContext<'linker> { // module-only stuff ports: FlatAlloc, interfaces: FlatAlloc, - domains: FlatAlloc, + domains: FlatAlloc, + /// This is initially true, but when the first `domain xyz` statement is encountered this is set to false. + implicit_clk_domain: bool, // struct-only stuff fields: FlatAlloc, + errors: ErrorCollector<'linker>, + file_text: &'linker FileText, } @@ -34,7 +38,7 @@ impl<'linker> InitializationContext<'linker> { fn gather_initial_global_object(&mut self, cursor: &mut Cursor) -> (Span, String) { let name_span = cursor.field_span(field!("name"), kind!("identifier")); let name = self.file_text[name_span].to_owned(); - self.domains.alloc(name.clone()); + self.domains.alloc(DomainInfo { name: "clk".to_string() }); if cursor.optional_field(field!("template_declaration_arguments")) { cursor.list(kind!("template_declaration_arguments"), |cursor| { let (kind, decl_span) = cursor.kind_span(); @@ -122,11 +126,25 @@ impl<'linker> InitializationContext<'linker> { cursor.list(kind!("block"), |cursor| { match cursor.kind() { kind!("domain_statement") => { + let whole_domain_statement_span = cursor.span(); cursor.go_down_no_check(|cursor| { let domain_name_span = cursor.field_span(field!("name"), kind!("identifier")); let name = &self.file_text[domain_name_span]; - self.domains.alloc(name.to_owned()) + + if self.implicit_clk_domain { + if let Some(existing_port) = self.ports.iter().next() { + // Sad Path: Having ports on the implicit clk domain is not allowed. + self.errors.error(whole_domain_statement_span, "When using explicit domains, no port is allowed to be declared on the implicit 'clk' domain.") + .info_same_file(existing_port.1.decl_span, "A domain should be explicitly defined before this port"); + } else { + // Happy Path: The implicit clk domain hasn't been used yet, so we can safely get rid of it. + self.domains.clear(); + } + + self.implicit_clk_domain = false; + } + self.domains.alloc(DomainInfo { name: name.to_owned() }) }); } kind!("interface_statement") => { @@ -298,8 +316,10 @@ fn initialize_global_object(builder: &mut FileBuilder, parsing_errors: ErrorColl ports: FlatAlloc::new(), interfaces: FlatAlloc::new(), domains: FlatAlloc::new(), + implicit_clk_domain: true, parameters: FlatAlloc::new(), fields: FlatAlloc::new(), + errors: parsing_errors, file_text: &builder.file_data.file_text, }; @@ -320,15 +340,15 @@ fn initialize_global_object(builder: &mut FileBuilder, parsing_errors: ErrorColl checkpoints: ArrayVec::new() }; - link_info.reabsorb_errors_globals((parsing_errors, ResolvedGlobals::empty()), AFTER_INITIAL_PARSE_CP); + link_info.reabsorb_errors_globals((ctx.errors, ResolvedGlobals::empty()), AFTER_INITIAL_PARSE_CP); match global_obj_kind { GlobalObjectKind::Module => { builder.add_module(Module { link_info, ports: ctx.ports, - domain_names: ctx.domains, - domains: FlatAlloc::new(), + domains: ctx.domains, + implicit_clk_domain: ctx.implicit_clk_domain, interfaces: ctx.interfaces, instantiations: InstantiationCache::new(), }); diff --git a/src/flattening/mod.rs b/src/flattening/mod.rs index 79a8034..2a755cc 100644 --- a/src/flattening/mod.rs +++ b/src/flattening/mod.rs @@ -55,14 +55,12 @@ pub struct Module { pub ports: FlatAlloc, /// Created in Stage 1: Initialization - pub domain_names: FlatAlloc, + pub domains: FlatAlloc, + pub implicit_clk_domain: bool, /// Created in Stage 1: Initialization pub interfaces: FlatAlloc, - /// Created in Stage 2: Typechecking - pub domains: FlatAlloc, - /// Created in Stage 3: Instantiation pub instantiations: InstantiationCache, } @@ -126,8 +124,11 @@ impl Module { } } - pub fn is_multi_domain(&self) -> bool { - self.domains.len() > 1 + /// Temporary upgrade such that we can name the singular clock of the module, such that weirdly-named external module clocks can be used + /// + /// See #7 + pub fn get_clock_name(&self) -> &str { + &self.domains.iter().next().unwrap().1.name } } diff --git a/src/flattening/typechecking.rs b/src/flattening/typechecking.rs index 5b4e2cc..0769dbc 100644 --- a/src/flattening/typechecking.rs +++ b/src/flattening/typechecking.rs @@ -518,7 +518,7 @@ pub fn apply_types( ) { // Set the remaining domain variables that aren't associated with a module port. // We just find domain IDs that haven't been - let mut leftover_domain_alloc = UUIDAllocator::new_start_from(working_on.domain_names.get_next_alloc_id()); + let mut leftover_domain_alloc = UUIDAllocator::new_start_from(working_on.domains.get_next_alloc_id()); for d in type_checker.domain_substitutor.iter() { if d.get().is_none() { assert!(d.set(DomainType::Physical(leftover_domain_alloc.alloc())).is_ok()); @@ -528,8 +528,8 @@ pub fn apply_types( // Assign names to all of the domains in this module working_on.domains = leftover_domain_alloc.into_range().map(|id| DomainInfo { name: { - if let Some(name) = working_on.domain_names.get(id) { - name.clone() + if let Some(name) = working_on.domains.get(id) { + name.name.clone() } else { format!("domain_{}", id.get_hidden_value()) } diff --git a/src/instantiation/concrete_typecheck.rs b/src/instantiation/concrete_typecheck.rs index b74aaf9..c294ae2 100644 --- a/src/instantiation/concrete_typecheck.rs +++ b/src/instantiation/concrete_typecheck.rs @@ -111,6 +111,7 @@ impl<'fl, 'l> InstantiationContext<'fl, 'l> { let typ_as_str = w.typ.display(&self.linker.types); let span = self.md.get_instruction_span(w.original_instruction); + span.debug(); self.errors.error(span, format!("Could not finalize this type, some parameters were still unknown: {typ_as_str}")); } } diff --git a/src/instantiation/latency_count.rs b/src/instantiation/latency_count.rs index 71ad336..f433ad9 100644 --- a/src/instantiation/latency_count.rs +++ b/src/instantiation/latency_count.rs @@ -205,7 +205,7 @@ impl<'fl, 'l> InstantiationContext<'fl, 'l> { continue; }; - for domain_id in self.linker.modules[sm.module_uuid].domain_names.id_range() { + for domain_id in self.linker.modules[sm.module_uuid].domains.id_range() { struct Prev { first: (WireID, i64), prev: (WireID, i64), diff --git a/src/to_string.rs b/src/to_string.rs index 4eca7a2..e89e01b 100644 --- a/src/to_string.rs +++ b/src/to_string.rs @@ -160,10 +160,10 @@ impl> Display for ConcreteTypeDisplay<'_ f, "{}[{}]", elem_typ.display(self.linker_types), - arr_size.unwrap_value().unwrap_integer() + arr_size.display(self.linker_types) ) } - ConcreteType::Value(v) => write!(f, "{{concrete_type_{v}}}"), + ConcreteType::Value(v) => write!(f, "{v}"), ConcreteType::Unknown(u) => write!(f, "{{{u:?}}}"), } } diff --git a/src/typing/concrete_type.rs b/src/typing/concrete_type.rs index a6504ed..59e7608 100644 --- a/src/typing/concrete_type.rs +++ b/src/typing/concrete_type.rs @@ -35,13 +35,13 @@ impl ConcreteType { #[track_caller] pub fn unwrap_value(&self) -> &Value { let ConcreteType::Value(v) = self else { - unreachable!("unwrap_value") + unreachable!("unwrap_value on {self:?}") }; v } pub fn down_array(&self) -> &ConcreteType { let ConcreteType::Array(arr_box) = self else { - unreachable!("Must be an array!") + unreachable!("Must be an array! Is {self:?} instead") }; let (sub, _sz) = arr_box.deref(); sub diff --git a/test.sus b/test.sus index 002de01..8e52adb 100644 --- a/test.sus +++ b/test.sus @@ -1011,3 +1011,12 @@ module use_sized_int_add { c = sized_int_add(a, b) } + + +module implicit_domain_forbidden { + input int bad_port + + domain bad_domain + + output int port_after_domain +}