From d42f9c095b4b13267f156062600dc99a377bab50 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Tue, 9 Jun 2020 07:13:40 -0400 Subject: [PATCH] Delay parsing of associative arrays until dtypes known. --- src/V3AstNodes.h | 60 ++++++++++++++++++++----------- src/V3Param.cpp | 6 ++++ src/V3ParseGrammar.cpp | 13 +++---- src/V3Width.cpp | 38 +++++++++++++++++++- src/verilog.y | 9 +++-- test_regress/.gdbinit | 2 +- test_regress/t/t_param_bracket.pl | 17 +++++++++ test_regress/t/t_param_bracket.v | 18 ++++++++++ test_regress/t/t_xml_tag.out | 6 ++-- 9 files changed, 131 insertions(+), 38 deletions(-) create mode 100755 test_regress/t/t_param_bracket.pl create mode 100644 test_regress/t/t_param_bracket.v diff --git a/src/V3AstNodes.h b/src/V3AstNodes.h index 200a6c877..e92aee501 100644 --- a/src/V3AstNodes.h +++ b/src/V3AstNodes.h @@ -231,33 +231,23 @@ public: virtual bool same(const AstNode* samep) const { return true; } }; -class AstAssocRange : public AstNodeRange { - // Associative array range specification - // Only for early parsing - becomes AstAssocDType +class AstBracketRange : public AstNodeRange { + // Parser only concept "[lhsp]", a AstUnknownRange, QueueRange or Range, + // unknown until lhsp type is determined public: - AstAssocRange(FileLine* fl, AstNodeDType* dtp) + AstBracketRange(FileLine* fl, AstNode* elementsp) : ASTGEN_SUPER(fl) { - setOp1p(dtp); + setOp1p(elementsp); } - ASTNODE_NODE_FUNCS(AssocRange) - virtual string emitC() { V3ERROR_NA_RETURN(""); } - virtual string emitVerilog() { V3ERROR_NA_RETURN(""); } - virtual V3Hash sameHash() const { return V3Hash(); } - virtual bool same(const AstNode* samep) const { return true; } - AstNodeDType* keyDTypep() const { return VN_CAST(op1p(), NodeDType); } -}; - -class AstQueueRange : public AstNodeRange { - // Queue range specification - // Only for early parsing - becomes AstQueueDType -public: - explicit AstQueueRange(FileLine* fl) - : ASTGEN_SUPER(fl) {} - ASTNODE_NODE_FUNCS(QueueRange) + ASTNODE_NODE_FUNCS(BracketRange) virtual string emitC() { V3ERROR_NA_RETURN(""); } virtual string emitVerilog() { V3ERROR_NA_RETURN(""); } virtual V3Hash sameHash() const { return V3Hash(); } virtual bool same(const AstNode* samep) const { return true; } + // Will be removed in V3Width, which relies on this + // being a child not a dtype pointed node + virtual bool maybePointedTo() const { return false; } + AstNode* elementsp() const { return op1p(); } }; class AstUnsizedRange : public AstNodeRange { @@ -568,6 +558,36 @@ public: virtual int widthTotalBytes() const { return subDTypep()->widthTotalBytes(); } }; +class AstBracketArrayDType : public AstNodeDType { + // Associative/Queue/Normal array data type, ie "[dtype_or_expr]" + // only for early parsing then becomes another data type + // Children: DTYPE (moved to refDTypep() in V3Width) + // Children: DTYPE (the key) +public: + AstBracketArrayDType(FileLine* fl, VFlagChildDType, AstNodeDType* dtp, AstNode* elementsp) + : ASTGEN_SUPER(fl) { + setOp1p(dtp); // Only for parser + setOp2p(elementsp); // Only for parser + } + ASTNODE_NODE_FUNCS(BracketArrayDType) + virtual bool similarDType(AstNodeDType* samep) const { V3ERROR_NA_RETURN(false); } + // op1 = Range of variable + AstNodeDType* childDTypep() const { return VN_CAST(op1p(), NodeDType); } + virtual AstNodeDType* subDTypep() const { return childDTypep(); } + // op2 = Range of variable + AstNode* elementsp() const { return op2p(); } + // METHODS + // Will be removed in V3Width, which relies on this + // being a child not a dtype pointed node + virtual bool maybePointedTo() const { return false; } + virtual AstBasicDType* basicp() const { return NULL; } + virtual AstNodeDType* skipRefp() const { return (AstNodeDType*)this; } + virtual AstNodeDType* skipRefToConstp() const { return (AstNodeDType*)this; } + virtual AstNodeDType* skipRefToEnump() const { return (AstNodeDType*)this; } + virtual int widthAlignBytes() const { V3ERROR_NA_RETURN(0); } + virtual int widthTotalBytes() const { V3ERROR_NA_RETURN(0); } +}; + class AstDynArrayDType : public AstNodeDType { // Dynamic array data type, ie "[]" // Children: DTYPE (moved to refDTypep() in V3Width) diff --git a/src/V3Param.cpp b/src/V3Param.cpp index 11aeeacc7..ad43808c2 100644 --- a/src/V3Param.cpp +++ b/src/V3Param.cpp @@ -181,6 +181,12 @@ private: if (AstUnpackArrayDType* adtypep = VN_CAST(nodep, UnpackArrayDType)) { return adtypep->subDTypep(); } + // We have not resolved parameter of the child yet, so still + // have BracketArrayDType's. We'll presume it'll end up as assignment + // compatible (or V3Width will complain). + if (AstBracketArrayDType* adtypep = VN_CAST(nodep, BracketArrayDType)) { + return adtypep->subDTypep(); + } return NULL; } void collectPins(CloneMap* clonemapp, AstNodeModule* modp) { diff --git a/src/V3ParseGrammar.cpp b/src/V3ParseGrammar.cpp index e42142483..37d040a96 100644 --- a/src/V3ParseGrammar.cpp +++ b/src/V3ParseGrammar.cpp @@ -120,8 +120,6 @@ AstNodeDType* V3ParseGrammar::createArray(AstNodeDType* basep, AstNodeRange* nra if (rangep && isPacked) { arrayp = new AstPackArrayDType(rangep->fileline(), VFlagChildDType(), arrayp, rangep); - } else if (VN_IS(nrangep, QueueRange)) { - arrayp = new AstQueueDType(nrangep->fileline(), VFlagChildDType(), arrayp, NULL); } else if (rangep && (VN_IS(rangep->leftp(), Unbounded) || VN_IS(rangep->rightp(), Unbounded))) { @@ -132,12 +130,11 @@ AstNodeDType* V3ParseGrammar::createArray(AstNodeDType* basep, AstNodeRange* nra rangep); } else if (VN_IS(nrangep, UnsizedRange)) { arrayp = new AstUnsizedArrayDType(nrangep->fileline(), VFlagChildDType(), arrayp); - } else if (VN_IS(nrangep, AssocRange)) { - AstAssocRange* arangep = VN_CAST(nrangep, AssocRange); - AstNodeDType* keyp = arangep->keyDTypep(); - keyp->unlinkFrBack(); - arrayp - = new AstAssocArrayDType(nrangep->fileline(), VFlagChildDType(), arrayp, keyp); + } else if (VN_IS(nrangep, BracketRange)) { + AstBracketRange* arangep = VN_CAST(nrangep, BracketRange); + AstNode* keyp = arangep->elementsp()->unlinkFrBack(); + arrayp = new AstBracketArrayDType(nrangep->fileline(), VFlagChildDType(), arrayp, + keyp); } else { UASSERT_OBJ(0, nrangep, "Expected range or unsized range"); } diff --git a/src/V3Width.cpp b/src/V3Width.cpp index b00a52ec1..e314d93f9 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -1061,7 +1061,7 @@ private: } virtual void visit(AstUnbounded* nodep) VL_OVERRIDE { nodep->dtypeSetSigned32(); // Used in int context - if (!VN_IS(nodep->backp(), IsUnbounded) + if (!VN_IS(nodep->backp(), IsUnbounded) && !VN_IS(nodep->backp(), BracketArrayDType) && !(VN_IS(nodep->backp(), Var) && VN_CAST(nodep->backp(), Var)->isParam())) { nodep->v3error("Unsupported/illegal unbounded ('$') in this context."); } @@ -1354,6 +1354,35 @@ private: nodep->dtypep(nodep); // The array itself, not subDtype UINFO(4, "dtWidthed " << nodep << endl); } + virtual void visit(AstBracketArrayDType* nodep) VL_OVERRIDE { + // Type inserted only because parser didn't know elementsp() type + // Resolve elementsp's type + userIterateChildren(nodep, WidthVP(SELF, BOTH).p()); + // We must edit when dtype still under normal nodes and before type table + // See notes in iterateEditMoveDTypep + AstNodeDType* childp = nodep->childDTypep(); + childp->unlinkFrBack(); + AstNode* elementsp = nodep->elementsp()->unlinkFrBack(); + AstNode* newp; + if (VN_IS(elementsp, Unbounded)) { + newp = new AstQueueDType(nodep->fileline(), VFlagChildDType(), childp, NULL); + VL_DO_DANGLING(elementsp->deleteTree(), elementsp); + } else if (AstNodeDType* keyp = VN_CAST(elementsp, NodeDType)) { + newp = new AstAssocArrayDType(nodep->fileline(), VFlagChildDType(), childp, keyp); + } else { + // Must be expression that is constant, but we'll determine that later + newp = new AstUnpackArrayDType( + nodep->fileline(), VFlagChildDType(), childp, + new AstRange(nodep->fileline(), new AstConst(elementsp->fileline(), 0), + new AstSub(elementsp->fileline(), elementsp, + new AstConst(elementsp->fileline(), 1)))); + } + nodep->replaceWith(newp); + VL_DO_DANGLING(nodep->deleteTree(), nodep); + // Normally parent's iteration would cover this, but we might have entered by a specific + // visit + VL_DO_DANGLING(userIterate(newp, NULL), newp); + } virtual void visit(AstDynArrayDType* nodep) VL_OVERRIDE { if (nodep->didWidthAndSet()) return; // This node is a dtype & not both PRELIMed+FINALed // Iterate into subDTypep() to resolve that type and update pointer. @@ -1432,6 +1461,10 @@ private: } userIterateChildren(nodep, NULL); if (nodep->subDTypep()) { + // Normally iterateEditMoveDTypep iterate would work, but the refs are under + // the TypeDef which will upset iterateEditMoveDTypep as it can't find it under + // this node's childDTypep + userIterate(nodep->subDTypep(), NULL); nodep->refDTypep(iterateEditMoveDTypep(nodep, nodep->subDTypep())); nodep->typedefp(NULL); // Note until line above subDTypep() may have followed this // Widths are resolved, but special iterate to check for recurstion @@ -4979,6 +5012,9 @@ private: } if (!dtnodep->didWidth()) { UINFO(9, "iterateEditMoveDTypep pointer iterating " << dtnodep << endl); + // See notes in visit(AstBracketArrayDType*) + UASSERT_OBJ(!VN_IS(dtnodep, BracketArrayDType), parentp, + "Brackets should have been iterated as children"); userIterate(dtnodep, NULL); UASSERT_OBJ(dtnodep->didWidth(), parentp, "iterateEditMoveDTypep didn't get width resolution"); diff --git a/src/verilog.y b/src/verilog.y index d841259d8..214131c72 100644 --- a/src/verilog.y +++ b/src/verilog.y @@ -1962,11 +1962,10 @@ variable_dimension: // ==IEEE: variable_dimension '[' ']' { $$ = new AstUnsizedRange($1); } // // IEEE: unpacked_dimension | anyrange { $$ = $1; } - | '[' constExpr ']' { if (VN_IS($2, Unbounded)) { $2->deleteTree(); $$ = new AstQueueRange($1); } - else { $$ = new AstRange($1, new AstConst($1, 0), - new AstSub($1, $2, new AstConst($1, 1))); } } - // // IEEE: associative_dimension - | '[' data_type ']' { $$ = new AstAssocRange($1, $2); } + // // IEEE: unpacked_dimension (if const_expr) + // // IEEE: associative_dimension (if data_type) + // // Can't tell which until see if expr is data type or not + | '[' exprOrDataType ']' { $$ = new AstBracketRange($1, $2); } | yP_BRASTAR ']' { $$ = NULL; BBUNSUP($1, "Unsupported: [*] wildcard associative arrays"); } | '[' '*' ']' { $$ = NULL; BBUNSUP($2, "Unsupported: [*] wildcard associative arrays"); } // // IEEE: queue_dimension diff --git a/test_regress/.gdbinit b/test_regress/.gdbinit index 8b63c91ae..e960ad760 100644 --- a/test_regress/.gdbinit +++ b/test_regress/.gdbinit @@ -1 +1 @@ -source ../src/.gdbinit +source ~/SandBox/homecvs/v4/verilator/src/.gdbinit diff --git a/test_regress/t/t_param_bracket.pl b/test_regress/t/t_param_bracket.pl new file mode 100755 index 000000000..09b2ce4eb --- /dev/null +++ b/test_regress/t/t_param_bracket.pl @@ -0,0 +1,17 @@ +#!/usr/bin/env perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2003 by Wilson Snyder. This program is free software; you +# can redistribute it and/or modify it under the terms of either the GNU +# Lesser General Public License Version 3 or the Perl Artistic License +# Version 2.0. +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +scenarios(linter => 1); + +lint( + ); + +ok(1); +1; diff --git a/test_regress/t/t_param_bracket.v b/test_regress/t/t_param_bracket.v new file mode 100644 index 000000000..b2890ad9a --- /dev/null +++ b/test_regress/t/t_param_bracket.v @@ -0,0 +1,18 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2020 by Wilson Snyder; +// SPDX-License-Identifier: CC0-1.0 + +module t + #(parameter WIDTH = 8) + (/*AUTOARG*/ + // Outputs + o + ); + output [WIDTH-1:0] o; + localparam DEPTH = $clog2(5); + // Note single bracket below + reg [WIDTH-1:0] arid [1< - @@ -69,11 +68,12 @@ + - - + +