From fb8bcdc0df60acd02b10aaa37aa0346c71bb1ad7 Mon Sep 17 00:00:00 2001 From: Marco Bartoli Date: Sun, 13 Sep 2026 19:34:10 +0200 Subject: [PATCH] Fix format of signed enums (#8338) --- include/verilated.cpp | 9 +++- include/verilatedos.h | 1 + src/V3Number.h | 3 +- src/V3Width.cpp | 21 +++++--- test_regress/t/t_display_pattern_format.v | 58 +++++++++++++++++++++++ 5 files changed, 82 insertions(+), 10 deletions(-) diff --git a/include/verilated.cpp b/include/verilated.cpp index 361801e27..09f3c4d03 100644 --- a/include/verilated.cpp +++ b/include/verilated.cpp @@ -1178,8 +1178,12 @@ void _vl_vsformat(std::string& output, const std::string& format, int argc, } else if (formatAttr == VL_VFORMATATTR_STRING) { thingp = va_arg(ap, std::string*); if (fmt != 'p' && fmt != 'x') fmt = 's'; // Override - } else if (formatAttr == VL_VFORMATATTR_ENUM) { + } else if (formatAttr == VL_VFORMATATTR_ENUM + || formatAttr == VL_VFORMATATTR_ENUM_SIGNED) { // Always <= VL_QUADSIZE; emit uses non-ENUM format for wider enums + const int numericAttr = formatAttr == VL_VFORMATATTR_ENUM_SIGNED + ? VL_VFORMATATTR_SIGNED + : VL_VFORMATATTR_UNSIGNED; lbits = va_arg(ap, int); ld = VL_VA_ARG_Q_(ap, lbits); strwide.resize(2); @@ -1192,6 +1196,7 @@ void _vl_vsformat(std::string& output, const std::string& format, int argc, enump = va_arg(ap, std::string*); if (enump && !enump->empty()) { formatAttr = (fmt == 'p') ? VL_VFORMATATTR_COMPLEX : VL_VFORMATATTR_STRING; + if (fmt == 'd') formatAttr = numericAttr; thingp = const_cast(enump); } else if (fmt == 'p' && widthSet && width == 0) { output += "'h"; @@ -1201,7 +1206,7 @@ void _vl_vsformat(std::string& output, const std::string& format, int argc, if (fmt == 'p') width = 0; widthSet = true; fmt = 'd'; - formatAttr = VL_VFORMATATTR_UNSIGNED; + formatAttr = numericAttr; } if (widthSet && width == 0) { while (lsb && !VL_BITISSET_W(lwp, lsb)) --lsb; diff --git a/include/verilatedos.h b/include/verilatedos.h index 21ea88e11..d720fdc32 100644 --- a/include/verilatedos.h +++ b/include/verilatedos.h @@ -462,6 +462,7 @@ using ssize_t = uint32_t; ///< signed size_t; returned from read() #define VL_VFORMATATTR_COMPLEX '!' // (std::string*); for non-POD; e.g. struct, requires %p typically #define VL_VFORMATATTR_DOUBLE 'D' // (double); promote %p to %f #define VL_VFORMATATTR_ENUM 'E' // (width, IData/QData, std::string* name); <= 64 bit enum with runtime %p/%s +#define VL_VFORMATATTR_ENUM_SIGNED 'F' // Same arguments as ENUM, with a signed numeric value #define VL_VFORMATATTR_SCOPE 'M' // (char* name, char* scope); for scopes #define VL_VFORMATATTR_STRING 'S' // (char* name, char* scope); for scopes // (std::string*); for %p/%s #define VL_VFORMATATTR_TIMEUNIT 'T' // (int timeunit); timeunits passed from V3Emit to runtime diff --git a/src/V3Number.h b/src/V3Number.h index d81e32d72..6568f27ff 100644 --- a/src/V3Number.h +++ b/src/V3Number.h @@ -47,6 +47,7 @@ public: COMPLEX = VL_VFORMATATTR_COMPLEX, DOUBLE = VL_VFORMATATTR_DOUBLE, ENUM = VL_VFORMATATTR_ENUM, + ENUM_SIGNED = VL_VFORMATATTR_ENUM_SIGNED, SCOPE = VL_VFORMATATTR_SCOPE, STRING = VL_VFORMATATTR_STRING, TIMEUNIT = VL_VFORMATATTR_TIMEUNIT @@ -63,7 +64,7 @@ public: char ascii() const { return m_e; } bool isComplex() const { return m_e == COMPLEX; } bool isDouble() const { return m_e == DOUBLE; } - bool isEnum() const { return m_e == ENUM; } + bool isEnum() const { return m_e == ENUM || m_e == ENUM_SIGNED; } bool isSigned() const { return m_e == SIGNED; } bool isString() const { return m_e == STRING; } bool isUnsigned() const { return m_e == UNSIGNED; } diff --git a/src/V3Width.cpp b/src/V3Width.cpp index b25435113..c5c03da1a 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -6768,7 +6768,9 @@ class WidthVisitor final : public VNVisitor { argp = newp; } else if (nodep->exprFormat()) { if (AstEnumDType* const enumDtp = formatEnumDType(argp)) { - nodep->addExprsp(new AstSFormatArg{argp->fileline(), VFormatAttr::ENUM, argp}); + const VFormatAttr attr + = enumDtp->isSigned() ? VFormatAttr::ENUM_SIGNED : VFormatAttr::ENUM; + nodep->addExprsp(new AstSFormatArg{argp->fileline(), attr, argp}); AstNodeExpr* const namep = enumSelect(argp->cloneTreePure(false), enumDtp, VAttrType::ENUM_NAME); nodep->addExprsp( @@ -8796,12 +8798,17 @@ class WidthVisitor final : public VNVisitor { } if (widthSet && width == 0) fallbackFormat = "'h%0h"; } - AstNodeExpr* const newp = new AstCond{ - subargp->fileline(), enumTestValid(subargp, enumDtp), - enumSelect(subargp->cloneTreePure(false), enumDtp, - VAttrType::ENUM_NAME), - new AstSFormatF{subargp->fileline(), fallbackFormat, true, - subargp->cloneTreePure(false)}}; + AstNodeExpr* fallbackp = subargp->cloneTreePure(false); + if (enumDtp->isSigned()) { + fallbackp = new AstSFormatArg{subargp->fileline(), + VFormatAttr::SIGNED, fallbackp}; + } + AstNodeExpr* const newp + = new AstCond{subargp->fileline(), enumTestValid(subargp, enumDtp), + enumSelect(subargp->cloneTreePure(false), enumDtp, + VAttrType::ENUM_NAME), + new AstSFormatF{subargp->fileline(), fallbackFormat, + true, fallbackp}}; subargp->replaceWith(new AstSFormatArg{subargp->fileline(), VFormatAttr::COMPLEX, newp}); VL_DO_DANGLING(pushDeletep(subargp), subargp); diff --git a/test_regress/t/t_display_pattern_format.v b/test_regress/t/t_display_pattern_format.v index c2f38d612..b93d7ed16 100644 --- a/test_regress/t/t_display_pattern_format.v +++ b/test_regress/t/t_display_pattern_format.v @@ -21,6 +21,19 @@ module t; SECOND = 7'd65 } enum_t; typedef enum_t enum_alias_t; + typedef enum logic signed [6:0] { + SIGNED7_NEG = -7'sd3, + SIGNED7_POS = 7'sd7 + } signed7_t; + typedef signed7_t signed7_alias_t; + typedef enum logic signed [32:0] { + SIGNED33_NEG = -33'sd3, + SIGNED33_POS = 33'sd7 + } signed33_t; + typedef enum logic signed [64:0] { + SIGNED65_NEG = -65'sd18446744073709551615, + SIGNED65_POS = 65'sd7 + } signed65_t; localparam text_t TEXT_PARAM = "quote=\" slash=\\ bell=\a form=\f vert=\v ctrl=\001"; localparam string ESCAPED_PARAM_STRING = $sformatf("%p", TEXT_PARAM); @@ -58,6 +71,10 @@ module t; string real_expected; string formatted; string enum_text; + signed7_alias_t signed7_value; + signed33_t signed33_value; + signed65_t signed65_value; + string signed_expected; plain = $sformatf("round %0d", cyc); escaped = {"quote=\" slash=\\ line=\n cr=\r tab=\t bell=\a form=\f vert=\v ctrl=\001 ", plain}; @@ -116,6 +133,47 @@ module t; formatted = $sformatf(fmt, real_value); `checks(formatted, real_expected); + signed7_value = cyc[0] ? SIGNED7_NEG : signed7_t'(-7'sd2); + signed33_value = cyc[0] ? SIGNED33_NEG : signed33_t'(-33'sd2); + signed_expected = cyc[0] ? "-3" : "-2"; + formatted = $sformatf("%0d", signed7_value); + `checks(formatted, signed_expected); + formatted = $sformatf("%0d", signed33_value); + `checks(formatted, signed_expected); + fmt = cyc[0] ? "%0d" : "%0D"; + formatted = $sformatf(fmt, signed7_value); + `checks(formatted, signed_expected); + formatted = $sformatf(fmt, signed33_value); + `checks(formatted, signed_expected); + + signed65_value = cyc[0] ? signed65_t'(-65'sd2) : SIGNED65_NEG; + signed_expected = cyc[0] ? "-2" : "-18446744073709551615"; + formatted = $sformatf("%0d", signed65_value); + `checks(formatted, signed_expected); + formatted = $sformatf(fmt, signed65_value); + `checks(formatted, signed_expected); + + signed_expected = cyc[0] ? "SIGNED7_NEG" : "-2"; +`ifdef QUESTA + // Questa 2025.2 zero-extends unnamed enums narrower than 32 bits for %p/%s. + if (!cyc[0]) signed_expected = "126"; +`endif + formatted = $sformatf("%p", signed7_value); + `checks(formatted, signed_expected); + formatted = $sformatf("%s", signed7_value); + `checks(formatted, signed_expected); + fmt = cyc[1] ? "%p" : "%s"; + formatted = $sformatf(fmt, signed7_value); + `checks(formatted, signed_expected); + + signed_expected = cyc[0] ? "SIGNED33_NEG" : "-2"; + formatted = $sformatf("%p", signed33_value); + `checks(formatted, signed_expected); + formatted = $sformatf("%s", signed33_value); + `checks(formatted, signed_expected); + formatted = $sformatf(fmt, signed33_value); + `checks(formatted, signed_expected); + cyc <= cyc + 1; if (cyc == 3) begin $write("*-* All Finished *-*\n");