From 2a01365f9b775205f5d65c9cb8d40108be469d9b Mon Sep 17 00:00:00 2001
From: Geza Lore <gezalore@gmail.com>
Date: Thu, 26 Sep 2024 16:31:47 +0100
Subject: [PATCH] Fix V3Table trying to generate 'x' bits in the lookup table.
 (#5491)

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 #5490, about leaving partially out of range
selects in the logic after inserting bounds checks in V3Unknown.
---
 src/V3Simulate.h                 | 15 +++++++++++++--
 src/V3Table.cpp                  |  1 +
 test_regress/t/t_select_bound1.v |  2 ++
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/src/V3Simulate.h b/src/V3Simulate.h
index d9b950602..f9832e27b 100644
--- a/src/V3Simulate.h
+++ b/src/V3Simulate.h
@@ -258,6 +258,7 @@ public:
     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 @@ public:
     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 @@ private:
         // 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 @@ private:
         // 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 @@ private:
         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
index a7bddd93e..a25af67dc 100644
--- a/src/V3Table.cpp
+++ b/src/V3Table.cpp
@@ -315,6 +315,7 @@ private:
             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
index adecef33b..811f2bea6 100644
--- 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);