From e9fcd6eb250ec5309c441d94bfcdb524b8ba62f2 Mon Sep 17 00:00:00 2001 From: Yutetsu TAKATSUKASA Date: Wed, 15 Jan 2020 23:01:13 +0900 Subject: [PATCH] split_var supports assignment of unpacked arrays. --- bin/verilator | 2 - src/V3SplitVar.cpp | 90 ++++++++++++++++++++-------- test_regress/t/t_split_var_0.v | 33 ++++++++-- test_regress/t/t_split_var_1_bad.out | 20 +------ test_regress/t/t_split_var_1_bad.v | 28 --------- 5 files changed, 95 insertions(+), 78 deletions(-) diff --git a/bin/verilator b/bin/verilator index ee0d66631..89788a418 100755 --- a/bin/verilator +++ b/bin/verilator @@ -4411,8 +4411,6 @@ Possible reasons are The datatype is not supported for splitting. (e.g. real) The access pattern of the variable can not be determined statically. (e.g. memory) - The unpacked array is accessed other than subscript. - slicing (unpacked_array[lsb:msb]) and accessing without subscript are not supported yet. =item STMTDLY diff --git a/src/V3SplitVar.cpp b/src/V3SplitVar.cpp index 425fd676d..5582432e4 100644 --- a/src/V3SplitVar.cpp +++ b/src/V3SplitVar.cpp @@ -73,13 +73,6 @@ // // // -// Limitations: (planned to be resolved) -// - Unpacked array must be accessed via AstArraySel. -// e.g. unpacked_array[3] : supported -// unpacked_array[0:1] : not supported because AstSliceSel is used -// unpacked_array : not supported -// to allow such access, concatenate op node to build unpacked array is necessary on AST. -// //************************************************************************* #include "config_build.h" @@ -107,6 +100,18 @@ static AstConst* constifyIfNot(AstNode* nodep) { return constp; } +// returns of outer most dimension of an unpacked array +static std::pair outerMostSizeOfUnpackedArray(AstVar* nodep) { + AstUnpackArrayDType* const dtypep = VN_CAST(nodep->dtypep(), UnpackArrayDType); + AstConst* const lsbp = constifyIfNot(dtypep->rangep()->lsbp()); + AstConst* const msbp = constifyIfNot(dtypep->rangep()->msbp()); + UASSERT_OBJ(lsbp, dtypep->rangep()->lsbp(), "must be constant"); + UASSERT_OBJ(msbp, dtypep->rangep()->msbp(), "must be constant"); + const vlsint32_t lsb = lsbp->toSInt(), msb = msbp->toSInt(); + UASSERT_OBJ(lsb <= msb, dtypep->rangep(), "lsb must not greater than msb"); + return std::make_pair(msb, lsb); +} + //###################################################################### // Find a variable with pragma @@ -200,6 +205,57 @@ class SplitUnpackedVarVisitor : public AstNVisitor { " Such access is not supported yet.\n"); m_refs.erase(varp); } + + static void splitSimpleAssign(AstNodeAssign* asnp, AstVarRef* lhsp, AstVarRef *rhsp, int lstart, int rstart, int num) { + for (int i = 0; i < num; ++i) { + AstVarRef* const lrefp = new AstVarRef(lhsp->fileline(), lhsp->varp(), true); + AstVarRef* const rrefp = new AstVarRef(rhsp->fileline(), rhsp->varp(), false); + AstArraySel* const lselp = new AstArraySel(lhsp->fileline(), lrefp, lstart + i); + AstArraySel* const rselp = new AstArraySel(rhsp->fileline(), rrefp, rstart + i); + // the added new assignment statement will be visited later. + asnp->addNext(asnp->cloneType(lselp, rselp)); + } + } + + // Unroll assignments of SliceSel or entire unpacked array to multiple assignment + virtual void visit(AstNodeAssign* nodep) VL_OVERRIDE { + AstSliceSel* lsel = VN_CAST(nodep->lhsp(), SliceSel); + AstSliceSel* rsel = VN_CAST(nodep->rhsp(), SliceSel); + AstVarRef* const lhsp = VN_CAST(lsel ? lsel->fromp() : nodep->lhsp(), VarRef); + AstVarRef* const rhsp = VN_CAST(rsel ? rsel->fromp() : nodep->rhsp(), VarRef); + // unless simple assignment, nothing to do in this function + if (!lhsp || !rhsp) { + iterateChildren(nodep); + return; + } + + // if nodep is a simple assignment of variables without split_var pragma, quick exit + if (m_refs.find(lhsp->varp()) == m_refs.end() && + m_refs.find(rhsp->varp()) == m_refs.end()) return; + + int lstart, lnum, rstart, rnum; + if (lsel) { + lstart = lsel->declRange().lo(); + lnum = lsel->declRange().elements(); + } else { // LHS is entire array + const std::pair lrange = outerMostSizeOfUnpackedArray(lhsp->varp()); + lstart = 0; + lnum = lrange.first - lrange.second + 1; + } + if (rsel) { + rstart = rsel->declRange().lo(); + rnum = rsel->declRange().elements(); + } else { // RHS is entire array + const std::pair rrange = outerMostSizeOfUnpackedArray(rhsp->varp()); + rstart = 0; + rnum = rrange.first - rrange.second + 1; + } + if (lnum != rnum) return; // strange. V3Slice will show proper diagnosis + splitSimpleAssign(nodep, lhsp, rhsp, lstart, rstart, lnum); + nodep->unlinkFrBack()->deleteTree(); + VL_DANGLING(nodep); + } + virtual void visit(AstArraySel* nodep) VL_OVERRIDE { AstVarRef* const vrefp = VN_CAST(nodep->fromp(), VarRef); if (!vrefp) { @@ -224,18 +280,6 @@ class SplitUnpackedVarVisitor : public AstNVisitor { m_refs.erase(varp); } } - virtual void visit(AstSliceSel* nodep) VL_OVERRIDE { - AstVarRef* const vrefp = VN_CAST(nodep->fromp(), VarRef); - if (!vrefp) return; - - AstVar* const varp = vrefp->varp(); - if (m_refs.find(varp) == m_refs.end()) return; // variable without split_var pragma - if (m_firstRun) - nodep->v3warn(SPLITVAR, "Variable " << vrefp->prettyNameQ() - << " will not be split because slicing an " - "unpacked array is not supported yet."); - m_refs.erase(varp); - } // The actual splitting operation is done in this function. void split() { for (vl_unordered_map >::iterator it = m_refs.begin(), @@ -249,12 +293,8 @@ class SplitUnpackedVarVisitor : public AstNVisitor { AstUnpackArrayDType* const dtypep = VN_CAST(varp->dtypep(), UnpackArrayDType); std::vector vars; // Add the split variables - AstConst* const lsbp = constifyIfNot(dtypep->rangep()->lsbp()); - AstConst* const msbp = constifyIfNot(dtypep->rangep()->msbp()); - UASSERT_OBJ(lsbp, dtypep->rangep()->lsbp(), "must be constant"); - UASSERT_OBJ(msbp, dtypep->rangep()->msbp(), "must be constant"); - const vlsint32_t lsb = lsbp->toSInt(), msb = msbp->toSInt(); - UASSERT_OBJ(lsb <= msb, dtypep->rangep(), "lsb must not greater than msb"); + const std::pair arraySize = outerMostSizeOfUnpackedArray(varp); + const int msb = arraySize.first, lsb = arraySize.second; for (vlsint32_t i = 0; i <= msb - lsb; ++i) { // const std::string name = varp->name() + "__BRA__" + AstNode::encodeNumber(i + lsb) + "__KET__"; // unpacked array is traced as var(idx). diff --git a/test_regress/t/t_split_var_0.v b/test_regress/t/t_split_var_0.v index 649fd61b4..72097ac56 100644 --- a/test_regress/t/t_split_var_0.v +++ b/test_regress/t/t_split_var_0.v @@ -47,23 +47,46 @@ module barshift_2d_unpacked #(parameter depth = 2, localparam width = 2**depth) localparam offset = 1; localparam n = 3; - reg [width-1:0]tmp[depth+offset:offset][offset:offset+n-1]; /*verilator split_var*/ + reg [width-1:0]tmp0[depth+offset:offset][offset:offset+n-1]; /*verilator split_var*/ + reg [width-1:0]tmp1[depth+offset:offset][offset:offset+n-1]; /*verilator split_var*/ + reg [width-1:0]tmp2[depth+offset:offset][offset:offset+n-1]; + reg [width-1:0]tmp3[depth+offset:offset][offset:offset+n-1]; /*verilator split_var*/ + reg [width-1:0]tmp4[depth+offset:offset][offset:offset+n-1]; /*verilator split_var*/ + reg [width-1:0]tmp5[depth+offset:offset][offset:offset+n-1]; + reg [width-1:0]tmp6[depth+offset:offset][offset:offset+n-1]; /*verilator split_var*/ + + reg [width-1:0]tmp7[depth+offset+1:offset+1][offset:offset+n-1]; /*verilator split_var*/ + reg [width-1:0]tmp8[depth+offset+3:offset-1][offset:offset+n-1]; /*verilator split_var*/ + reg [width-1:0]tmp9[depth+offset+3:offset+3][offset:offset+n-1]; /*verilator split_var*/ + reg [width-1:0]tmp10[depth+offset:offset][offset:offset+n-1]; /*verilator split_var*/ + generate for(genvar i = 0; i < depth; ++i) begin for(genvar j = offset; j < n + offset; ++j) begin always_comb if (shift[i]) begin - tmp[i+1+offset][j] = {tmp[i+offset][j][(1 << i)-1:0], tmp[i+offset][j][width-1:(2**i)]}; + tmp0[i+1+offset][j] = {tmp0[i+offset][j][(1 << i)-1:0], tmp0[i+offset][j][width-1:(2**i)]}; end else begin - tmp[i+1+offset][j] = tmp[i+offset][j]; + tmp0[i+1+offset][j] = tmp0[i+offset][j]; end end end for(genvar j = offset; j < n + offset; ++j) begin - assign tmp[0 + offset][j] = in; + assign tmp0[0 + offset][j] = in; end endgenerate - assign out = tmp[depth+offset][offset]; + assign tmp1 = tmp0; // split both side + assign tmp2 = tmp1; // split only rhs + assign tmp3 = tmp2; // split only lhs + always_comb tmp4 = tmp3; // split both side + always_comb tmp5 = tmp4; // split only rhs + always_comb tmp6 = tmp5; // split only lhs + + assign tmp7 = tmp6; + assign tmp8[depth+offset+1:offset+1] = tmp7; + assign tmp9 = tmp8[depth+offset+1:offset+1]; + assign tmp10[depth+offset:offset] = tmp9[depth+offset+3:offset+3]; + assign out = tmp10[depth+offset][offset]; endmodule diff --git a/test_regress/t/t_split_var_1_bad.out b/test_regress/t/t_split_var_1_bad.out index ea8686501..2f0000b65 100644 --- a/test_regress/t/t_split_var_1_bad.out +++ b/test_regress/t/t_split_var_1_bad.out @@ -3,26 +3,10 @@ /*verilator split_var*/ ^~~~~~~~~~~~~~~~~~~~~~~ ... Use "/* verilator lint_off SPLITVAR */" and lint_on around source to disable this message. -%Warning-SPLITVAR: t/t_split_var_1_bad.v:31: Variable 'cannot_split' will not be split because index cannot be determined statically. +%Warning-SPLITVAR: t/t_split_var_1_bad.v:28: Variable 'cannot_split' will not be split because index cannot be determined statically. : ... In instance t.i_sub0 rd_data = cannot_split[addr]; ^~~~ -%Warning-SPLITVAR: t/t_split_var_1_bad.v:41: Variable 'cannot_split0' will not be split because the entire unpacked array is referred. Such access is not supported yet. - : ... In instance t.i_sub1 - cannot_split1 = cannot_split0; - ^~~~~~~~~~~~~ -%Warning-SPLITVAR: t/t_split_var_1_bad.v:41: Variable 'cannot_split1' will not be split because the entire unpacked array is referred. Such access is not supported yet. - : ... In instance t.i_sub1 - cannot_split1 = cannot_split0; - ^~~~~~~~~~~~~ -%Warning-SPLITVAR: t/t_split_var_1_bad.v:63: Variable 'cannot_split1' will not be split because slicing an unpacked array is not supported yet. - : ... In instance t.i_sub3 - cannot_split1[0:1] = cannot_split1[2:3]; - ^ -%Warning-SPLITVAR: t/t_split_var_1_bad.v:64: Variable 'cannot_split0' will not be split because the entire unpacked array is referred. Such access is not supported yet. - : ... In instance t.i_sub3 - cannot_split1 = cannot_split0; - ^~~~~~~~~~~~~ %Warning-SPLITVAR: t/t_split_var_1_bad.v:5: Pragma split_var is specified on a variable whose type is unsupported or public. Packed portion must be an aggregate type of bit or logic. : ... In instance t real should_show_warning0; /*verilator split_var*/ @@ -35,7 +19,7 @@ : ... In instance t wire should_show_warning2; /*verilator split_var*/ ^~~~~~~~~~~~~~~~~~~~~~~ -%Warning-SPLITVAR: t/t_split_var_1_bad.v:52: Variable 'cannot_split' will not be split because bit range cannot be determined statically. +%Warning-SPLITVAR: t/t_split_var_1_bad.v:37: Variable 'cannot_split' will not be split because bit range cannot be determined statically. : ... In instance t.i_sub2 rd_data = cannot_split[addr]; ^ diff --git a/test_regress/t/t_split_var_1_bad.v b/test_regress/t/t_split_var_1_bad.v index 9bb4b5d1f..8b5fbed13 100644 --- a/test_regress/t/t_split_var_1_bad.v +++ b/test_regress/t/t_split_var_1_bad.v @@ -10,10 +10,7 @@ module t(); logic [7:0] rd_data0, rd_data1, rd_data2; sub0 i_sub0(.addr(addr), .rd_data(rd_data0)); - sub1 i_sub1( .rd_data(rd_data1)); sub2 i_sub2(.addr(addr), .rd_data(rd_data2)); - sub3 i_sub3(); - initial begin addr = 0; @@ -33,18 +30,6 @@ module sub0(input [3:0]addr, output logic [7:0] rd_data); endmodule -module sub1(output logic [7:0] rd_data); - - logic [7:0] cannot_split0[0:15]; /*verilator split_var*/ - logic [7:0] cannot_split1[0:15]; /*verilator split_var*/ - always_comb begin - cannot_split1 = cannot_split0; - rd_data = cannot_split1[0]; - end - -endmodule - - module sub2(input [3:0]addr, output logic [7:0] rd_data); logic [15:0] [7:0] cannot_split; /*verilator split_var*/ @@ -53,16 +38,3 @@ module sub2(input [3:0]addr, output logic [7:0] rd_data); endmodule - -module sub3(); - - logic cannot_split0 [15:0] [7:0] ; /*verilator split_var*/ - logic cannot_split1 [0:15] [7:0] ; /*verilator split_var*/ - /* verilator lint_off ALWCOMBORDER */ - always_comb begin - cannot_split1[0:1] = cannot_split1[2:3]; - cannot_split1 = cannot_split0; - end - /* verilator lint_on ALWCOMBORDER */ - -endmodule