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

Icarus only using the lowest 32 bits of right shift operand #19

Closed
cliffordwolf opened this issue Feb 27, 2014 · 10 comments
Closed

Icarus only using the lowest 32 bits of right shift operand #19

cliffordwolf opened this issue Feb 27, 2014 · 10 comments

Comments

@cliffordwolf
Copy link
Contributor

@cliffordwolf cliffordwolf commented Feb 27, 2014

The following module should set the output to constant 4'b0000:

module issue_029(y);
  output [3:0] y;
  assign y = 4'b1 << 33'h100000000;
endmodule

But Icarus Verilog (git ed2e339) assigns 4'b0001 instead.

(Sorry for another "stupidly large right shift operand" issue, but that's what keeps coming up in my test cases.)

@martinwhitaker
Copy link
Collaborator

@martinwhitaker martinwhitaker commented Feb 28, 2014

Yes, there's still a number of places where the compiler assumes the user won't write insane code. Unfortunately automated regression test generators tend not to be so obliging!

I've just pushed a fix for this one to the git master branch.

@cliffordwolf
Copy link
Contributor Author

@cliffordwolf cliffordwolf commented Mar 1, 2014

Thanks for the bugfix. It still seems to be broken in the interpreter though:

module issue_029(y);
  output [3:0] y;
  wire a;
  assign y = 4'b1 << (a === 1'bx ? 33'h100000000 : 0);
  initial #1 $display("%b", y);
endmodule

Unfortunately other auto-generated code (such as from HLS tools) sometimes also contain "strange code". And that is usually exactly the kind of code where you do not want to start hunting for bugs in the simulator..

@martinwhitaker
Copy link
Collaborator

@martinwhitaker martinwhitaker commented Mar 1, 2014

I think it's working correctly with this latest example. 'a' has the value 1'bz (because it is undriven), so your expression collapses to y = 4'b1 << 0. Icarus is giving the the result 4'b0001.

To get the result I think you are expecting, either change "wire a" to "reg a" or change "a === 1'bx" to "a === 1'bz". In both cases Icarus gives the result 4'b0000.

On your other comment, I absolutely agree with you. The last think one wants from any compiler or simulator is for it to silently generate the wrong results (better for it to crash - at least you know something is wrong!). So I do encourage you to keep reporting any bugs you find, however obscure.

@cliffordwolf
Copy link
Contributor Author

@cliffordwolf cliffordwolf commented Mar 2, 2014

To get the result I think you are expecting, either change "wire a" to "reg a" or change "a === 1'bx" to "a === 1'bz". In both cases Icarus gives the result 4'b0000.

Oops. You are of course right.

Here is a testcase that actually demonstrates a bug:

module test(y);
  output y;
  wire a = 1;
  assign y = 1 >> {a, 64'b0};
  initial #1 $display("%b", y);
endmodule

Returns 0 in modelsim 10.1d and 1 in iverilog (git c61d215).

@martinwhitaker
Copy link
Collaborator

@martinwhitaker martinwhitaker commented Mar 2, 2014

Yes, I need to apply the fix in the compiler to the simulation engine as well. I'm going to hold off on this for a little while, to give Steve a chance to get his vec4-stack branch finished and merged. I'll reopen this issue so I don't forget about it.

@martinwhitaker martinwhitaker reopened this Mar 2, 2014
@cliffordwolf
Copy link
Contributor Author

@cliffordwolf cliffordwolf commented May 4, 2014

I'll reopen this issue so I don't forget about it.

@martinwhitaker It's been two months now. Are there any news regarding this issue?

@martinwhitaker
Copy link
Collaborator

@martinwhitaker martinwhitaker commented May 4, 2014

You're right, it's time I pushed this fix. Done.

@cliffordwolf
Copy link
Contributor Author

@cliffordwolf cliffordwolf commented May 11, 2014

Hmmm..

module test(y);
  output y;
  wire a = 1;
  assign y = 1 >> {a, 64'b0};
  initial #1 $display("%b", y);
endmodule

Still returns 0 in modelsim 10.1d and 1 in iverilog (git be8df11).

@martinwhitaker
Copy link
Collaborator

@martinwhitaker martinwhitaker commented May 11, 2014

git be8df11 is the commit before the one where I fixed this.

@cliffordwolf
Copy link
Contributor Author

@cliffordwolf cliffordwolf commented May 11, 2014

sorry: pebkac. I pulled from the wrong remote branch.. ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants