YosysHQ / yosys

Yosys Open SYnthesis Suite
https://yosyshq.net/yosys/
ISC License
3.48k stars 890 forks source link

“converting real value” error message can be more helpful #487

Open AlexDaniel opened 6 years ago

AlexDaniel commented 6 years ago

In some cases yosys produces a warning like this:

Warning: converting real value 7.960000e+01 to binary 80 at foo.sv:19.

The warning is correct and I expected that it would do that without even warning me, although I don't mind. But what would be the most correct way to silence this warning? How can I tell it “yeah, just round the value to the closest integer and use that, I mean it”? And if there is an easy way to do that, can we change the warning to recommend that?

I got this warning when I was initializing block ram in the initial block where I had some integers which were then put into the block ram. I think that's a valid situation to do conversion like this.

cliffordwolf commented 6 years ago

Use the standard Verilog $rtoi() system function to convert reals to ints without warning. (Note: This converts by "truncating" the real part, not by rounding to the next integer.)

AlexDaniel commented 6 years ago

But the default behavior does the rounding to the next integer, how to get that behavior? Or am I misunderstanding something?

Also, the proposal was to change the warning message. Something like Warning: converting real value 7.960000e+01 to binary 80 at foo.sv:19 (use $rtoi to hide the warning).… and if that is rejected, why?

RossBencina commented 3 months ago

[Apologies for length of comment, I want to spell everything out. Proposed workaround at end.]

I have the same problem. If I understand correctly, the requirement is to be able to write elaboration-time constant code like the following that performs an intentional round-to-nearest operation without receiving a warning. For me, ideally the code should be portable (since I want to keep using icecube2 as a cross-check). This is the code that causes a warning:

local real a = 2;
local real b = 3;
local integer c = a/b; // intentional round-to-nearest, expect c == 1

The above works, but Yosys gives a warning like:

Warning: converting real value 6.666667e-01 to binary 1.

Indeed Synplify Pro warns similarly, but not always: it didn't warn for a=2, b=3, but did warn for a=25000000, b=115200:

Rounding real from 217.013889 to 217 (simulation mismatch possible)

As I understand it, the "modern" way to do this in SystemVerilog is with an integer cast:

local integer c = int'(a/b); // integer cast produces intentional round-to-nearest

In Yosys the above code gives:

ERROR: Static cast is only supported in SystemVerilog mode.

and Synplify Pro fails with a parsing error.

So the question remains: how to do an intentional round-to-nearest-integer operation at elaboration time in Verilog 2001 with Yosys, or ideally, portably, without emitting a warning?

The following appears to work in Yosys:

localparam integer c= $rtoi((a/b) + 0.5); // intentional round to nearest without warning

but fails miserably in Synplify Pro ("cannot synthesize expression"), so it is not a portable solution but maybe a workaround for some.

If there was a pre-defined __YOSYS__ symbol then we could do something like the following and maybe wrap it in a function:

`ifdef __YOSYS__
localparam integer c = $rtoi((a/b) + 0.5); // intentional round to nearest without warning
`else
localparam integer c = a/b; // intentional round to nearest
`endif

For now I guess I'll just pass -D__YOSYS__ on the command line.

whitequark commented 3 months ago

Have you tried int'(...) while using read_verilog -sv?

RossBencina commented 3 months ago

Have you tried int'(...) while using read_verilog -sv?

I have not, and with my current limited knowledge it seems not easy for me to try because I'm just passing all of the .v files to yosys on the command line.

In any case, you seem to be proposing to process the file as SystemVerilog, which completely misses the point of checking that the rtl is valid Verilog 2001 or 2005 so that it has some chance of remaining portable between tools.

whitequark commented 3 months ago

Ah yeah, sorry, I misread what you said.