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

Raise error / warning on continous assignment to reg #1369

Open
veripoolbot opened this issue Nov 28, 2018 · 15 comments
Open

Raise error / warning on continous assignment to reg #1369

veripoolbot opened this issue Nov 28, 2018 · 15 comments

Comments

@veripoolbot
Copy link

@veripoolbot veripoolbot commented Nov 28, 2018


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 <wsnyder@wsnyder.org>
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.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Nov 29, 2018


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?

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Nov 30, 2018


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

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Dec 1, 2018


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2018-12-01T15:25:53Z


Fixed in git towards 4.008.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Dec 1, 2018


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2018-12-01T20:17:32Z


In 4.008.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Dec 4, 2018


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.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Aug 18, 2019


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());
             } // <--
         }

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Aug 27, 2019


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.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Aug 28, 2019


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 ?

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Aug 28, 2019


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.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Sep 3, 2019


Original Redmine Comment
Author Name: Kris Jeon
Original Date: 2019-09-03T11:23:51Z


Oh, where can I get the unreleased git version ?

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Sep 3, 2019


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2019-09-03T11:55:42Z


See https://www.veripool.org/projects/verilator/wiki/Installing

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Sep 3, 2019


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.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Sep 3, 2019


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.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Nov 27, 2019


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.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Dec 7, 2019


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...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.