Fix V3Table trying to generate 'x' bits in the lookup table.

Due to out of range selects, V3Table attempted to create a table in the constant pool with an 'x' value in it, which caused an internal error. Ensure V3Table behaves the same for out of range selects as the original logic would. There is a related bug verilator#5490, about leaving partially out of range selects in the logic after inserting bounds checks in V3Unknown.
gezalore · Sep 26, 2024 · 4a789af · 4a789af
1 parent 3bc09d4
commit 4a789af
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 2 deletions.
diff --git a/src/V3Simulate.h b/src/V3Simulate.h
@@ -258,6 +258,7 @@ class SimulateVisitor VL_NOT_FINAL : public VNVisitorConst {
     void newValue(AstNode* nodep, const AstNodeExpr* valuep) {
         if (const AstConst* const constp = VN_CAST(valuep, Const)) {
             newConst(nodep)->num().opAssign(constp->num());
+            UINFO(9, "     new val " << valuep->name() << " on " << nodep << endl);
         } else if (fetchValueNull(nodep) != valuep) {
             // const_cast, as clonep() is set on valuep, but nothing should care
             setValue(nodep, newTrackedClone(const_cast<AstNodeExpr*>(valuep)));
@@ -266,6 +267,7 @@ class SimulateVisitor VL_NOT_FINAL : public VNVisitorConst {
     void newOutValue(AstNode* nodep, const AstNodeExpr* valuep) {
         if (const AstConst* const constp = VN_CAST(valuep, Const)) {
             newOutConst(nodep)->num().opAssign(constp->num());
+            UINFO(9, "     new oval " << valuep->name() << " on " << nodep << endl);
         } else if (fetchOutValueNull(nodep) != valuep) {
             // const_cast, as clonep() is set on valuep, but nothing should care
             setOutValue(nodep, newTrackedClone(const_cast<AstNodeExpr*>(valuep)));
@@ -282,7 +284,7 @@ class SimulateVisitor VL_NOT_FINAL : public VNVisitorConst {
         // Set a constant value for this node
         if (!VN_IS(m_varAux(nodep).valuep, Const)) {
             AstConst* const constp = allocConst(nodep);
-            setValue(nodep, constp);
+            m_varAux(nodep).valuep = constp;
             return constp;
         } else {
             return fetchConst(nodep);
@@ -292,7 +294,7 @@ class SimulateVisitor VL_NOT_FINAL : public VNVisitorConst {
         // Set a var-output constant value for this node
         if (!VN_IS(m_varAux(nodep).outValuep, Const)) {
             AstConst* const constp = allocConst(nodep);
-            setOutValue(nodep, constp);
+            m_varAux(nodep).outValuep = constp;
             return constp;
         } else {
             return fetchOutConst(nodep);
@@ -594,9 +596,18 @@ class SimulateVisitor VL_NOT_FINAL : public VNVisitorConst {
         checkNodeInfo(nodep);
         iterateChildrenConst(nodep);
         if (!m_checkOnly && optimizable()) {
+            AstConst* const valuep = newConst(nodep);
             nodep->numberOperate(newConst(nodep)->num(), fetchConst(nodep->lhsp())->num(),
                                  fetchConst(nodep->rhsp())->num(),
                                  fetchConst(nodep->thsp())->num());
+            // See #5490. 'numberOperate' on partially out of range select yields 'x' bits,
+            // but in reality it would yield '0's without V3Table, so force 'x' bits to '0',
+            // to ensure the result is the same with and without V3Table.
+            if (!m_params && VN_IS(nodep, Sel) && valuep->num().isAnyX()) {
+                V3Number num{valuep, valuep->width()};
+                num.opAssign(valuep->num());
+                valuep->num().opBitsOne(num);
+            }
         }
     }
     void visit(AstNodeQuadop* nodep) override {

diff --git a/src/V3Table.cpp b/src/V3Table.cpp
@@ -315,6 +315,7 @@ class TableVisitor final : public VNVisitor {
             for (TableOutputVar& tov : m_outVarps) {
                 if (V3Number* const outnump = simvis.fetchOutNumberNull(tov.varScopep())) {
                     UINFO(8, "   Output " << tov.name() << " = " << *outnump << endl);
+                    UASSERT_OBJ(!outnump->isAnyXZ(), outnump, "Table should not contain X/Z");
                     outputAssignedMask.setBit(tov.ord(), 1);  // Mark output as assigned
                     tov.addValue(inValue, *outnump);
                 } else {

diff --git a/test_regress/t/t_select_bound1.v b/test_regress/t/t_select_bound1.v
@@ -83,6 +83,8 @@ module Test (/*AUTOARG*/
       // verilator lint_off WIDTH
       out <= p[15 + in -: 5];
       // verilator lint_on WIDTH
+   end
+   always @(posedge clk) begin
       mask[3] <= ((15 + in - 5) < 12);
       mask[2] <= ((15 + in - 5) < 13);
       mask[1] <= ((15 + in - 5) < 14);