Fix REALCVT warning on integral timescale conversions (#5378).

Changes

@@ -14,6 +14,7 @@ Verilator 5.029 devel
 **Minor:**
 
 * Improve Verilation thread pool (#5161). [Bartłomiej Chmiel, Antmicro Ltd.]
+* Fix REALCVT warning on integral timescale conversions (#5378). [Liam Braun]
 * Fix dot fallback finding wrong symbols (#5394). [Arkadiusz Kozdra, Antmicro Ltd.]
 
 

src/V3Number.cpp

@@ -388,6 +388,17 @@ void V3Number::nodep(AstNode* nodep) VL_MT_STABLE {
 //======================================================================
 // Global
 
+bool V3Number::epsilonEqual(double a, double b) {
+    return std::fabs(a - b)
+           <= (std::numeric_limits<double>::epsilon() * std::max(1.0, std::max(a, b)));
+}
+
+bool V3Number::epsilonIntegral(double a) {
+    const double dint = static_cast<double>(static_cast<int64_t>(a));
+    // Check all the rounding possibilities, as we did truncation above rather than rounding
+    return epsilonEqual(a, dint) || epsilonEqual(a, dint - 1) || epsilonEqual(a, dint + 1);
+}
+
 int V3Number::log2b(uint32_t num) {
     // See also opCLog2
     for (int bit = 31; bit > 0; bit--) {
@@ -2551,6 +2562,23 @@ void V3Number::selfTest() {
 
 void V3Number::selfTestThis() {
     // The self test has a "this" so UASSERT_SELFTEST/errorEndFatal works correctly
+
+    UASSERT_SELFTEST(bool, V3Number::epsilonEqual(0, 0), true);
+    UASSERT_SELFTEST(bool, V3Number::epsilonEqual(1e19, 1e19), true);
+    UASSERT_SELFTEST(bool, V3Number::epsilonEqual(9, 0.0001), false);
+    UASSERT_SELFTEST(bool, V3Number::epsilonEqual(1, 1 + std::numeric_limits<double>::epsilon()),
+                     true);
+    UASSERT_SELFTEST(bool, V3Number::epsilonEqual(0.009, 0.00899999999999999931998839741709),
+                     true);
+
+    UASSERT_SELFTEST(bool, V3Number::epsilonIntegral(0), true);
+    UASSERT_SELFTEST(bool, V3Number::epsilonIntegral(1), true);
+    UASSERT_SELFTEST(bool, V3Number::epsilonIntegral(-1), true);
+    UASSERT_SELFTEST(bool, V3Number::epsilonIntegral(1.0001), false);
+    UASSERT_SELFTEST(bool, V3Number::epsilonIntegral(0.9999), false);
+    UASSERT_SELFTEST(bool, V3Number::epsilonIntegral(-1.0001), false);
+    UASSERT_SELFTEST(bool, V3Number::epsilonIntegral(-0.9999), false);
+
     UASSERT_SELFTEST(int, log2b(0), 0);
     UASSERT_SELFTEST(int, log2b(1), 0);
     UASSERT_SELFTEST(int, log2b(0x40000000UL), 30);

src/V3Number.h

@@ -32,14 +32,6 @@
 
 //============================================================================
 
-// Return if two numbers within Epsilon of each other
-inline bool v3EpsilonEqual(double a, double b) {
-    return std::fabs(a - b)
-           <= (std::numeric_limits<double>::epsilon() * std::max(1.0, std::max(a, b)));
-}
-
-//============================================================================
-
 class AstNode;
 class AstNodeDType;
 class FileLine;
@@ -662,6 +654,8 @@ public:
     bool operator<(const V3Number& rhs) const { return isLtXZ(rhs); }
 
     // STATICS
+    static bool epsilonEqual(double a, double b);  // True if number within Epsilon of the other
+    static bool epsilonIntegral(double a);  // True if number rounds to integer within Epsilon
     static int log2b(uint32_t num);
     static int log2bQuad(uint64_t num);
     static void selfTest();

src/V3Param.cpp

@@ -200,7 +200,7 @@ public:
                 varNum.opIToRD(pinValuep->num());
                 var = varNum.toDouble();
             }
-            return v3EpsilonEqual(var, hierOptParamp->num().toDouble());
+            return V3Number::epsilonEqual(var, hierOptParamp->num().toDouble());
         } else {  // Now integer type is assumed
             // Bitwidth of hierOptParamp is accurate because V3Width already calculated in the
             // previous run. Bitwidth of pinValuep is before width analysis, so pinValuep is casted

src/V3Width.cpp

@@ -1506,9 +1506,9 @@ class WidthVisitor final : public VNVisitor {
         const AstConst* const constp = VN_CAST(nodep->lhsp(), Const);
         UASSERT_OBJ(constp && constp->isDouble(), nodep, "Times should be doubles");
         UASSERT_OBJ(!nodep->timeunit().isNone(), nodep, "$time import no units");
-        double time = constp->num().toDouble();
+        const double timePrescale = constp->num().toDouble();
         UASSERT_OBJ(!v3Global.rootp()->timeprecision().isNone(), nodep, "Never set precision?");
-        time /= nodep->timeunit().multiplier();
+        const double time = timePrescale / nodep->timeunit().multiplier();
         // IEEE claims you should round to time precision here, but no simulator seems to do this
         AstConst* const newp = new AstConst{nodep->fileline(), AstConst::RealDouble{}, time};
         nodep->replaceWith(newp);
@@ -7305,10 +7305,7 @@ class WidthVisitor final : public VNVisitor {
             if (const AstConst* const constp = VN_CAST(nodep, Const)) {
                 // We convert to/from int32_t rather than use floor() as want to make sure is
                 // representable in integer's number of bits
-                if (constp->isDouble()
-                    && v3EpsilonEqual(
-                        constp->num().toDouble(),
-                        static_cast<double>(static_cast<int32_t>(constp->num().toDouble())))) {
+                if (constp->isDouble() && V3Number::epsilonIntegral(constp->num().toDouble())) {
                     warnOn = false;
                 }
             }

test_regress/t/t_lint_realcvt_bad.out

@@ -1,6 +1,15 @@
-%Warning-REALCVT: t/t_lint_realcvt_bad.v:10:11: Implicit conversion of real to integer
-   10 |       i = 23.2;
-      |           ^~~~
+%Warning-REALCVT: t/t_lint_realcvt_bad.v:12:18: Implicit conversion of real to integer
+   12 |    time t_bad1 = 9.001ns;   
+      |                  ^~~~~~~
                   ... For warning description see https://verilator.org/warn/REALCVT?v=latest
                   ... Use "/* verilator lint_off REALCVT */" and lint_on around source to disable this message.
+%Warning-REALCVT: t/t_lint_realcvt_bad.v:13:18: Implicit conversion of real to integer
+   13 |    time t_bad2 = 9.999ns;   
+      |                  ^~~~~~~
+%Warning-REALCVT: t/t_lint_realcvt_bad.v:17:18: Implicit conversion of real to integer
+   17 |    time t_bad3 = 9ps;   
+      |                  ^~~
+%Warning-REALCVT: t/t_lint_realcvt_bad.v:23:22: Implicit conversion of real to integer
+   23 |    integer i_bad21 = 23.1;
+      |                      ^~~~
 %Error: Exiting due to

test_regress/t/t_lint_realcvt_bad.v

@@ -4,10 +4,22 @@
 // any use, without warranty, 2011 by Wilson Snyder.
 // SPDX-License-Identifier: CC0-1.0
 
+`timescale 1ns/1ps
+
 module sub;
-   integer i;
-   initial begin
-      i = 23.2;
-      i = 23.0; // No warning - often happens with units of time
-   end
+
+   time t_ok1 = 9ns;  // > 1ns units
+   time t_bad1 = 9.001ns;  // < 1ns units
+   time t_bad2 = 9.999ns;  // < 1ns units
+
+   time t_ok2 = 9.001us;  // > 1ns units
+
+   time t_bad3 = 9ps;  // < 1ns units
+
+   realtime rt_ok10 = 9.001ns;  // < 1ns units
+   realtime rt_ok11 = 9ps;  // < 1ns units
+
+   integer i_ok20 = 23.0; // No warning
+   integer i_bad21 = 23.1;
+
 endmodule