diff --git a/src/V3MergeCond.cpp b/src/V3MergeCond.cpp index 483d32c15..2d8d39f36 100644 --- a/src/V3MergeCond.cpp +++ b/src/V3MergeCond.cpp @@ -108,11 +108,11 @@ private: virtual void visit(AstNode* nodep) VL_OVERRIDE { iterateChildrenConst(nodep); } public: - // Set user1 on all referenced AstVar - void operator()(AstNode* node) { - AstNode::user1ClearTree(); - iterate(node); - } + // Remove marks from AstVars (clear user1) + void clear() { AstNode::user1ClearTree(); } + + // Mark all AstVars referenced by setting user1 + void mark(AstNode* node) { iterate(node); } }; class MergeCondVisitor : public AstNVisitor { @@ -239,6 +239,7 @@ private: m_mgCondp = NULL; m_mgLastp = NULL; m_mgNextp = NULL; + m_markVars.clear(); } void addToList(AstNode* nodep, AstNode* condp) { @@ -248,7 +249,7 @@ private: m_mgFirstp = nodep; m_mgCondp = condp; m_listLenght = 0; - m_markVars(condp); + m_markVars.mark(condp); } // Add node ++m_listLenght; @@ -268,9 +269,14 @@ private: if (AstNodeCond* const condp = extractCond(rhsp)) { if (!m_checkMergeable(nodep)) { // Node not mergeable. - // Finish current list if any, do not start a new one. - if (m_mgFirstp) mergeEnd(); - return; + // If no current list, then this node is just special, move on. + if (!m_mgFirstp) return; + // Otherwise finish current list + mergeEnd(); + // Finishing the list might make the node mergeable again, e.g. + // if the reason we could not merge was due to the condition + // being assigned, so check again and stop only if still no. + if (!m_checkMergeable(nodep)) return; } if (m_mgFirstp && (m_mgNextp != nodep || !condp->condp()->sameTree(m_mgCondp))) { // Node in different list, or has different condition. @@ -282,7 +288,7 @@ private: } else if (m_mgFirstp) { // RHS is not a conditional, but we already started a list. // If it's a 1-bit signal, and a mergeable assignment, try reduced forms - if (rhsp->widthMin() == 1 && m_checkMergeable(nodep)) { + if (m_mgNextp == nodep && rhsp->widthMin() == 1 && m_checkMergeable(nodep)) { // Is it a 'lhs = cond & value' or 'lhs = value & cond'? if (AstAnd* const andp = VN_CAST(rhsp, And)) { if (andp->lhsp()->sameTree(m_mgCondp) || andp->rhsp()->sameTree(m_mgCondp)) { @@ -320,6 +326,7 @@ public: m_mgLastp = NULL; m_mgNextp = NULL; m_listLenght = 0; + m_markVars.clear(); iterate(nodep); } virtual ~MergeCondVisitor() { diff --git a/test_regress/t/t_merge_cond.pl b/test_regress/t/t_merge_cond.pl index 529b23ff4..834265b01 100755 --- a/test_regress/t/t_merge_cond.pl +++ b/test_regress/t/t_merge_cond.pl @@ -21,9 +21,9 @@ execute( if ($Self->{vlt}) { # Note, with vltmt this might be split differently, so only checking vlt file_grep($Self->{stats}, qr/Optimizations, MergeCond merges\s+(\d+)/i, - 10); + 11); file_grep($Self->{stats}, qr/Optimizations, MergeCond merged items\s+(\d+)/i, - 640); + 644); file_grep($Self->{stats}, qr/Optimizations, MergeCond longest merge\s+(\d+)/i, 64); } diff --git a/test_regress/t/t_merge_cond.v b/test_regress/t/t_merge_cond.v index 10a12bb29..2484c11cc 100644 --- a/test_regress/t/t_merge_cond.v +++ b/test_regress/t/t_merge_cond.v @@ -162,32 +162,73 @@ module t (/*AUTOARG*/ // Things not to merge always @(posedge clk) begin - reg x; - reg y; - reg z; - reg w; + reg bits [63:0]; + + reg x; + reg y; + reg z; + reg w; + + // Unpack these to test core algorithm + for (int i = 0; i < 64; i = i + 1) begin + bits[i] = crc[i]; + end // Do not merge if condition appears in an LVALUE - x = crc[0]; - y = x ? crc[2] : crc[1]; - x = x ? crc[3] : crc[4]; - x = x ? crc[5] : crc[6]; + x = bits[0]; + y = x ? bits[2] : bits[1]; + x = x ? bits[3] : bits[4]; + x = x ? bits[5] : bits[6]; - `check(x, (crc[0] ? crc[3] : crc[4]) ? crc[5] : crc[6]); - `check(y, crc[0] ? crc[2] : crc[1]); + `check(x, (bits[0] ? bits[3] : bits[4]) ? bits[5] : bits[6]); + `check(y, bits[0] ? bits[2] : bits[1]); + + // However do merge when starting a new list in the same block with the + // previous condition variable, but without the condition being an LVALUE + x = cond2 ? bits[0] : bits[1]; + y = cond2 & bits[2]; + z = cond2 & bits[3]; + w = cond2 & bits[4]; + + `check(x, cond2 ? bits[0] : bits[1]); + `check(y, cond2 & bits[2]); + `check(z, cond2 & bits[3]); + `check(w, cond2 & bits[4]); // Do not merge if condition is not a pure expression $c("int _cnt = 0;"); - x = $c("_cnt++") ? crc[0] : crc[1]; - y = $c("_cnt++") ? crc[2] : crc[3]; - z = $c("_cnt++") ? crc[4] : crc[5]; - w = $c("_cnt++") ? crc[6] : crc[7]; + x = $c("_cnt++") ? bits[0] : bits[1]; + y = $c("_cnt++") ? bits[2] : bits[3]; + z = $c("_cnt++") ? bits[4] : bits[5]; + w = $c("_cnt++") ? bits[6] : bits[7]; $c("if (_cnt != 4) abort();"); - `check(x, crc[1]); - `check(y, crc[2]); - `check(z, crc[4]); - `check(w, crc[6]); + `check(x, bits[1]); + `check(y, bits[2]); + `check(z, bits[4]); + `check(w, bits[6]); + + // Do not merge with assignment under other statement + x = cond2 ? bits[0] : bits[1]; + if (bits[1]) begin + y = cond2 ? bits[2] : bits[3]; + end + + `check(x, cond2 ? bits[0] : bits[1]); + if (bits[1]) begin + `check(y, cond2 ? bits[2] : bits[3]); + end + + // Do not merge with assignment under other statement + x = cond2 ? bits[0] : bits[1]; + if (bits[1]) begin + y = cond2 & bits[2]; + end + + `check(x, cond2 ? bits[0] : bits[1]); + if (bits[1]) begin + `check(y, cond2 & bits[2]); + end end endmodule