Fix netlist creation when code contains concurrent assertions #1005
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1005 +/- ##
=======================================
Coverage 94.70% 94.70%
=======================================
Files 191 191
Lines 47569 47593 +24
=======================================
+ Hits 45048 45071 +23
- Misses 2521 2522 +1 see 4 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
| @@ -605,6 +605,14 @@ class NetlistVisitor : public ast::ASTVisitor<NetlistVisitor, true, false> { | |||
| ast::EdgeKind edgeKind = ast::EdgeKind::None; | |||
| if (symbol.procedureKind == ast::ProceduralBlockKind::AlwaysFF || | |||
| symbol.procedureKind == ast::ProceduralBlockKind::Always) { | |||
| // auto& body = symbol.getBody(); | |||
| if (symbol.getBody().kind == ast::StatementKind::Block) { | |||
There was a problem hiding this comment.
Do you want to just ignore all statements that aren't a TimedStatement? Otherwise you'll still crash below.
There was a problem hiding this comment.
As I'm not sure what elements can be nested in the AST, I would rather get a fault during debugging rather than silently skipping netlist elements by mistake.
At the moment, I'm explicitly checking for those AST elements I know I need to skip, in this case a concurrent assertion under a sequential block.
Can concurrent assertions be found elsewhere?
tools/netlist/tests/NameTests.cpp
Outdated
| @@ -55,3 +55,40 @@ endmodule | |||
| auto netlist = createNetlist(compilation); | |||
| CHECK(netlist.numNodes() > 0); | |||
| } | |||
|
|
|||
| TEST_CASE("Ignore concurrent assertions") { | |||
| // Test that unused modules are not visited by the netlist builder. | |||
There was a problem hiding this comment.
Comment is copy pasta from the previous test case.
There was a problem hiding this comment.
fixed locally, will be pushed when I have more meaningful changes later.
|
What's the status of this PR? |
|
I apologize, after being blocked by #1007 I forgot about this, thanks for reminding me. |
udif
force-pushed
the
pr_netlist_prop_err
branch
from
b8b2b26
to
9a27a95
Compare
Also fixed test in NameTest.cpp due to more strict slang errors.
|
I think it is OK now. |
This is the beginning of a PR to fix #1002 .
It works for a simple testcase that fails otherwise, but I still see different failures (DirectedGraph issue, might be unrelated) when reading larger examples such as those provided in #1002, so I don't think it is ready yet, but it's a start and it can be reviewed.