8000 Incorrect Optimization Causes Mismatched Output in Verilator · Issue #6007 · verilator/verilator · GitHub
[go: up one dir, main page]
Skip to content

Incorrect Optimization Causes Mismatched Output in Verilator #6007

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.

Sign up for GitHub

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

Jump to bottom
Closed
sdjasj opened this issue May 16, 2025 · 4 comments · Fixed by #6015
Closed

Incorrect Optimization Causes Mismatched Output in Verilator #6007

sdjasj opened this issue May 16, 2025 · 4 comments · Fixed by #6015

Comments

@sdjasj
Copy link
Contributor
sdjasj commented May 16, 2025 •
edited
Loading

The following is the problematic code test.v.

`timescale 1ns/1ps
`define stop   $stop
`define checkd(gotv, expv) \
  do if ((gotv) !== (expv)) begin \
       $write("%%Error: %s:%0d:  got=%0d exp=%0d\n", `__FILE__, `__LINE__, (gotv), (expv)); \
       `stop; \
     end while(0);
module top (out33);

output wire [6:0] out33;

    assign out33 = (6'o66 <<< 32'hFFFF_FFFF);

    initial begin
      #10;
      `checkd(out33, '0);
      $write("*-* All Finished *-*\n");
      $finish; 
    end

endmodule

You can reproduce the issue with the following commands:

verilator --binary -Wno-lint --timing  test.v
./obj_dir/Vtest

Verilator output:

%Error: test.v:16:  got=27 exp=0
%Error: test.v:16: Verilog $stop
Aborting...
Aborted (core dumped)

However, Icarus Verilog completes successfully with *-* All Finished *-*. Moreover, examining the expression shows that the result should also be 0.

System details:

  • Verilator version: Verilator 5.037 devel rev v5.036-64-g2f6ecd185
  • OS: Ubuntu 22.04.5 LTS (Jammy), Kernel/Release: 22.04
@sdjasj sdjasj added the new New issue not seen by maintainers label May 16, 2025
@yTakatsukasa
Copy link
Member

It looks

verilator/src/V3Number.cpp

Line 1880 in 6b42d78

for (int bit = 0; bit < width(); ++bit) {

should be

    for (uint64_t bit = 0; bit < width(); ++bit) {

@wsnyder
Copy link
Member
wsnyder commented May 16, 2025

@sdjasj mind attempting a second pull with @yTakatsukasa (thanks) suggestion please?

@sdjasj
Copy link
Contributor Author
sdjasj commented May 16, 2025

No problem, I'm happy to help :) and thank you @yTakatsukasa for your fix suggestion.

@yTakatsukasa
Copy link
Member

verilator/src/V3Number.cpp

Line 1881 in 6b42d78

if (bit >= static_cast<int>(rhsval)) setBit(bit, lhs.bitIs(bit - rhsval));

Ah I also think static_cast in this line is not necessary if bit is uint64_t.

I'm not sure whether an obvious shortcut of if (rhsval >= width()) return *this; is beneficial or not.

@sdjasj sdjasj mentioned this issue May 17, 2025
Merged
wsnyder pushed a commit that referenced this issue May 17, 2025
@sdjasj
Fix arithmetic left-shift by constants over 32 bits (#6007) (#6015)
fc70053
@wsnyder wsnyder removed the new New issue not seen by maintainers label May 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix arithmetic left-shift by constants over 32 bits (#6007) sdjasj/verilator
3 participants
@wsnyder @yTakatsukasa @sdjasj
0