Verilator

I knew about the side effect part related to ++ and --, but this is an interesting case. I'm surprised that & also short circuits.

What Verilator is seeing is the MASK[CONSTANT] and throwing the warning on that in isolation. It performs this check before it processes the block as part of the data type resolution, so doesn't (yet) know it won't be executed.

The fix isn't obvious, it has to process the expression before evaluating it due to Verilog's nasty width resolution rules. Maybe it needs to remember the error and only throw it later, or in the case of generates check later, but not too late as all of the constants will soon be folded in. This'll need some thought.

I'm looking further into this bug. For convenience, I have created a rather simpler patch, which I attach as a diff against git HEAD (commit 0ae00fc921f18fc3dbcd3a50400f8e3c528f5c51).

Here is a patch to deal with this. It only affects generated if, since the test in generated for loops is restricted to simple comparisons at present in Verilator, while in generated case blocks, the dependency is on a constant expression, with much less scope for short-circuiting.

We set a flag (m_generate) in WidthVisitor to indicate that we are inside a generate block for which selection width errors should be ignored. We don't print the error and we don't reduce the width in this case.

1. In replaceConst we finally check widths. By this stage we know we are being used.
2. When evaluating logical AND or OR we don't iterate the children if the generate flag is set. Instead we iterate just the first child, and if its result permits short-circuiting we need not evaluate the second child. Otherwise we continue as normal.

To make this work, I had to modify astgen. I added an optional third argument to TREEOP, which I used to indicate the visit function should not be generated automatically. I could then substitute hand-written versions for AstLogAnd, AstAnd, AstLogOr and AstOr to do short-circuiting. I'm not a PERL expert, but ultimately it would be good to provide this automatically through a third argument.

I've extended the test case to check all the possible short-circuiting operations.

Supplied as a git patch against current HEAD (commit 4a5e775a2b5ff03232053c436ee384975682ed10).

Updated patch, which has a modified TREEOP to generate short-circuited visitors. Various improvements to the C++ code as suggested by email.

For general short-circuiting, the C compiler will presumably do this, even if we do not. However we may be able to generate smaller programs if we can short-circuit constants within Verilator to prune unused code. Potentially that opens up other optimizations (pruning code often does).

I haven't tried the DPI short-circuit test yet.

Some more changes, following email discussion. This is now strictly IEEE 1800-2009. Short-circuiting is used only for && || -> and ?:, and is applied both to generate IF statements and expressions in general.

I've added a new set of tests to check that this fails when it should. For example with bitwise & and | and where there are selection width errors remaining after short-circuiting.

Attached as a tar and bzipped git format-patch against current HEAD (commit 7eb407fe2bc8438fdbc8dd8ac7614b423758d237)

I ran "make test" and it showed two failures t_lint_unused_bad and t_lint_syncasyncnet_bad, related to short circuiting suppressing variable unused warnings. To match what GCC and other tools do, "0 && a" should still consider "a" as a used variable. I realized though this was broken related to my last set of suggestions, and just needed to include m_doVConst in astgen.

Everything is merged in, so please "git pull" and start from the mainline before sending other patches, thanks. (BTW your using a github account would be useful.)

Fixed in git towards 3.840.

In 3.840.