verilator
Raise error / warning on continous assignment to reg
Author Name: Peter Gerst Original Redmine Issue: 1369 from https://www.veripool.org
Original Assignee: Wilson Snyder (@wsnyder)
Verilator does not throw error / warning on continuous assignment to a register. See the following example:
content of wire_test.v:
module wire_test (
input clk,
input rst,
input in1,
input in2,
input mux,
output reg out
);
// caught by verilator git version 15af70
// wire internal_reg;
reg internal_reg;
reg out_muxed;
always @(posedge clk) begin
if (rst) begin
internal_reg <= 1'b0;
end else begin
internal_reg <= in1;
end
end
assign out_muxed = (mux) ? internal_reg: in2;
assign out = (rst) ? out_muxed : 1'b0;
endmodule
Calling verilator as @verilator --lint-only wire_test.v@ does not complain about the assignment on internal_reg and out registers.
git version of verilator is used (last commit on Nov 26) but the issue is also present in version 4.006:
$ git log -n 1
commit 15af706286212c933595ba8228d2d334fb81e0f7 (HEAD -> master, origin/master, origin/HEAD)
Author: Wilson Snyder <[email protected]>
Date: Mon Nov 26 19:09:08 2018 -0500
Fix crash due to cygwin bug in getline, #.
This issue is in connection with forum message https://www.veripool.org/boards/3/topics/1559-Verilator-Verilator-fails-to-warn-error-on-procedural-assignment-to-wire that seems to be corrected: Procedural assignment to wire causes error message to be thrown using git version referenced above.
Original Redmine Comment Author Name: Wilson Snyder (@wsnyder) Original Date: 2018-11-29T23:12:10Z
I believe the code sent is legal in SystemVerilog, so no warning is appropriate. What complains and does it support SystemVerilog?
Original Redmine Comment Author Name: Peter Gerst Original Date: 2018-11-30T06:50:38Z
Xilinx ISE 14.7 syntheser complains about this:
ERROR:HDLCompiler:329 - "wire_test\wire_test.v" Line 23: Target <out_muxed> of concurrent assignment or output port connection should be a net type.
ERROR:HDLCompiler:329 - "wire_test\wire_test.v" Line 24: Target <out> of concurrent assignment or output port connection should be a net type.
ERROR:HDLCompiler:598 - "wire_test\wire_test.v" Line 1: Module <wire_test> ignored due to previous errors.
I need to use this syntheser for Spartan 6 targets. As far as I know it does not support SystemVerilog. (https://www.xilinx.com/support/documentation/sw_manuals/xilinx14_4/sim.pdf page 11 and 101)
I also forgot to mention that I use verilator with specifying the Verilog standard that ISE supports. For linting verilator is called in the following way:
verilator --lint-only --default-language 1364-2001 -Wall -Wno-PINCONNECTEMPTY wire_test.v
Original Redmine Comment Author Name: Wilson Snyder (@wsnyder) Original Date: 2018-12-01T15:25:53Z
Fixed in git towards 4.008.
Original Redmine Comment Author Name: Wilson Snyder (@wsnyder) Original Date: 2018-12-01T20:17:32Z
In 4.008.
Original Redmine Comment Author Name: Peter Gerst Original Date: 2018-12-04T06:46:18Z
Thank you for the fix! verilator now raises error on internal_reg but does not recognize the case of out signal in the example code above.
Original Redmine Comment Author Name: Kris Jeon Original Date: 2019-08-18T01:09:25Z
In order to raise the warning for port, I've changed like the following:
In 'V3ParseGrammar.cpp'
AstVar* V3ParseGrammar::createVariable(FileLine* fileline, string name,
AstNodeRange* arrayp, AstNode* attrsp) {
AstNodeDType* dtypep = GRAMMARP->m_varDTypep;
UINFO(5," creVar "<<name<<" decl="<<GRAMMARP->m_varDecl
<<" io="<<GRAMMARP->m_varIO<<" dt="<<(dtypep?"set":"")<<endl);
// added lines -->
if (v3Global.opt.lintOnly() && !fileline->language().systemVerilog()
&& GRAMMARP->m_varDecl == AstVarType::PORT && GRAMMARP->m_varIO == VDirection::OUTPUT && (!dtypep)) {
GRAMMARP->m_varDecl = AstVarType::WIRE;
} // <--
...
In 'V3ParseGrammar.cpp'
virtual void visit(AstNodeVarRef* nodep) {
// Any variable
if (nodep->lvalue()
&& !VN_IS(nodep, VarXRef)) { // Ignore interface variables and similar ugly items
if (m_inProcAssign && !nodep->varp()->varType().isProcAssignable()) {
nodep->v3warn(PROCASSWIRE, "Procedural assignment to wire, perhaps intended var"
" (IEEE 2017 6.5): "
+nodep->prettyName());
}
if (m_inContAssign && !nodep->varp()->varType().isContAssignable()
&& !nodep->fileline()->language().systemVerilog()) {
nodep->v3warn(CONTASSREG, "Continuous assignment to reg, perhaps intended wire"
" (IEEE 2005 6.1; Verilog only, legal in SV): "
+nodep->prettyName());
}
// added lines -->
if (v3Global.opt.lintOnly() && m_inContAssign && nodep->varp()->varType() == AstVarType::PORT
&& !nodep->fileline()->language().systemVerilog()) {
nodep->v3warn(CONTASSREG, "Continuous assignment to reg, perhaps intended wire"
" (IEEE 2005 6.1; Verilog only, legal in SV): "
+nodep->prettyName());
} // <--
}
Original Redmine Comment Author Name: Wilson Snyder (@wsnyder) Original Date: 2019-08-27T20:40:03Z
I wouldn't have thought your patch would be needed as the unreleased git version of verilator should have fixed this about a month ago. Can you please check it? If there is a problem with those warnings please file a new issue.
Original Redmine Comment Author Name: Kris Jeon Original Date: 2019-08-28T12:10:07Z
Hi,
It's been modified from 4.016. 4.016 version doesn't warn continuous assignments for the port (out) declared as output reg in wire_test.v.
%Error-CONTASSREG: wire_test.v:34: Continuous assignment to reg, perhaps intended wire (IEEE 2005 6.1; Verilog only, legal in SV): out_muxed
%Error: Exiting due to 1 error(s)
... See the manual and http://www.veripool.org/verilator for more assistance.
The modified version warns like the following:
%Error-CONTASSREG: wire_test.v:34: Continuous assignment to reg, perhaps intended wire (IEEE 2005 6.1; Verilog only, legal in SV): out_muxed
%Error-CONTASSREG: wire_test.v:35: Continuous assignment to reg, perhaps intended wire (IEEE 2005 6.1; Verilog only, legal in SV): out
%Error: Exiting due to 2 error(s)
... See the manual and http://www.veripool.org/verilator for more assistance.
I missed something ?
Original Redmine Comment Author Name: Wilson Snyder (@wsnyder) Original Date: 2019-08-28T12:34:45Z
Please use the unreleased git version, it intended to fix this.
Original Redmine Comment Author Name: Kris Jeon Original Date: 2019-09-03T11:23:51Z
Oh, where can I get the unreleased git version ?
Original Redmine Comment Author Name: Wilson Snyder (@wsnyder) Original Date: 2019-09-03T11:55:42Z
See https://www.veripool.org/projects/verilator/wiki/Installing
Original Redmine Comment Author Name: Kris Jeon Original Date: 2019-09-03T12:06:54Z
Is the git version different from tarball version ?I thought that they were the same. I'll have to use git version. Thank you for information.
Original Redmine Comment Author Name: Wilson Snyder (@wsnyder) Original Date: 2019-09-03T12:23:32Z
Git is the change-by-change repo, which is snapshotted for the tarballs.
Anyhow the version released this weekend has the change you want, so use the tarball if you want.
Original Redmine Comment Author Name: Peter Gerst Original Date: 2019-11-27T14:07:23Z
Warning about assigning to output reg does not work for me. I'm using tag 4.022 without success.
verilator --lint-only --default-language 1364-2001 -Wall -Wno-PINCONNECTEMPTY wire_test.v
warns only about out_muxed but not about out using the initial example code in this issue. There is no warning after declaration of out_muxed has been corrected from reg to wire.
Original Redmine Comment Author Name: Wilson Snyder (@wsnyder) Original Date: 2019-12-07T17:44:07Z
Sorry this has been such a mess, will make another pass. Getting this right is surprisingly difficult...
I think that the remainder of this issue is due to Verilator seeing the varType() of "out" as a port. It appears to me that Verilator doesn't differentiate between a reg output and a wire output.
Specifically this condition in V3Undriven cause the error to not be printed: !nodep->varp()->varType().isContAssignable()
That sounds roughly right. If you could please make a pull it would be appreciated, also please check against the verilator_ext_tests as these warnings tend to have false positives until properly tuned. Thanks!
Confirmed the code passes on all simulators I tried, as noted in the comments this is legal in SystemVerilog but not IEEE 1364.
As general policy Verilator doesn't plan at this time to add any warnings to enforce older-language-only constructs that are legal in later versions. I'm open though to taking pull requests if someone wants to contribute fixes to do that, including to this issue.