From f0cd6dd95cfe981fb8fc80ac62986e9d986b0775 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Sat, 24 Aug 2024 08:01:28 -0400 Subject: [PATCH] Fix REALCVT warning on integral timescale conversions (#5378). --- Changes | 1 + src/V3Number.cpp | 28 +++++++++++++++++++++++++++ src/V3Number.h | 10 ++-------- src/V3Param.cpp | 2 +- src/V3Width.cpp | 9 +++------ test_regress/t/t_lint_realcvt_bad.out | 15 +++++++++++--- test_regress/t/t_lint_realcvt_bad.v | 22 ++++++++++++++++----- 7 files changed, 64 insertions(+), 23 deletions(-) diff --git a/Changes b/Changes index bf36eb405..6c33ffd97 100644 --- a/Changes +++ b/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.] diff --git a/src/V3Number.cpp b/src/V3Number.cpp index f57c583cc..ff9ce7424 100644 --- a/src/V3Number.cpp +++ b/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::epsilon() * std::max(1.0, std::max(a, b))); +} + +bool V3Number::epsilonIntegral(double a) { + const double dint = static_cast(static_cast(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::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); diff --git a/src/V3Number.h b/src/V3Number.h index 3b18041aa..df03a78b6 100644 --- a/src/V3Number.h +++ b/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::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(); diff --git a/src/V3Param.cpp b/src/V3Param.cpp index e5b3d1b6b..d5c5d87d9 100644 --- a/src/V3Param.cpp +++ b/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 diff --git a/src/V3Width.cpp b/src/V3Width.cpp index 154be410c..835c7638b 100644 --- a/src/V3Width.cpp +++ b/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(static_cast(constp->num().toDouble())))) { + if (constp->isDouble() && V3Number::epsilonIntegral(constp->num().toDouble())) { warnOn = false; } } diff --git a/test_regress/t/t_lint_realcvt_bad.out b/test_regress/t/t_lint_realcvt_bad.out index 41625798c..b68e7e362 100644 --- a/test_regress/t/t_lint_realcvt_bad.out +++ b/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 diff --git a/test_regress/t/t_lint_realcvt_bad.v b/test_regress/t/t_lint_realcvt_bad.v index 3ebd6ed7c..daefdda26 100644 --- a/test_regress/t/t_lint_realcvt_bad.v +++ b/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