Skip to content

Commit

Permalink
Fix #13, and the first domain of a module implicitly names the clock
Browse files Browse the repository at this point in the history
- See #7

This change was inspired such that the sus-float work can use the aclk IP port more easily.
  • Loading branch information
VonTum committed Jan 14, 2025
1 parent 651ca72 commit 6735b28
Show file tree
Hide file tree
Showing 12 changed files with 64 additions and 28 deletions.
10 changes: 7 additions & 3 deletions src/codegen/system_verilog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
Expand Down Expand Up @@ -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
Expand All @@ -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);
Expand Down Expand Up @@ -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();
Expand Down
3 changes: 2 additions & 1 deletion src/codegen/vhdl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
6 changes: 3 additions & 3 deletions src/dev_aid/lsp/hover_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ pub fn hover(info: LocationInfo, linker: &Linker, file_data: &FileData) -> Vec<M
match info {
LocationInfo::InModule(_md_id, md, decl_id, InModule::NamedLocal(decl)) => {
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 {
Expand Down Expand Up @@ -152,7 +152,7 @@ pub fn hover(info: LocationInfo, linker: &Linker, file_data: &FileData) -> Vec<M
.0
));

let show_interfaces = submodule.is_multi_domain().then_some(InterfaceToDomainMap {
let show_interfaces = submodule.implicit_clk_domain.then_some(InterfaceToDomainMap {
local_domain_map: &submod.local_interface_domains,
domains: &md.domains,
});
Expand All @@ -170,7 +170,7 @@ pub fn hover(info: LocationInfo, linker: &Linker, file_data: &FileData) -> Vec<M
match wire.typ.domain {
DomainType::Generative => 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)))
}
Expand Down
2 changes: 1 addition & 1 deletion src/flattening/flatten.rs
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,7 @@ impl<'l, 'errs> FlatteningContext<'l, 'errs> {
fn alloc_submodule_instruction(&mut self, module_ref: GlobalReference<ModuleUUID>, 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{
Expand Down
32 changes: 26 additions & 6 deletions src/flattening/initialization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,23 @@ struct InitializationContext<'linker> {
// module-only stuff
ports: FlatAlloc<Port, PortIDMarker>,
interfaces: FlatAlloc<Interface, InterfaceIDMarker>,
domains: FlatAlloc<String, DomainIDMarker>,
domains: FlatAlloc<DomainInfo, DomainIDMarker>,
/// 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<StructField, FieldIDMarker>,

errors: ErrorCollector<'linker>,

file_text: &'linker FileText,
}

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();
Expand Down Expand Up @@ -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") => {
Expand Down Expand Up @@ -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,
};

Expand All @@ -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(),
});
Expand Down
13 changes: 7 additions & 6 deletions src/flattening/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,12 @@ pub struct Module {
pub ports: FlatAlloc<Port, PortIDMarker>,

/// Created in Stage 1: Initialization
pub domain_names: FlatAlloc<String, DomainIDMarker>,
pub domains: FlatAlloc<DomainInfo, DomainIDMarker>,
pub implicit_clk_domain: bool,

/// Created in Stage 1: Initialization
pub interfaces: FlatAlloc<Interface, InterfaceIDMarker>,

/// Created in Stage 2: Typechecking
pub domains: FlatAlloc<DomainInfo, DomainIDMarker>,

/// Created in Stage 3: Instantiation
pub instantiations: InstantiationCache,
}
Expand Down Expand Up @@ -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
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/flattening/typechecking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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())
}
Expand Down
1 change: 1 addition & 0 deletions src/instantiation/concrete_typecheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}"));
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/instantiation/latency_count.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
4 changes: 2 additions & 2 deletions src/to_string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,10 +160,10 @@ impl<T: Index<TypeUUID, Output = StructType>> 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:?}}}"),
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/typing/concrete_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions test.sus
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

0 comments on commit 6735b28

Please sign in to comment.