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

Fixedpoint wrong generation for Verilog only #110

Closed
postoroniy opened this issue Jan 15, 2018 · 8 comments
Closed

Fixedpoint wrong generation for Verilog only #110

postoroniy opened this issue Jan 15, 2018 · 8 comments

Comments

@postoroniy
Copy link

postoroniy commented Jan 15, 2018

val a0  = in  Sfix( 0 exp,  -8 exp)
val b   = out Sfix(2 exp, -8 exp)
  b:= ((a0<<|1)+a0)<<|1

This can be correctly generated for vhdl only
in verilog you use operators '<<<' or '>>>' which are NOT adding any extra bits to LHS
In Vhdl functions pkg_shiftLeft are good way to use
but shifts >> or >>> or <<< or << are common mistake in verilog bit expansion

@Dolu1990
Copy link
Member

Hi,
I added a test about it : 6e5468a
And things pass the test for the VHDL and the Verilog
I checked the wave, and

  input  [8:0] io_inSFix2,

wire [9:0] _zz_6;
assign _zz_6 = (io_inSFix2 <<< 1);

is working as an logical shift left which add one bit on the left and fill with zero on the right. (as i wanted)

So i tested with Icarus Verilog, are you sure that "'<<<' which are NOT adding any extra bits to LHS" ?
I think between << >> and <<< >>> the only difference is that one is aritmetic (sign extension, while the other one is logical

http://www-inst.eecs.berkeley.edu/~cs150/fa06/Labs/verilog-ieee.pdf 4.1.12 Shift operators

Let's me know your thought :)

@postoroniy
Copy link
Author

My bad, just leaving comment below for history reference
All good, thanks.
ps. didn't touch verilog for ages. :)
4.5.1 Rules for expression types
The following are the rules for determining the resulting type of an expression:
Expression type depends only on the operands. It does not depend on the LHS (if any).
4.5.2 Steps for evaluating an expression
Determine the expression size based upon the standard rules of expression size determination.
Determine the sign of the expression using the rules outlined in 4.5.1.
Coerce the type of each operand of the expression (excepting those which are self-determined) to the
type of the expression.
Extend the size of each operand (excepting those which are self-determined) to the size of the
expression. Perform sign extension if and only if the operand type (after type coercion) is signed.
4.5.3 Steps for evaluating an assignment
Determine the size of the RHS by the standard assignment size determination rules (see 4.4)
If needed, extend the size of the RHS, performing sign extension if and only if the type of the RHS is
signed.

@Dolu1990
Copy link
Member

Hoo no worries :)
I personnaly learned Verilog the day i implemented the SpinalHDL backend for it XD

@postoroniy postoroniy reopened this Jan 18, 2018
@postoroniy
Copy link
Author

postoroniy commented Jan 18, 2018

ok...opening again because vcs2017 is not happy at all but vivado seems ok
the real issue is here

module bla(
      input  [7:0] d2_1,
...
);
...
  wire [8:0] d2_2;
  assign d2_2 = (d2_1 <<< 1);

The last assign is not correct(vcs2017 at least)
if I made

    assign d2_2 = ($signed(d2_1) <<< 1);

or declare d2_1 as signed then all is ok.
I guess due to this
If needed, extend the size of the RHS, performing sign extension if and only if the type of the RHS is
signed.

@Dolu1990
Copy link
Member

What happend in VCS in the assign d2_2 = (d2_1 <<< 1); case ?
the d2_2 msb is always zero ?

@postoroniy
Copy link
Author

postoroniy commented Jan 18, 2018

Correct and it makes sense,
just repeating myself :)
If needed, extend the size of the RHS, performing sign extension if and only if the type of the RHS is
signed.

since RHS is not signed then no extended size is required and sign bit in LHS is zero always.

@Dolu1990
Copy link
Member

Should be fixed by 4f7a1dd

@Dolu1990
Copy link
Member

Fix released in 1.1.3

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

No branches or pull requests

2 participants