From 7f1011e5f7d194e62e25ceae090d935baf223d0c Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Wed, 16 Jul 2025 18:07:07 -0400 Subject: [PATCH] Make some CVTREAL fatal where IEEE requires it. --- src/V3Width.cpp | 47 ++++++++++++++--------- test_regress/t/t_lint_realcvt_bad.out | 55 +++++++++++++++++++++------ test_regress/t/t_lint_realcvt_bad.v | 28 +++++++++----- 3 files changed, 89 insertions(+), 41 deletions(-) diff --git a/src/V3Width.cpp b/src/V3Width.cpp index 1571a7081..7153c441a 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -629,8 +629,8 @@ class WidthVisitor final : public VNVisitor { iterateCheckString(nodep, "RHS", nodep->rhsp(), BOTH); nodep->dtypeSetString(); } else { - iterateCheckSelf(nodep, "LHS", nodep->lhsp(), SELF, BOTH); - iterateCheckSelf(nodep, "RHS", nodep->rhsp(), SELF, BOTH); + iterateCheckIntegralSelf(nodep, "LHS", nodep->lhsp(), SELF, BOTH); + iterateCheckIntegralSelf(nodep, "RHS", nodep->rhsp(), SELF, BOTH); if (m_streamConcat) { packIfUnpacked(nodep->lhsp()); @@ -837,7 +837,7 @@ class WidthVisitor final : public VNVisitor { if (vdtypep && vdtypep->isString()) { iterateCheckString(nodep, "LHS", nodep->srcp(), BOTH); } else { - iterateCheckSelf(nodep, "LHS", nodep->srcp(), SELF, BOTH); + iterateCheckIntegralSelf(nodep, "LHS", nodep->srcp(), SELF, BOTH); } if ((vdtypep && vdtypep->isString()) || nodep->srcp()->isString()) { @@ -971,7 +971,7 @@ class WidthVisitor final : public VNVisitor { if (debug() >= 9) nodep->dumpTree("- selWidth: "); userIterateAndNext(nodep->fromp(), WidthVP{CONTEXT_DET, PRELIM}.p()); userIterateAndNext(nodep->lsbp(), WidthVP{SELF, PRELIM}.p()); - checkCvtUS(nodep->fromp()); + checkCvtUS(nodep->fromp(), false); iterateCheckSizedSelf(nodep, "Select LHS", nodep->fromp(), SELF, BOTH); int width = nodep->widthConst(); if (width <= 0) { @@ -1510,7 +1510,7 @@ class WidthVisitor final : public VNVisitor { return; } - checkCvtUS(nodep->lhsp()); + checkCvtUS(nodep->lhsp(), false); iterateCheckSizedSelf(nodep, "RHS", nodep->rhsp(), SELF, BOTH); nodep->dtypeFrom(nodep->lhsp()); } @@ -6826,7 +6826,7 @@ class WidthVisitor final : public VNVisitor { UASSERT_OBJ(!nodep->op2p(), nodep, "For unary ops only!"); if (m_vup->prelim()) { userIterateAndNext(nodep->lhsp(), WidthVP{CONTEXT_DET, PRELIM}.p()); - if (!real_ok) checkCvtUS(nodep->lhsp()); + if (!real_ok) checkCvtUS(nodep->lhsp(), false); } if (real_ok && nodep->lhsp()->isDouble()) { spliceCvtD(nodep->lhsp()); @@ -6867,7 +6867,7 @@ class WidthVisitor final : public VNVisitor { UASSERT_OBJ(!nodep->op2p(), nodep, "For unary ops only!"); if (m_vup->prelim()) { userIterateAndNext(nodep->lhsp(), WidthVP{SELF, PRELIM}.p()); - checkCvtUS(nodep->lhsp()); + checkCvtUS(nodep->lhsp(), true); const int width = nodep->lhsp()->width(); AstNodeDType* const expDTypep = nodep->findLogicDType(width, width, rs_out); nodep->dtypep(expDTypep); @@ -6895,7 +6895,7 @@ class WidthVisitor final : public VNVisitor { // RHS is self-determined. RHS is always treated as unsigned, has no effect on result. if (m_vup->prelim()) { userIterateAndNext(nodep->lhsp(), WidthVP{SELF, PRELIM}.p()); - checkCvtUS(nodep->lhsp()); + checkCvtUS(nodep->lhsp(), false); iterateCheckSizedSelf(nodep, "RHS", nodep->rhsp(), SELF, BOTH); nodep->dtypeFrom(nodep->lhsp()); } @@ -6955,8 +6955,8 @@ class WidthVisitor final : public VNVisitor { // Determine expression widths only relying on what's in the subops userIterateAndNext(nodep->lhsp(), WidthVP{CONTEXT_DET, PRELIM}.p()); userIterateAndNext(nodep->rhsp(), WidthVP{CONTEXT_DET, PRELIM}.p()); - checkCvtUS(nodep->lhsp()); - checkCvtUS(nodep->rhsp()); + checkCvtUS(nodep->lhsp(), false); + checkCvtUS(nodep->rhsp(), false); const int width = std::max(nodep->lhsp()->width(), nodep->rhsp()->width()); const int mwidth = std::max(nodep->lhsp()->widthMin(), nodep->rhsp()->widthMin()); const bool expSigned = (nodep->lhsp()->isSigned() && nodep->rhsp()->isSigned()); @@ -6990,8 +6990,8 @@ class WidthVisitor final : public VNVisitor { userIterateAndNext(nodep->lhsp(), WidthVP{CONTEXT_DET, PRELIM}.p()); userIterateAndNext(nodep->rhsp(), WidthVP{CONTEXT_DET, PRELIM}.p()); if (!real_ok) { - checkCvtUS(nodep->lhsp()); - checkCvtUS(nodep->rhsp()); + checkCvtUS(nodep->lhsp(), false); + checkCvtUS(nodep->rhsp(), false); } if (nodep->lhsp()->isDouble() || nodep->rhsp()->isDouble()) { spliceCvtD(nodep->lhsp()); @@ -7330,8 +7330,12 @@ class WidthVisitor final : public VNVisitor { } (void)underp; // cppcheck } + void iterateCheckIntegralSelf(AstNode* nodep, const char* side, AstNode* underp, Determ determ, + Stage stage) { + iterateCheckSelf(nodep, side, underp, determ, stage, true); + } void iterateCheckSelf(AstNode* nodep, const char* side, AstNode* underp, Determ determ, - Stage stage) { + Stage stage, bool integralOnly = false) { // Coerce child to any data type; child is self-determined // i.e. isolated from expected type. // e.g. nodep=CONCAT, underp=lhs in CONCAT(lhs,rhs) @@ -7341,7 +7345,8 @@ class WidthVisitor final : public VNVisitor { if (stage & PRELIM) { underp = userIterateSubtreeReturnEdits(underp, WidthVP{SELF, PRELIM}.p()); } - underp = VN_IS(underp, NodeExpr) ? checkCvtUS(VN_AS(underp, NodeExpr)) : underp; + underp + = VN_IS(underp, NodeExpr) ? checkCvtUS(VN_AS(underp, NodeExpr), integralOnly) : underp; AstNodeDType* const expDTypep = underp->dtypep(); underp = iterateCheck(nodep, side, underp, SELF, FINAL, expDTypep, EXTEND_EXP); (void)underp; // cppcheck @@ -7357,7 +7362,7 @@ class WidthVisitor final : public VNVisitor { if (stage & PRELIM) { underp = userIterateSubtreeReturnEdits(underp, WidthVP{SELF, PRELIM}.p()); } - underp = VN_IS(underp, NodeExpr) ? checkCvtUS(VN_AS(underp, NodeExpr)) : underp; + underp = VN_IS(underp, NodeExpr) ? checkCvtUS(VN_AS(underp, NodeExpr), false) : underp; AstNodeDType* const expDTypep = underp->dtypep(); underp = iterateCheck(nodep, side, underp, SELF, FINAL, expDTypep, EXTEND_EXP); AstNodeDType* const checkDtp = expDTypep->skipRefToEnump(); @@ -7674,11 +7679,15 @@ class WidthVisitor final : public VNVisitor { //---------------------------------------------------------------------- // SIGNED/DOUBLE METHODS - AstNodeExpr* checkCvtUS(AstNodeExpr* nodep) { + AstNodeExpr* checkCvtUS(AstNodeExpr* nodep, bool fatal) { if (nodep && nodep->dtypep()->skipRefp()->isDouble()) { - nodep->v3warn(REALCVT, - "Implicit conversion of real to integer; expected integral input to " - << nodep->backp()->prettyTypeName()); + if (fatal) { + nodep->v3error("Expected integral input to " << nodep->backp()->prettyTypeName()); + } else { + nodep->v3warn(REALCVT, + "Implicit conversion of real to integer; expected integral input to " + << nodep->backp()->prettyTypeName()); + } nodep = spliceCvtS(nodep, false, 32); } return nodep; diff --git a/test_regress/t/t_lint_realcvt_bad.out b/test_regress/t/t_lint_realcvt_bad.out index b68e7e362..d4cc2c9a6 100644 --- a/test_regress/t/t_lint_realcvt_bad.out +++ b/test_regress/t/t_lint_realcvt_bad.out @@ -1,15 +1,46 @@ -%Warning-REALCVT: t/t_lint_realcvt_bad.v:12:18: Implicit conversion of real to integer - 12 | time t_bad1 = 9.001ns; - | ^~~~~~~ +%Warning-REALCVT: t/t_lint_realcvt_bad.v:12:17: 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; - | ^~~~ +%Warning-REALCVT: t/t_lint_realcvt_bad.v:13:17: Implicit conversion of real to integer + 13 | time t_bad2 = 9.999ns; + | ^~~~~~~ +%Warning-REALCVT: t/t_lint_realcvt_bad.v:17:17: Implicit conversion of real to integer + 17 | time t_bad3 = 9ps; + | ^~~ +%Warning-REALCVT: t/t_lint_realcvt_bad.v:23:21: Implicit conversion of real to integer + 23 | integer i_bad21 = 23.1; + | ^~~~ +%Error: t/t_lint_realcvt_bad.v:27:17: Expected integral input to SIGNED + : ... note: In instance 'sub' + 27 | i = $signed(1.0); + | ^~~ + ... See the manual at https://verilator.org/verilator_doc.html?v=latest for more assistance. +%Error: t/t_lint_realcvt_bad.v:28:19: Expected integral input to UNSIGNED + : ... note: In instance 'sub' + 28 | i = $unsigned(1.0); + | ^~~ +%Error: t/t_lint_realcvt_bad.v:29:10: Expected integral input to CONCAT + : ... note: In instance 'sub' + 29 | i = {1.2, 1.3}; + | ^~~ +%Error: t/t_lint_realcvt_bad.v:29:15: Expected integral input to CONCAT + : ... note: In instance 'sub' + 29 | i = {1.2, 1.3}; + | ^~~ +%Warning-WIDTHTRUNC: t/t_lint_realcvt_bad.v:29:7: Operator ASSIGN expects 32 bits on the Assign RHS, but Assign RHS's REPLICATE generates 64 bits. + : ... note: In instance 'sub' + 29 | i = {1.2, 1.3}; + | ^ + ... For warning description see https://verilator.org/warn/WIDTHTRUNC?v=latest + ... Use "/* verilator lint_off WIDTHTRUNC */" and lint_on around source to disable this message. +%Error: t/t_lint_realcvt_bad.v:30:12: Expected integral input to REPLICATE + : ... note: In instance 'sub' + 30 | i = {6{1.2}}; + | ^~~ +%Warning-WIDTHTRUNC: t/t_lint_realcvt_bad.v:30:7: Operator ASSIGN expects 32 bits on the Assign RHS, but Assign RHS's REPLICATE generates 192 bits. + : ... note: In instance 'sub' + 30 | i = {6{1.2}}; + | ^ %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 daefdda26..819f218ab 100644 --- a/test_regress/t/t_lint_realcvt_bad.v +++ b/test_regress/t/t_lint_realcvt_bad.v @@ -4,22 +4,30 @@ // any use, without warranty, 2011 by Wilson Snyder. // SPDX-License-Identifier: CC0-1.0 -`timescale 1ns/1ps +`timescale 1ns / 1ps module sub; - time t_ok1 = 9ns; // > 1ns units - time t_bad1 = 9.001ns; // < 1ns units - time t_bad2 = 9.999ns; // < 1ns units + 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_ok2 = 9.001us; // > 1ns units - time t_bad3 = 9ps; // < 1ns units + time t_bad3 = 9ps; // < 1ns units - realtime rt_ok10 = 9.001ns; // < 1ns units - realtime rt_ok11 = 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; + integer i_ok20 = 23.0; // No warning + integer i_bad21 = 23.1; + + initial begin + int i; + i = $signed(1.0); // Error, doesn't support real, not just warning + i = $unsigned(1.0); // Error, doesn't support real, not just warning + i = {1.2, 1.3}; // Error, doesn't support real, not just warning + i = {6{1.2}}; // Error, doesn't support real, not just warning + end endmodule