From 18e317fb783f5da9df6e3ab33882b8cb358de970 Mon Sep 17 00:00:00 2001 From: Andrew Nolte Date: Wed, 1 Feb 2023 17:30:02 -0500 Subject: [PATCH] Fix inconsistent naming of generate scope arrays (#3840) --- src/V3Ast.cpp | 73 ++++++++++++++++++++++++++++++++- src/V3Ast.h | 1 + src/V3EmitCSyms.cpp | 26 +++++++++--- test_regress/t/t_vpi_module.cpp | 6 +-- test_regress/t/t_vpi_module.v | 6 +-- test_regress/t/t_vpi_var.cpp | 6 +++ test_regress/t/t_vpi_var.v | 10 ++++- 7 files changed, 115 insertions(+), 13 deletions(-) diff --git a/src/V3Ast.cpp b/src/V3Ast.cpp index b77b329e4..cf8b6b1bb 100644 --- a/src/V3Ast.cpp +++ b/src/V3Ast.cpp @@ -158,7 +158,6 @@ string AstNode::vcdName(const string& namein) { string AstNode::prettyName(const string& namein) { // This function is somewhat hot, so we short-circuit some compares string pretty; - pretty = ""; pretty.reserve(namein.length()); for (const char* pos = namein.c_str(); *pos;) { if (pos[0] == '-' && pos[1] == '>') { // -> @@ -206,6 +205,78 @@ string AstNode::prettyName(const string& namein) { return pretty; } +string AstNode::vpiName(const string& namein) { + // This is slightly different from prettyName, in that when we encounter escaped characters, + // we change that identifier to an escaped identifier, wrapping it with '\' and ' ' + // as specified in LRM 23.6 + string pretty; + pretty.reserve(namein.length()); + bool inEscapedIdent = false; + int lastIdent = 0; + + for (const char* pos = namein.c_str(); *pos;) { + char specialChar = 0; + if (pos[0] == '-' && pos[1] == '>') { // -> + specialChar = '.'; + pos += 2; + } else if (pos[0] == '_' && pos[1] == '_') { // __ + if (0 == std::strncmp(pos, "__BRA__", 7)) { + specialChar = '['; + pos += 7; + } else if (0 == std::strncmp(pos, "__KET__", 7)) { + specialChar = ']'; + pos += 7; + } else if (0 == std::strncmp(pos, "__DOT__", 7)) { + specialChar = '.'; + pos += 7; + } else if (0 == std::strncmp(pos, "__PVT__", 7)) { + pos += 7; + continue; + } else if (pos[0] == '_' && pos[1] == '_' && pos[2] == '0' && isxdigit(pos[3]) + && isxdigit(pos[4])) { + char value = 0; + value += 16 * (isdigit(pos[3]) ? (pos[3] - '0') : (tolower(pos[3]) - 'a' + 10)); + value += (isdigit(pos[4]) ? (pos[4] - '0') : (tolower(pos[4]) - 'a' + 10)); + + // __ doesn't always imply escaped ident + if (value != '_') inEscapedIdent = true; + + pretty += value; + pos += 5; + continue; + } + } else if (pos[0] == '.') { + specialChar = '.'; + ++pos; + } + + if (specialChar) { + if (inEscapedIdent && (specialChar == '[' || specialChar == '.')) { + pretty += " "; + pretty.insert(lastIdent, "\\"); + inEscapedIdent = false; + } + + pretty += specialChar; + + if (specialChar == ']' || specialChar == '.') { + lastIdent = pretty.length(); + inEscapedIdent = false; + } + } else { + pretty += pos[0]; + ++pos; + } + } + if (inEscapedIdent) { + pretty += " "; + pretty.insert(lastIdent, "\\"); + } + if (pretty[0] == 'T' && pretty.substr(0, 4) == "TOP.") pretty.replace(0, 4, ""); + if (pretty[0] == 'T' && pretty.substr(0, 5) == "TOP->") pretty.replace(0, 5, ""); + return pretty; +} + string AstNode::prettyTypeName() const { if (name() == "") return typeName(); return std::string{typeName()} + " '" + prettyName() + "'"; diff --git a/src/V3Ast.h b/src/V3Ast.h index f437f1ef9..5858c688d 100644 --- a/src/V3Ast.h +++ b/src/V3Ast.h @@ -1666,6 +1666,7 @@ public: string shortName() const; // Name with __PVT__ removed for concatenating scopes static string dedotName(const string& namein); // Name with dots removed static string prettyName(const string& namein); // Name for printing out to the user + static string vpiName(const string& namein); // Name for vpi access static string prettyNameQ(const string& namein) { // Quoted pretty name (for errors) return std::string{"'"} + prettyName(namein) + "'"; } diff --git a/src/V3EmitCSyms.cpp b/src/V3EmitCSyms.cpp index 8b3e75d84..c3d65bdf1 100644 --- a/src/V3EmitCSyms.cpp +++ b/src/V3EmitCSyms.cpp @@ -156,6 +156,22 @@ class EmitCSyms final : EmitCBaseVisitor { string out = scpname; // Remove hierarchy string::size_type pos = out.rfind('.'); + + // If there's more than one ident and an escape, find the true last ident + if (pos != string::npos && scpname.find('\\') != string::npos) { + size_t i = 0; + // always makes progress + while (i < scpname.length()) { + if (scpname[i] == '\\') { + while (i < scpname.length() && scpname[i] != ' ') ++i; + ++i; // Proc ' ', it should always be there. Then grab '.' on next cycle + } else { + while (i < scpname.length() && scpname[i] != '.') ++i; + if (i < scpname.length()) { pos = i++; } + } + } + } + if (pos != std::string::npos) out.erase(0, pos + 1); // Decode all escaped characters while ((pos = out.find("__0")) != string::npos) { @@ -296,10 +312,10 @@ class EmitCSyms final : EmitCBaseVisitor { const string type = (nodep->origModName() == "__BEGIN__") ? "SCOPE_OTHER" : "SCOPE_MODULE"; const string name = nodep->scopep()->shortName() + "__DOT__" + nodep->name(); - const string name_dedot = AstNode::dedotName(name); + const string name_pretty = AstNode::vpiName(name); const int timeunit = m_modp->timeunit().powerOfTen(); - m_vpiScopeCandidates.insert( - std::make_pair(name, ScopeData(scopeSymString(name), name_dedot, timeunit, type))); + m_vpiScopeCandidates.insert(std::make_pair( + name, ScopeData(scopeSymString(name), name_pretty, timeunit, type))); } } void visit(AstScope* nodep) override { @@ -310,10 +326,10 @@ class EmitCSyms final : EmitCBaseVisitor { if (v3Global.opt.vpi() && !nodep->isTop()) { const string type = VN_IS(nodep->modp(), Package) ? "SCOPE_OTHER" : "SCOPE_MODULE"; - const string name_dedot = AstNode::dedotName(nodep->shortName()); + const string name_pretty = AstNode::vpiName(nodep->shortName()); const int timeunit = m_modp->timeunit().powerOfTen(); m_vpiScopeCandidates.insert( - std::make_pair(nodep->name(), ScopeData(scopeSymString(nodep->name()), name_dedot, + std::make_pair(nodep->name(), ScopeData(scopeSymString(nodep->name()), name_pretty, timeunit, type))); } } diff --git a/test_regress/t/t_vpi_module.cpp b/test_regress/t/t_vpi_module.cpp index 650c7f55f..7a40a9374 100644 --- a/test_regress/t/t_vpi_module.cpp +++ b/test_regress/t/t_vpi_module.cpp @@ -120,7 +120,7 @@ int mon_check() { CHECK_RESULT_NZ(mod2); const char* mod_a_name = vpi_get_str(vpiName, mod2); - CHECK_RESULT_CSTR(mod_a_name, "mod_a"); + CHECK_RESULT_CSTR(mod_a_name, "\\mod.a "); TestVpiHandle it3 = vpi_iterate(vpiModule, mod2); CHECK_RESULT_NZ(it3); @@ -129,13 +129,13 @@ int mon_check() { CHECK_RESULT_NZ(mod3); const char* mod_c_name = vpi_get_str(vpiName, mod3); - if (std::strcmp(mod_c_name, "mod_b") == 0) { + if (std::strcmp(mod_c_name, "\\mod_b$ ") == 0) { // Full visibility in other simulators, skip mod_b TestVpiHandle mod4 = vpi_scan(it3); CHECK_RESULT_NZ(mod4); mod_c_name = vpi_get_str(vpiName, mod4); } - CHECK_RESULT_CSTR(mod_c_name, "mod_c."); + CHECK_RESULT_CSTR(mod_c_name, "\\mod\\c$ "); return 0; // Ok } diff --git a/test_regress/t/t_vpi_module.v b/test_regress/t/t_vpi_module.v index b997270f1..be1d4a46c 100644 --- a/test_regress/t/t_vpi_module.v +++ b/test_regress/t/t_vpi_module.v @@ -31,7 +31,7 @@ extern "C" int mon_check(); wire a, b, x; - A mod_a(/*AUTOINST*/ + A \mod.a (/*AUTOINST*/ // Outputs .x (x), // Inputs @@ -72,14 +72,14 @@ module A(/*AUTOARG*/ wire y, c; - B mod_b(/*AUTOINST*/ + B \mod_b$ (/*AUTOINST*/ // Outputs .y (y), // Inputs .b (b), .c (c)); - C \mod_c. (/*AUTOINST*/ + C \mod\c$ (/*AUTOINST*/ // Outputs .x (x), // Inputs diff --git a/test_regress/t/t_vpi_var.cpp b/test_regress/t/t_vpi_var.cpp index 4a0e1b0f7..e06fda16c 100644 --- a/test_regress/t/t_vpi_var.cpp +++ b/test_regress/t/t_vpi_var.cpp @@ -648,6 +648,12 @@ int _mon_check_putget_str(p_cb_data cb_data) { = vpi_handle_by_name((PLI_BYTE8*)"verbose", data[i].scope)); } + for (int i = 1; i <= 6; i++) { + char buf[32]; + snprintf(buf, sizeof(buf), TestSimulator::rooted("subs[%d].subsub"), i); + CHECK_RESULT_NZ(data[i].scope = vpi_handle_by_name((PLI_BYTE8*)buf, NULL)); + } + static t_cb_data cb_data; static s_vpi_value v; TestVpiHandle count_h = VPI_HANDLE("count"); diff --git a/test_regress/t/t_vpi_var.v b/test_regress/t/t_vpi_var.v index b01d6699b..79ee9e019 100644 --- a/test_regress/t/t_vpi_var.v +++ b/test_regress/t/t_vpi_var.v @@ -95,7 +95,15 @@ extern "C" int mon_check(); generate for (i=1; i<=6; i=i+1) begin : arr arr #(.LENGTH(i)) arr(); - end endgenerate + end + endgenerate + + genvar k; + generate + for (k=1; k<=6; k=k+1) begin : subs + sub subsub(); + end + endgenerate endmodule : t