ignore delay statements in processes #46
Conversation
There was a problem hiding this comment.
Hello Anders. Thanks for the patch. Please make this conditioned on a command-line flag, e.g. --ignore-sim-statements or something else added to SynthesisSettings. I don't want the frontend silently ignoring a part of the input even if that's what other synthesis tools do.
|
|
||
| module top(input wire clk); | ||
|
|
||
| wire #10 a; |
There was a problem hiding this comment.
Huh, you can insert delay statements here?!
There was a problem hiding this comment.
indeed, this is a "net delay", 28.16 in the 2023 LRM. i've sure never used this, i just checked the BNF for where-ever it was possible and where slang might put in some AST node.
There was a problem hiding this comment.
agree on settings, will take a little bit to add this. no problem.
| @@ -684,6 +684,12 @@ ER EvalVisitor::visit(const ImmediateAssertionStatement &stmt) | |||
| return ER::Success; | |||
| } | |||
|
|
|||
| ER EvalVisitor::visit(const TimedStatement &stmt) | |||
| { | |||
| ER result = stmt.stmt.visit(*this); | |||
There was a problem hiding this comment.
We can't access SynthesisSettings here unless we make a provision for it. It's okay to ignore timed statements here unconditionally as the initial visitor will be reworked later anyway (it will be folded into the generic procedural visitor)
There was a problem hiding this comment.
sorry for the delay in communicating, i was away last week.
i took another look at this last night and it's going to be messy. the problem is that ProceduralVisitor needs access to settings. But Procedural visitor is constructed in multiple contexts that then would also need access to settings and so on. Adding further constructor arguments, which are in some cases a pretty long list, to all of these seems clunky. i'd be adding 10-20 lines of boilerplate to forward a bool to a single line of code. so i stopped for the moment to see if you had any thoughts.
do you have a better way to do this? is this what you want to see?
There was a problem hiding this comment.
The visitor already holds a reference to NetlistContext, I think we should add a way to get the settings from that, i.e. a new SynthesisSettings &settings; field
There was a problem hiding this comment.
I was in the area so I've made the change in c53bd23
There was a problem hiding this comment.
i was able to rebase on that change and it helped. i was also able to implement the feature in the ER EvalVistor easily enough (even if it is temporary)
i did add a 2nd commit that adds a basic .gitignore if you like it, other wish we can drop it.
There was a problem hiding this comment.
No, great, .gitignore looks useful
Sometimes code that is shared between sim and synthesis may contain various
delay statements. Yosys is primarily a synthesis tool, so to remain
compatible with other synthesis tools, simply ignore delay statements.
// will correctly initialize foo to 32'hdeadbeef, but will still
// display both values in order
int foo;
initial begin
povik#10 foo = 32'h0;
$display(foo);
povik#10 foo = 32'hdeadbeef;
$display(foo);
end
anders-code
force-pushed
the
ignore-delay-statements
branch
from
99db1f3 to
631d445
Compare
Sometimes code that is shared between sim and synthesis may contain various delay statements. Yosys is primarily a synthesis tool, so to remain compatible with other synthesis tools, simply ignore delay statements.