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

Merge ModuleItem and InterfaceItem #950

Merged

Conversation

taichi-ishitani
Copy link
Contributor

@taichi-ishitani taichi-ishitani commented Sep 7, 2024

For #793, generate constructions for module and interface need to be merged.
To do this, GenerateItem will be introduced and all items except for ModportDeclaration will be moved from ModuleItem and InterfaceItem to GenerateItem.

By merging ModuleItem and InterfaceItem, interface will be able to have interface instances (#908) and always_* blocks (#947).

resolve #947

@dalance
Copy link
Collaborator

dalance commented Sep 8, 2024

I think merging them is not so good because the items of them are not guaranteed the equivalence in the future even if they have the almost same items now.
If module only or interface only declaration is added in the future, additional semantic checking is required each time.

About #908, I think more specialized syntax may be required instead of mere interface instantiation.

@taichi-ishitani
Copy link
Contributor Author

This PR is also preparetion for #793.
I think #793 should be applied for both of module and interface but it is difficult to define the syntax for #793 without merging ModuleItem and ItnerfaceItem.

@dalance
Copy link
Collaborator

dalance commented Sep 9, 2024

I think #793 can be achieved by adding BlockItem individually.
But merging them at the moment and spliting them again in the future if syntax diverging occurs are acceptable too.
How do you think?

@taichi-ishitani
Copy link
Contributor Author

Module and interface have own syntax for generate constructs because ModuleItem and IterfaceItem are different.
I think we need to merge these generate syntax to introduce BlockItem. Currently, differences are

  • Module cannot have modport.
  • Interface has no sytnax for module/interface instantiation.

The 1st item can resolved by introducing a new limitation that modport cannot be declared in generate blocks. I think this is acceptable limitation.

The 2nd item can be resolved by introducing the existing sytnax for module instantiatoin to interface. But an additinal semantic check will be required like below.

match symbol.found.kind {
SymbolKind::Module(ref x) if !self.in_interface => {
params.append(&mut x.parameters.clone());
ports.append(&mut x.ports.clone());
check_port_connection = true;
}
SymbolKind::Interface(_) => (),
SymbolKind::SystemVerilog => (),
SymbolKind::GenericParameter(ref x) => {
if let GenericBoundKind::Proto(ref x) = x.bound {
if let Ok(symbol) = symbol_table::resolve((x, &symbol.found.namespace))
{
if let SymbolKind::ProtoModule(x) = symbol.found.kind {
params.append(&mut x.parameters.clone());
ports.append(&mut x.ports.clone());
check_port_connection = true;
} else {
self.errors.push(AnalyzerError::mismatch_type(
name,
"module or interface",
&symbol.found.kind.to_kind_name(),
self.text,
&arg.identifier.as_ref().into(),
));
}
}
}
}
_ => {
let expected = if self.in_interface {
"interface"
} else {
"module or interface"
};
self.errors.push(AnalyzerError::mismatch_type(
name,
expected,
&symbol.found.kind.to_kind_name(),
self.text,
&arg.identifier.as_ref().into(),
));
}
}

@dalance
Copy link
Collaborator

dalance commented Sep 9, 2024

OK. I agree merging the items.
I think GenerateGroup and GenerateItem may be better than ModuleInterfaceGroup and ModuleInterfaceItem at the view point of naming consistency.

@taichi-ishitani
Copy link
Contributor Author

Thank you for your feedback.
I've renamed ModuleItnerfaceItem to GenerateItem and moved ModportDelcaration to InterfaceItem.

@dalance
Copy link
Collaborator

dalance commented Sep 9, 2024

I removed the resolve link to #908 which requires more consideration.

@dalance dalance merged commit a77c7ee into veryl-lang:master Sep 9, 2024
7 checks passed
@taichi-ishitani taichi-ishitani deleted the merge_module_and_interface_item branch September 9, 2024 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] always_* blocks inside interface
2 participants