From 4e4fdd3b66b115096dce410003a3e328c168726e Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Mon, 9 Sep 2024 19:56:09 -0400 Subject: [PATCH] Fix multidimensional function return value selects (#5382). --- Changes | 2 ++ src/V3AstNodeExpr.h | 11 ++++++ src/V3ParseGrammar.cpp | 17 +++++++++ src/verilog.y | 59 +++++++++++++++++--------------- test_regress/t/t_bitsel_concat.v | 10 ++++++ 5 files changed, 72 insertions(+), 27 deletions(-) diff --git a/Changes b/Changes index 34e5f038c..7b279a9d4 100644 --- a/Changes +++ b/Changes @@ -27,6 +27,7 @@ Verilator 5.029 devel * Improve performance of V3VariableOrder with parallelism (#5406). [Bartłomiej Chmiel, Antmicro Ltd.] * Fix performance of V3Trace when many activity blocks (#5372). [Deniz Güzel] * Fix REALCVT warning on integral timescale conversions (#5378). [Liam Braun] +* Fix multidimensional function return value selects (#5382). [Gökçe Aydos] * Fix dot fallback finding wrong symbols (#5394). [Arkadiusz Kozdra, Antmicro Ltd.] * Fix infinite recursion due to recursive functions/tasks (#5398). [Krzysztof Bieganski, Antmicro Ltd.] * Fix V3Randomize compile error on old GCC (#5403) (#5417). [Krzysztof Bieganski, Antmicro Ltd.] @@ -40,6 +41,7 @@ Verilator 5.029 devel * Fix associative array next/prev/first/last mis-propagating constants (#5435). [Ethan Sifferman] + Verilator 5.028 2024-08-21 ========================== diff --git a/src/V3AstNodeExpr.h b/src/V3AstNodeExpr.h index f053ece54..4ad72ba54 100644 --- a/src/V3AstNodeExpr.h +++ b/src/V3AstNodeExpr.h @@ -1660,6 +1660,17 @@ public: bool same(const AstNode* /*samep*/) const override { return true; } int instrCount() const override { return widthInstrs(); } }; +class AstParseHolder final : public AstNodeExpr { + // A reference to something soon to replace, used in a select at parse time + // that needs conversion to pull the upper lvalue later +public: + AstParseHolder(FileLine* fl) + : ASTGEN_SUPER_ParseHolder(fl) {} + ASTGEN_MEMBERS_AstParseHolder; + string emitVerilog() override { V3ERROR_NA_RETURN(""); } + string emitC() override { V3ERROR_NA_RETURN(""); } + bool cleanOut() const override { V3ERROR_NA_RETURN(true); } +}; class AstParseRef final : public AstNodeExpr { // A reference to a variable, function or task // We don't know which at parse time due to bison constraints diff --git a/src/V3ParseGrammar.cpp b/src/V3ParseGrammar.cpp index b4835bc80..6451ba514 100644 --- a/src/V3ParseGrammar.cpp +++ b/src/V3ParseGrammar.cpp @@ -128,6 +128,23 @@ AstRange* V3ParseGrammar::scrubRange(AstNodeRange* nrangep) { return VN_CAST(nrangep, Range); } +AstNodePreSel* V3ParseGrammar::scrubSel(AstNodeExpr* fromp, AstNodePreSel* selp) VL_MT_DISABLED { + // SEL(PARSELVALUE, ...) -> SEL(fromp, ...) + AstNodePreSel* subSelp = selp; + while (true) { + if (VN_IS(subSelp->fromp(), ParseHolder)) break; + if (AstNodePreSel* const lowerSelp = VN_CAST(subSelp->fromp(), NodePreSel)) { + subSelp = lowerSelp; + continue; + } + subSelp->v3fatalSrc("Couldn't find where to insert expression into select"); + } + AstNode* subSelFromp = subSelp->fromp(); + subSelFromp->replaceWith(fromp); + VL_DO_DANGLING(subSelFromp->deleteTree(), subSelFromp); + return selp; +} + AstNodeDType* V3ParseGrammar::createArray(AstNodeDType* basep, AstNodeRange* nrangep, bool isPacked) { // Split RANGE0-RANGE1-RANGE2 diff --git a/src/verilog.y b/src/verilog.y index f65dac85c..79da06d77 100644 --- a/src/verilog.y +++ b/src/verilog.y @@ -125,6 +125,7 @@ public: return v3Global.opt.trace() && m_tracingParse && fl->tracingOn(); } AstRange* scrubRange(AstNodeRange* rangep) VL_MT_DISABLED; + AstNodePreSel* scrubSel(AstNodeExpr* fromp, AstNodePreSel* selp) VL_MT_DISABLED; AstNodeDType* createArray(AstNodeDType* basep, AstNodeRange* rangep, bool isPacked) VL_MT_DISABLED; AstVar* createVariable(FileLine* fileline, const string& name, AstNodeRange* arrayp, @@ -1181,6 +1182,13 @@ BISONPRE_VERSION(3.7,%define api.header.include {"V3ParseBison.h"}) // Blank lines for type insertion // Blank lines for type insertion // Blank lines for type insertion +// Blank lines for type insertion +// Blank lines for type insertion +// Blank lines for type insertion +// Blank lines for type insertion +// Blank lines for type insertion +// Blank lines for type insertion +// Blank lines for type insertion %start source_text @@ -3112,13 +3120,26 @@ rangeList: // IEEE: {packed_dimension} | rangeList anyrange { $$ = $1->addNext($2); } ; -// IEEE: select -// Merged into more general idArray - anyrange: '[' constExpr ':' constExpr ']' { $$ = new AstRange{$1, $2, $4}; } ; +part_select_rangeList: // IEEE: part_select_range (as used after function calls) + part_select_range { $$ = $1; } + | part_select_rangeList part_select_range { $$ = GRAMMARP->scrubSel($1, $2); } + ; + +part_select_range: + '[' expr ']' + { $$ = new AstSelBit{$1, new AstParseHolder{$1}, $2}; } + | '[' constExpr ':' constExpr ']' + { $$ = new AstSelExtract{$1, new AstParseHolder{$1}, $2, $4}; } + | '[' expr yP_PLUSCOLON constExpr ']' + { $$ = new AstSelPlus{$1, new AstParseHolder{$1}, $2, $4}; } + | '[' expr yP_MINUSCOLON constExpr ']' + { $$ = new AstSelMinus{$1, new AstParseHolder{$1}, $2, $4}; } + ; + packed_dimensionListE: // IEEE: [{ packed_dimension }] /* empty */ { $$ = nullptr; } | packed_dimensionList { $$ = $1; } @@ -4964,38 +4985,22 @@ expr: // IEEE: part of expression/constant_expression/ { $$ = new AstSelMinus{$7, new AstReplicate{$3, $4, $2}, $8, $10}; } // // UNSUP some other rules above // + // // IEEE grammar error: function_subroutine_call [ range_expression ] + // // should be instead: function_subroutine_call [ part_select_range ] | function_subroutine_callNoMethod { $$ = $1; } - | function_subroutine_callNoMethod '[' expr ']' - { $$ = new AstSelBit{$2, $1, $3}; } - | function_subroutine_callNoMethod '[' constExpr ':' constExpr ']' - { $$ = new AstSelExtract{$2, $1, $3, $5}; } - | function_subroutine_callNoMethod '[' expr yP_PLUSCOLON constExpr ']' - { $$ = new AstSelPlus{$2, $1, $3, $5}; } - | function_subroutine_callNoMethod '[' expr yP_MINUSCOLON constExpr ']' - { $$ = new AstSelMinus{$2, $1, $3, $5}; } + | function_subroutine_callNoMethod part_select_rangeList + { $$ = GRAMMARP->scrubSel($1, $2); } // // method_call | ~l~expr '.' function_subroutine_callNoMethod { $$ = new AstDot{$2, false, $1, $3}; } - | ~l~expr '.' function_subroutine_callNoMethod '[' expr ']' - { $$ = new AstSelBit{$4, new AstDot{$2, false, $1, $3}, $5}; } - | ~l~expr '.' function_subroutine_callNoMethod '[' constExpr ':' constExpr ']' - { $$ = new AstSelExtract{$4, new AstDot{$2, false, $1, $3}, $5, $7}; } - | ~l~expr '.' function_subroutine_callNoMethod '[' expr yP_PLUSCOLON constExpr ']' - { $$ = new AstSelPlus{$4, new AstDot{$2, false, $1, $3}, $5, $7}; } - | ~l~expr '.' function_subroutine_callNoMethod '[' expr yP_MINUSCOLON constExpr ']' - { $$ = new AstSelMinus{$4, new AstDot{$2, false, $1, $3}, $5, $7}; } + | ~l~expr '.' function_subroutine_callNoMethod part_select_rangeList + { $$ = GRAMMARP->scrubSel(new AstDot{$2, false, $1, $3}, $4); } // // method_call:array_method requires a '.' | ~l~expr '.' array_methodWith { $$ = new AstDot{$2, false, $1, $3}; } - | ~l~expr '.' array_methodWith '[' expr ']' - { $$ = new AstSelBit{$4, new AstDot{$2, false, $1, $3}, $5}; } - | ~l~expr '.' array_methodWith '[' constExpr ':' constExpr ']' - { $$ = new AstSelExtract{$4, new AstDot{$2, false, $1, $3}, $5, $7}; } - | ~l~expr '.' array_methodWith '[' expr yP_PLUSCOLON constExpr ']' - { $$ = new AstSelPlus{$4, new AstDot{$2, false, $1, $3}, $5, $7}; } - | ~l~expr '.' array_methodWith '[' expr yP_MINUSCOLON constExpr ']' - { $$ = new AstSelMinus{$4, new AstDot{$2, false, $1, $3}, $5, $7}; } + | ~l~expr '.' array_methodWith part_select_rangeList + { $$ = GRAMMARP->scrubSel(new AstDot{$2, false, $1, $3}, $4); } // // // IEEE: let_expression // // see funcRef diff --git a/test_regress/t/t_bitsel_concat.v b/test_regress/t/t_bitsel_concat.v index c18aa2c2e..34c05f8ed 100644 --- a/test_regress/t/t_bitsel_concat.v +++ b/test_regress/t/t_bitsel_concat.v @@ -34,6 +34,13 @@ module t(/*AUTOARG*/); return 16'hf0ed; endfunction + int i; + typedef int arr_t[1:0][3:0]; + + function arr_t valarr(); + return '{'{1,2,3,4}, '{5,6,7,8}}; + endfunction + initial begin aa = 8'haa; bb = 8'hbb; @@ -84,6 +91,9 @@ module t(/*AUTOARG*/); s8 = q.sum()[11-:8]; `checkh(s8, 8'h0e); + i = valarr()[1][2]; + $display(i); + $write("*-* All Finished *-*\n"); $finish; end