Fix comparison of string parameters (#4308)

src/V3Ast.cpp

@@ -1082,10 +1082,10 @@ bool AstNode::sameTreeIter(const AstNode* node1p, const AstNode* node2p, bool ig
     if (!node1p && !node2p) return true;
     if (!node1p || !node2p) return false;
     if (node1p->type() != node2p->type()) return false;
-    if (node1p->dtypep() != node2p->dtypep()) return false;
     UASSERT_OBJ(
         (!node1p->dtypep() && !node2p->dtypep()) || (node1p->dtypep() && node2p->dtypep()), node1p,
         "Comparison of a node with dtypep() with a node without dtypep()\n-node2=" << node2p);
+    if (node1p->dtypep() && !node1p->dtypep()->similarDType(node2p->dtypep())) return false;
     if (!node1p->same(node2p) || (gateOnly && !node1p->isGateOptimizable())) return false;
     return (sameTreeIter(node1p->m_op1p, node2p->m_op1p, false, gateOnly)
             && sameTreeIter(node1p->m_op2p, node2p->m_op2p, false, gateOnly)

src/V3Param.cpp

@@ -394,6 +394,11 @@ class ParamProcessor final : public VNDeleter {
         }
         return nullptr;
     }
+    bool isString(AstNodeDType* nodep) {
+        if (AstBasicDType* const basicp = VN_CAST(nodep->skipRefToEnump(), BasicDType))
+            return basicp->isString();
+        return false;
+    }
     void collectPins(CloneMap* clonemapp, AstNodeModule* modp, bool originalIsCopy) {
         // Grab all I/O so we can remap our pins later
         for (AstNode* stmtp = modp->stmtsp(); stmtp; stmtp = stmtp->nextp()) {
@@ -670,6 +675,19 @@ class ParamProcessor final : public VNDeleter {
         return modInfop;
     }
 
+    void convertToStringp(AstNode* nodep) {
+        // Should be called on values of parameters of type string to convert them
+        // to properly typed string constants.
+        // Has no effect if the value is not a string constant.
+        AstConst* const constp = VN_CAST(nodep, Const);
+        // Check if it wasn't already converted
+        if (constp && !constp->num().isString()) {
+            constp->replaceWith(
+                new AstConst{constp->fileline(), AstConst::String{}, constp->num().toString()});
+            constp->deleteTree();
+        }
+    }
+
     void cellPinCleanup(AstNode* nodep, AstPin* pinp, AstNodeModule* srcModp, string& longnamer,
                         bool& any_overridesr) {
         if (!pinp->exprp()) return;  // No-connect
@@ -684,8 +702,16 @@ class ParamProcessor final : public VNDeleter {
                 any_overridesr = true;
             } else {
                 V3Const::constifyParamsEdit(pinp->exprp());
+                // String constants are parsed as logic arrays and converted to strings in V3Const.
+                // At this moment, some constants may have been already converted.
+                // To correctly compare constants, both should be of the same type,
+                // so they need to be converted.
+                if (isString(modvarp->subDTypep())) {
+                    convertToStringp(pinp->exprp());
+                    convertToStringp(modvarp->valuep());
+                }
                 AstConst* const exprp = VN_CAST(pinp->exprp(), Const);
-                const AstConst* const origp = VN_CAST(modvarp->valuep(), Const);
+                AstConst* const origp = VN_CAST(modvarp->valuep(), Const);
                 if (!exprp) {
                     if (debug()) pinp->dumpTree("-  ");
                     pinp->v3error("Can't convert defparam value to constant: Param "

test_regress/t/t_class_param.v

@@ -120,6 +120,16 @@ class Getter2 #(int T=5);
    endfunction
 endclass
 
+class ClsParamString #(string S="abcde");
+   typedef ClsParamString#(S) this_type;
+   static this_type m_inst;
+   int x = 0;
+   string name = S;
+endclass
+
+typedef ClsParamString#("abcde") cls_param_string_def_t;
+typedef ClsParamString#("xyz") cls_param_string_not_def_t;
+
 module t (/*AUTOARG*/);
 
    Cls c12;
@@ -137,6 +147,8 @@ module t (/*AUTOARG*/);
    Getter1 getter1;
    Getter1 #(1) getter1_param_1;
    Getter2 getter2;
+   cls_param_string_def_t cps_def;
+   cls_param_string_not_def_t cps_not_def;
    int arr [1:0] = '{1, 2};
    initial begin
       c12 = new;
@@ -217,6 +229,18 @@ module t (/*AUTOARG*/);
       if (Getter2#()::get_2() != 2) $stop;
       if (Getter2#(2)::get_2() != 2) $stop;
 
+      cls_param_string_def_t::m_inst = new;
+      cls_param_string_def_t::m_inst.x = 1;
+      cps_def = cls_param_string_def_t::m_inst;
+      if (cps_def.x != 1) $stop;
+      if (cps_def.name != "abcde") $stop;
+
+      cls_param_string_not_def_t::m_inst = new;
+      cls_param_string_not_def_t::m_inst.x = 2;
+      cps_not_def = cls_param_string_not_def_t::m_inst;
+      if (cps_not_def.x != 2) $stop;
+      if (cps_not_def.name != "xyz") $stop;
+
       $write("*-* All Finished *-*\n");
       $finish;
    end