verilog: Bad handling of signedness of real constants in expressions.

[mwkmwkmwk]

module top(...);

input [15:0] A;
output O;

assign O = A < 2.0;

endmodule

Parsing that fails with:

Parsing Verilog input from `r.v' to AST representation.
Generating RTLIL representation for module `\top'.
r.v:6: Warning: converting real value 2.000000e+00 to binary 2.
Successfully finished Verilog frontend.
ERROR: Found error in internal cell \top.$lt$r.v:6$1 ($lt) at kernel/rtlil.cc:997:
  attribute \src "r.v:6.12-6.19"
  cell $lt $lt$r.v:6$1
    parameter \Y_WIDTH 1
    parameter \B_WIDTH 32
    parameter \A_WIDTH 16
    parameter \B_SIGNED 1
    parameter \A_SIGNED 0
    connect \Y $lt$r.v:6$1_Y
    connect \B 2
    connect \A \A
  end

[buttercutter]

As quoted by @mwkmwkmwk , either adding [31:0] to localparam to infer integer type OR wire [31:0] tmp = 2.0; assign O = A < tmp; will work to avoid floating point operations which are not synthesizable, because they don't map to logic circuits without major effort. However, the first method (adding the type directly on localparam is more superior)

[zachjs]

This bug is a little tricky. detectSignWidthWorker() has a hack where it sets sign_hint = true when found_real is set. This appears intended to treat -1 * -1.17 correctly in an unsigned context (i.e., when compared with an unsigned expression). Without the hack, it simplifies to INT_MAX * -1.17. This is the only place where sign_hint is set true in this way, breaking the (quite reasonable) assumptions made in the getRTLIL code for comparison operators. I think we should remove the existing hack, but I don't have a great proposal for making something like -1 * -1.17 work correctly without it.

[mwkmwkmwk]

Isn't the frontend too liberal in this case anyway? AFAICS the rules here would require promoting A to a real and performing a real comparison, which is non-synthesizable, and should result in an error.