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

Undef propagation in power operator #7

Closed
cliffordwolf opened this issue Jan 6, 2014 · 2 comments
Closed

Undef propagation in power operator #7

cliffordwolf opened this issue Jan 6, 2014 · 2 comments

Comments

@cliffordwolf
Copy link
Contributor

@cliffordwolf cliffordwolf commented Jan 6, 2014

icarus verilog (git d1c9dd5) does not correctly propagate undef thru power operator. For example, y should be 4'bx when a is zero in the following test case, but iverilog returns 4'd1:

module issue_012(a, y);
  input [3:0] a;
  output [3:0] y;
  assign y = 4'd2 ** (4'd1/a);
endmodule

PS: In case anyone is wondering: I find this bugs using VlogHammer (http://www.clifford.at/yosys/vloghammer.html).

@caryr
Copy link
Collaborator

@caryr caryr commented Jan 6, 2014

There are a couple things here. At the moment the vec2 constructor that takes a vec4 is documented to return is_NaN when given a vector with undefined values. The issue is that the routine that copies the bits does not actually do this. This may be what we want and the constructor documentation needs to be changed and then in the vvp_arith_pow::recv_vec4() code the checks for undefined bits can use common code like what is done for a signed power (e.g. move the signed undefined check code outside of the if statement). Someone needs to spend some time checking which is the correct fix (change the constructor or change the power routine).

@martinwhitaker
Copy link
Collaborator

@martinwhitaker martinwhitaker commented Feb 15, 2014

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

Note to the Icarus developers: I've changed the vvp_vector2_t constructor that takes a vvp_vector4_t value to take an optional second parameter that selects whether NaN or standard Verilog (XZ -> 0) semantics are used, as both behaviours are needed. I've made the default be standard Verilog semantics, otherwise there is a nasty gotcha where

vvp_vector2_t bit2 = bit4;

and

vvp_vector2_t bit2;
bit2 = bit4;

give different behaviour.

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
3 participants