Major Tools
Other Tools
General Info

Issue #487

Support short-circuiting of bitswise AND and OR

Added by Jeremy Bennett about 7 years ago. Updated over 6 years ago.

% Done:



Verilog 2001 (and I believe 2005) allows considerable flexibility in short-circuiting. IEEE 1364-2001, section 4.1.4 states:

"The operators shall follow the associativity rules while evaluating an expression as described in 4.1.2. How- ever, if the final result of an expression can be determined early, the entire expression need not be evaluated."

The standard does not make the precise circumstances explicit, but the example used is based on bitwise-AND where the first operand evaluates to zero. I understand that VCS short-circuits evaluation of bitwise AND and bitwise OR.

I have written a patch with regression test to implement this when the Verilator language specified is Verilog, but not SystemVerilog. This is an extension of the work in Issue 413, which implements short-circuiting compliant with the SystemVerilog IEEE 1800-2009 standard. Please pull the changes from: View - Test driver (516 Bytes) Jeremy Bennett, 04/24/2012 01:10 PM

t_gen_cond_v2k1_bitrange_and.v - Test code (1.61 KB) Jeremy Bennett, 04/24/2012 01:10 PM


#1 Updated by Wilson Snyder almost 7 years ago

  • Category set to Unsupported
  • Status changed from New to Assigned
  • Assignee set to Jeremy Bennett

Isn't short-circuiting optional in Verilog-2001? Either way I do not believe Verilator should do anything different WRT this in Verilog vs SystemVerilog unless the spec forces it, as that will just confuse people.

Anyhow the big problem is test case currently fails on both VCS and NC. (And fails identically with either SystemVerilog or Verilog 2005 mode.)

Also minor issues with the specific patch; first the should use verilator_flags2 not v_flags2 as the flags are verilator specific, and second testing the --language switch is incorrect; --language is just the default language if not specified; it can be overridden on a file-by-file basis, so the best thing to do would be to put the language setting into the FileLine flags, then test it at the appropriate point. (This hasn't been needed yet as there isn't anything that simulates differently.)

#2 Updated by Jeremy Bennett almost 7 years ago

I have to confess that I don't have a copy of the 2005 standard, but the 2001 standard definitely leaves it up to the user. It may be that 2005 tightened things up.

I have an example which passes with VCS, but fails with Verilator. Since this is commercially confidential, I extracted the test case, which passes with Icarus Verilog, but fails with Verilator. I don't routinely have access to VCS - does it pass with VCS if you specify 2001 as the Verilog version? Maybe I need to tighten down the language variant test.

My objective was that synthesizable code that is acceptable to VCS (or for that matter any major simulator) should also be OK with Verilator. Maybe I should make this issue a warning about short-circuiting that can be turned off?

I've changed the code on github to use verilator_flags2. I'll work on making the language test per module as you suggest - I see the comment in the definition of class FileLine about doing this.

#3 Updated by Wilson Snyder almost 7 years ago

VCS fails with $stop at line 88. NC complains about bit-select out of bounds on 67 as the AND isn't short circuited. Both are identical in v2001/2005/SV modes, emphasizing my point that changing behavior on the language isn't appropriate.

#4 Updated by Jeremy Bennett almost 7 years ago

Hmmm. Things get more and more messy.

VCS appears to do bitwise constant folding, but evidently gets the wrong answer for bitwise OR (the $stop should never be executed). It also seems that for SystemVerilog it is not standards compliant (short-circuiting bitwise AND is explicitly not allowed).

NC doesn't do constant folding. Icarus Verilog does constant folding and seems to get it right in both cases.

The original code I have only uses bitwise AND short-circuiting and the designers use VCS, so that explains why it works for them. The Verilog 2001 standard is not helpful by using bitwise AND as the example of where short-circuiting could be used - presumably why VCS does it.

I have a ton of legacy code which uses this, and with a very strong prohibition on not modifying the RTL. Should I add a new option to offer bitwise short-circuiting?

#5 Updated by Wilson Snyder almost 7 years ago

Can you post a test that shows what you just said; namely it passes in VCS and fails in NC? (Warnings not being considered failures, for this purpose.)

#6 Updated by Jeremy Bennett almost 7 years ago

As noted, I don't have VCS to check this, but I have reduced the test to just using bitwise-AND. This passes in Icarus Verilog. From your previous comment, I believe this should pass with VCS and fail with a range check with NC. With --verbose, the output from both Verilator is:

Bitwise AND generate if MASK [0] = 1
Bitwise AND generate if MASK [1] = 1
Bitwise AND generate if MASK [0] = 1
Bitwise AND generate if MASK [1] = 1
*-* All Finished *-*

and from IV is:

Bitwise AND generate if MASK [1] = 1
Bitwise AND generate if MASK [0] = 1
Bitwise AND generate if MASK [0] = 1
Bitwise AND generate if MASK [1] = 1
*-* All Finished *-*

Identical, apart from the order in which parallel always blocks are evaluated.

New test t_gen_cond_v2k1_bitrange_and.{pl,v} pushed to GitHub, and attached here for convenience.

#7 Updated by Jeremy Bennett almost 7 years ago

Sorry - I should have added, that is Verilator with my patch to enable bitwise AND short-circuiting with Verilog. It fails with standard Verilator as follows:
%Error: t/t_gen_cond_v2k1_bitrange_and.v:67: Selection index out of range: 2:2 outside 1:0
%Error: t/t_gen_cond_v2k1_bitrange_and.v:67: Selection index out of range: 3:3 outside 1:0
%Error: Exiting due to 2 error(s)

#8 Updated by Wilson Snyder almost 7 years ago

NC does print a warning, but then passes and VCS passes identically, so this test doesn't prove the difference you indicated. I think what you're seeing is VCS just doesn't warn about this case, it presumes Xs. If that's true then what Verilator should do is make it a warning instead of an error, and the user should turn off the warning.

#9 Updated by Jeremy Bennett almost 7 years ago

Thanks for running the tests. I'll update my patch accordingly.

#10 Updated by Jeremy Bennett over 6 years ago

Continuing the conversation from Bug 532 (sorry for duplicating)...

Although Verilog 2001 does not mandate short-circuiting of bitwise-AND and bitwise-OR, it does use bitwise-AND as an example. As you note, NC and VCS accept this with a warning.

I have legacy code that breaks with Verilator (generate loops that go out of range) because of this. I'll follow your suggestion and modify the patch, so it generates a warning, and the warning can be turned off.

Also available in: Atom