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

Issue in AND-reduction #2300

Closed
jackkoenig opened this issue Apr 30, 2020 · 4 comments
Closed

Issue in AND-reduction #2300

jackkoenig opened this issue Apr 30, 2020 · 4 comments
Labels
area: wrong runtime result Issue involves an incorrect runtine result from Verilated model resolution: fixed Closed; fixed

Comments

@jackkoenig
Copy link

jackkoenig commented Apr 30, 2020

I have created a repo that illustrates the issue: https://github.com/jackkoenig/verilator-and-reduction-bug
Commit of the above repo at time of filing issue: 9a38b944bd9909d32611ed3c0a920a3f49170fd5

Pasted from README:

It seems there is a bug in handling of AND-reduction (and possibly other reductions, I haven't checked) for wires with >64-bits. I have confirmed the buggy behavior in versions >= 4.026.

This test case compares the results of equivalent operations AND-reduce and checking equality with all ones for a 68-bit register:

  assign and_reduce = &r;
  assign check_equal = r == 68'hffff_ffff_ffff_ffff_f;

To reproduce, setup verilator and run:

make run

On versions < 4.026 you will see the correct output:

reset = 0, r = 00000000000000000, and_reduce = 0, check_equal = 0
reset = 0, r = ffffffffffffffffe, and_reduce = 0, check_equal = 0
reset = 0, r = fffffffffffffffff, and_reduce = 1, check_equal = 1

For versions >= 4.026, you will see the incorrect output:

reset = 0, r = 00000000000000000, and_reduce = 1, check_equal = 0
reset = 0, r = ffffffffffffffffe, and_reduce = 0, check_equal = 0
reset = 0, r = fffffffffffffffff, and_reduce = 0, check_equal = 1
@jackkoenig jackkoenig added the new New issue not seen by maintainers label Apr 30, 2020
@wsnyder wsnyder added area: runtime result resolution: fixed Closed; fixed and removed new New issue not seen by maintainers labels Apr 30, 2020
@wsnyder
Copy link
Member

wsnyder commented Apr 30, 2020

Thanks for the good test. This should have been covered in the test suite, sorry for the hole.

Fixed towards v4.034.

@albert-magyar
Copy link

Thanks! I have to say, the tree dumps with --debug make it really easy to track issues like this -- you can just watch the REDAND go away after expand!

jackkoenig added a commit to chipsalliance/firrtl that referenced this issue May 1, 2020
Workaround for verilator/verilator#2300
present in Verilator versions v4.026 - v4.032. This transform turns AND
reductions for expressions > 64-bits into an equality check with all
ones. It is included in all Verilog emitters.
@ingallsj
Copy link

ingallsj commented May 5, 2020

Did this also affect bitwise-or/xor/xnor reductions?

@wsnyder
Copy link
Member

wsnyder commented May 5, 2020

No, I tested the others too and didn't find any issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: wrong runtime result Issue involves an incorrect runtine result from Verilated model resolution: fixed Closed; fixed
Projects
None yet
Development

No branches or pull requests

4 participants