diff --git a/Changes b/Changes index ecb293a7d..05dbcd8e5 100644 --- a/Changes +++ b/Changes @@ -5,6 +5,8 @@ indicates the contributor was also the author of the fix; Thanks! * Verilator 3.71**** +**** Fix writing to out-of-bounds arrays writing element 0. + **** Fix VCD files showing internal flattened hierarchy, broke in 3.714. * Verilator 3.714 2009/09/18 diff --git a/src/V3Ast.cpp b/src/V3Ast.cpp index 83fa35783..65ba63ba0 100644 --- a/src/V3Ast.cpp +++ b/src/V3Ast.cpp @@ -574,6 +574,14 @@ void AstNode::relinkOneLink(AstNode*& pointpr, // Ref to pointer that gets set pointpr = newp; } +void AstNode::addHereThisAsNext (AstNode* newp) { + // {old}->this->{next} becomes {old}->new->this->{next} + AstNRelinker handle; + this->unlinkFrBackWithNext(&handle); + newp->addNext(this); + handle.relink(newp); +} + void AstNode::swapWith (AstNode* bp) { AstNRelinker aHandle; AstNRelinker bHandle; diff --git a/src/V3Ast.h b/src/V3Ast.h index 222be95be..6b3c9f9f5 100644 --- a/src/V3Ast.h +++ b/src/V3Ast.h @@ -748,6 +748,7 @@ public: AstNode* addNext(AstNode* newp); // Returns this, adds to end of list AstNode* addNextNull(AstNode* newp); // Returns this, adds to end of list, NULL is OK void addNextHere(AstNode* newp); // Adds after speced node + void addHereThisAsNext(AstNode* newp); // Adds at old place of this, this becomes next void replaceWith(AstNode* newp); // Replace current node in tree with new node void v3errorEnd(ostringstream& str) const; virtual void dump(ostream& str=cout); @@ -756,6 +757,9 @@ public: void swapWith(AstNode* bp); void relink(AstNRelinker* linkerp); // Generally use linker->relink() instead void cloneRelinkNode() { cloneRelink(); } + // Iterate and insert - assumes tree format + virtual void addNextStmt(AstNode* newp, AstNode* belowp); // When calling, "this" is second argument + virtual void addBeforeStmt(AstNode* newp, AstNode* belowp); // When calling, "this" is second argument // METHODS - Iterate on a tree AstNode* cloneTree(bool cloneNextLink); @@ -950,6 +954,8 @@ struct AstNodeStmt : public AstNode { : AstNode(fl) {} ASTNODE_BASE_FUNCS(NodeStmt) // METHODS + virtual void addNextStmt(AstNode* newp, AstNode* belowp); // Stop statement searchback here + virtual void addBeforeStmt(AstNode* newp, AstNode* belowp); // Stop statement searchback here }; struct AstNodeAssign : public AstNodeStmt { diff --git a/src/V3AstNodes.cpp b/src/V3AstNodes.cpp index 561a17b41..03900c19f 100644 --- a/src/V3AstNodes.cpp +++ b/src/V3AstNodes.cpp @@ -199,6 +199,68 @@ bool AstSenTree::hasCombo() { return false; } +//====================================================================== +// Special walking tree inserters + +void AstNode::addBeforeStmt(AstNode* newp, AstNode*) { + if (!backp()) newp->v3fatalSrc("Can't find current statement to addBeforeStmt"); + // Look up; virtual call will find where to put it + this->backp()->addBeforeStmt(newp, this); +} +void AstNode::addNextStmt(AstNode* newp, AstNode*) { + if (!backp()) newp->v3fatalSrc("Can't find current statement to addBeforeStmt"); + // Look up; virtual call will find where to put it + this->backp()->addNextStmt(newp, this); +} + +void AstNodeStmt::addBeforeStmt(AstNode* newp, AstNode*) { + // Insert newp before current node + this->addHereThisAsNext(newp); +} +void AstNodeStmt::addNextStmt(AstNode* newp, AstNode*) { + // Insert newp after current node + this->addNextHere(newp); +} + +void AstWhile::addBeforeStmt(AstNode* newp, AstNode* belowp) { + // Special, as statements need to be put in different places + // Belowp is how we came to recurse up to this point + // Preconditions insert first just before themselves (the normal rule for other statement types) + if (belowp == precondsp()) { + // Must have been first statement in precondsp list, so newp is new first statement + belowp->addHereThisAsNext(newp); + } else if (belowp == condp()) { + // Goes before condition, IE in preconditions + addPrecondsp(newp); + } else if (belowp == bodysp()) { + // Was first statement in body, so new front + belowp->addHereThisAsNext(newp); + } else { + belowp->v3fatalSrc("Doesn't look like this was really under the while"); + } +} +void AstWhile::addNextStmt(AstNode* newp, AstNode* belowp) { + // Special, as statements need to be put in different places + // Belowp is how we came to recurse up to this point + // Preconditions insert first just before themselves (the normal rule for other statement types) + if (belowp == precondsp()) { + // Next in precond list + belowp->addNextHere(newp); + } else if (belowp == condp()) { + // Becomes first statement in body, body may have been empty + if (bodysp()) { + bodysp()->addHereThisAsNext(newp); + } else { + addBodysp(newp); + } + } else if (belowp == bodysp()) { + // Next statement in body + belowp->addNextHere(newp); + } else { + belowp->v3fatalSrc("Doesn't look like this was really under the while"); + } +} + //====================================================================== // Per-type Debugging diff --git a/src/V3AstNodes.h b/src/V3AstNodes.h index 88ce3d7fb..ed258df02 100644 --- a/src/V3AstNodes.h +++ b/src/V3AstNodes.h @@ -967,6 +967,14 @@ struct AstAssignW : public AstNodeAssign { : AstNodeAssign(fileline, lhsp, rhsp) { } ASTNODE_NODE_FUNCS(AssignW, ASSIGNW) virtual AstNode* cloneType(AstNode* lhsp, AstNode* rhsp) { return new AstAssignW(this->fileline(), lhsp, rhsp); } + AstAlways* convertToAlways() { + AstNode* lhs1p = lhsp()->unlinkFrBack(); + AstNode* rhs1p = rhsp()->unlinkFrBack(); + AstAlways* newp = new AstAlways (fileline(), NULL, + new AstAssign (fileline(), lhs1p, rhs1p)); + replaceWith(newp); // User expected to then deleteTree(); + return newp; + } }; struct AstPull : public AstNode { @@ -1428,6 +1436,8 @@ struct AstWhile : public AstNodeStmt { virtual int instrCount() const { return instrCountBranch(); } virtual V3Hash sameHash() const { return V3Hash(); } virtual bool same(AstNode* samep) const { return true; } + virtual void addBeforeStmt(AstNode* newp, AstNode* belowp); // Stop statement searchback here + virtual void addNextStmt(AstNode* newp, AstNode* belowp); // Stop statement searchback here }; struct AstGenIf : public AstNodeIf { diff --git a/src/V3Task.cpp b/src/V3Task.cpp index 7cb90bfeb..e5ab4c7e8 100644 --- a/src/V3Task.cpp +++ b/src/V3Task.cpp @@ -191,11 +191,7 @@ private: // Wire assigns must become always statements to deal with insertion // of multiple statements. Perhaps someday make all wassigns into always's? UINFO(5," IM_WireRep "<lhsp()->unlinkFrBack(); - AstNode* rhsp = m_assignwp->rhsp()->unlinkFrBack(); - AstNode* assignp = new AstAssign (m_assignwp->fileline(), lhsp, rhsp); - AstNode* alwaysp = new AstAlways (m_assignwp->fileline(), NULL, assignp); - m_assignwp->replaceWith(alwaysp); pushDeletep(m_assignwp); m_assignwp=NULL; + m_assignwp->convertToAlways(); pushDeletep(m_assignwp); m_assignwp=NULL; } // We make multiple edges if a task is called multiple times from another task. new TaskEdge (&m_callGraph, m_curVxp, getFTaskVertex(nodep->taskp())); @@ -609,16 +605,14 @@ private: m_scopep = oldscopep; } void insertBeforeStmt(AstNode* nodep, AstNode* newp) { + // See also AstNode::addBeforeStmt; this predates that function if (debug()>=9) { nodep->dumpTree(cout,"-newstmt:"); } if (!m_insStmtp) nodep->v3fatalSrc("Function not underneath a statement"); if (m_insMode == IM_BEFORE) { // Add the whole thing before insertAt UINFO(5," IM_Before "<unlinkFrBackWithNext(&handle); if (debug()>=9) { newp->dumpTree(cout,"-newfunc:"); } - newp->addNext(m_insStmtp); - handle.relink(newp); + m_insStmtp->addHereThisAsNext(newp); } else if (m_insMode == IM_AFTER) { UINFO(5," IM_After "< bool. Set true if already processed // AstArraySel::user() -> bool. Set true if already processed + // AstNode::user2p() -> AstIf* Inserted if assignment for conditional AstUser1InUse m_inuser1; + AstUser2InUse m_inuser2; // STATE - AstModule* m_modp; // Current module - bool m_constXCvt; // Convert X's - V3Double0 m_statUnkVars; // Statistic tracking + AstModule* m_modp; // Current module + bool m_constXCvt; // Convert X's + V3Double0 m_statUnkVars; // Statistic tracking + AstAssignW* m_assignwp; // Current assignment + AstAssignDly* m_assigndlyp; // Current assignment // METHODS static int debug() { @@ -79,6 +84,80 @@ private: return nodep; } + void replaceBoundLvalue(AstNode* nodep, AstNode* condp) { + // Spec says a out-of-range LHS SEL results in a NOP. + // This is a PITA. We could: + // 1. IF(...) around an ASSIGN, + // but that would break a "foo[TOO_BIG]=$fopen(...)". + // 2. Hack to extend the size of the output structure + // by one bit, and write to that temporary, but never read it. + // That makes there be two widths() and is likely a bug farm. + // 3. Make a special SEL to choose between the real lvalue + // and a temporary NOP register. + // 4. Assign to a temp, then IF that assignment. + // This is suspected to be nicest to later optimizations. + // 4 seems best but breaks later optimizations. 3 was tried, + // but makes a mess in the emitter as lvalue switching is needed. So 4. + // SEL(...) -> temp + // if (COND(LTE(bit<=maxlsb))) ASSIGN(SEL(...)),temp) + if (m_assignwp) { + // Wire assigns must become always statements to deal with insertion + // of multiple statements. Perhaps someday make all wassigns into always's? + UINFO(5," IM_WireRep "<convertToAlways(); pushDeletep(m_assignwp); m_assignwp=NULL; + } + bool needDly = m_assigndlyp; + if (m_assigndlyp) { + // Delayed assignments become normal assignments, + // then the temp created becomes the delayed assignment + AstNode* newp = new AstAssign(m_assigndlyp->fileline(), + m_assigndlyp->lhsp()->unlinkFrBackWithNext(), + m_assigndlyp->rhsp()->unlinkFrBackWithNext()); + m_assigndlyp->replaceWith(newp); pushDeletep(m_assigndlyp); m_assigndlyp=NULL; + } + AstNode* prep = nodep; + + // Scan back to put the condlvalue above all selects (IE top of the lvalue) + while (prep->backp()->castNodeSel() + || prep->backp()->castSel()) { + prep=prep->backp(); + } + FileLine* fl = nodep->fileline(); + nodep=NULL; // Zap it so we don't use it by mistake - use prep + + // Already exists; rather than IF(a,... IF(b... optimize to IF(a&&b, + // Saves us teaching V3Const how to optimize, and it won't be needed again. + if (AstIf* ifp = prep->user2p()->castNode()->castIf()) { + if (needDly) prep->v3fatalSrc("Should have already converted to non-delay"); + AstNRelinker replaceHandle; + AstNode* earliercondp = ifp->condp()->unlinkFrBack(&replaceHandle); + AstNode* newp = new AstLogAnd (condp->fileline(), + condp, + earliercondp); + UINFO(4, "Edit BOUNDLVALUE "<varNumGetInc())); + AstVar* varp = new AstVar(fl, AstVarType::MODULETEMP, name, + new AstRange(fl, prep->width()-1, 0)); + m_modp->addStmtp(varp); + + AstNode* abovep = prep->backp(); // Grab above point before loose it w/ next replace + prep->replaceWith(new AstVarRef(fl, varp, true)); + AstNode* newp = new AstIf(fl, condp, + (needDly + ? ((new AstAssignDly(fl, prep, + new AstVarRef(fl, varp, false)))->castNode()) + : ((new AstAssign (fl, prep, + new AstVarRef(fl, varp, false)))->castNode())), + NULL); + if (debug()>=9) newp->dumpTree(cout," _new: "); + abovep->addNextStmt(newp,abovep); + prep->user2p(newp); // Save so we may LogAnd it next time + } + } + // VISITORS virtual void visit(AstModule* nodep, AstNUser*) { UINFO(4," MOD "<iterateChildren(*this); m_modp = NULL; } + virtual void visit(AstAssignDly* nodep, AstNUser*) { + m_assigndlyp = nodep; + nodep->iterateChildren(*this); nodep=NULL; // May delete nodep. + m_assigndlyp = NULL; + } + virtual void visit(AstAssignW* nodep, AstNUser*) { + m_assignwp = nodep; + nodep->iterateChildren(*this); nodep=NULL; // May delete nodep. + m_assignwp = NULL; + } virtual void visit(AstCaseItem* nodep, AstNUser*) { m_constXCvt = false; // Avoid loosing the X's in casex nodep->condsp()->iterateAndNext(*this); @@ -292,18 +381,7 @@ private: newp->accept(*this); } else { // lvalue - // SEL(...) -> SEL(COND(LTE(bit<=maxlsb), bit, 0)) - AstNRelinker replaceHandle; - AstNode* lsbp = nodep->lsbp()->unlinkFrBack(&replaceHandle); - V3Number zeronum (nodep->fileline(), lsbp->width(), 0); - AstNode* newp = new AstCondBound (lsbp->fileline(), - condp, - lsbp, - new AstConst(lsbp->fileline(), zeronum)); - if (debug()>=9) newp->dumpTree(cout," _new: "); - replaceHandle.relink(newp); - // Added X's, tristate them too - newp->accept(*this); + replaceBoundLvalue(nodep, condp); } } } @@ -312,6 +390,7 @@ private: nodep->iterateChildren(*this); if (!nodep->user1()) { nodep->user1(1); + if (debug()==9) nodep->dumpTree(cout,"-in: "); // Guard against reading/writing past end of arrays AstNode* basefromp = AstArraySel::baseFromp(nodep->fromp()); int dimension = AstArraySel::dimension(nodep->fromp()); @@ -358,7 +437,7 @@ private: // Added X's, tristate them too newp->accept(*this); } - else { + else if (!lvalue) { // Mid-multidimension read, just use zero // ARRAYSEL(...) -> ARRAYSEL(COND(LT(bitbitp()->unlinkFrBack(&replaceHandle); @@ -372,6 +451,9 @@ private: replaceHandle.relink(newp); newp->accept(*this); } + else { // lvalue + replaceBoundLvalue(nodep, condp); + } } } //-------------------- @@ -383,6 +465,10 @@ private: public: // CONSTUCTORS UnknownVisitor(AstNetlist* nodep) { + m_modp = NULL; + m_assigndlyp = NULL; + m_assignwp = NULL; + m_constXCvt = false; nodep->accept(*this); } virtual ~UnknownVisitor() { diff --git a/test_regress/t/t_select_lhs_oob.pl b/test_regress/t/t_select_lhs_oob.pl new file mode 100644 index 000000000..f91289753 --- /dev/null +++ b/test_regress/t/t_select_lhs_oob.pl @@ -0,0 +1,18 @@ +#!/usr/bin/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. + +compile ( + ); + +execute ( + check_finished=>1, + ); + +ok(1); +1; diff --git a/test_regress/t/t_select_lhs_oob.v b/test_regress/t/t_select_lhs_oob.v new file mode 100644 index 000000000..37b4ba3e7 --- /dev/null +++ b/test_regress/t/t_select_lhs_oob.v @@ -0,0 +1,92 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2009 by Wilson Snyder. + +module t (/*AUTOARG*/ + // Inputs + clk + ); + input clk; + integer cyc=0; + + reg [6:0] mem1d; + reg [6:0] mem2d [5:0]; + reg [6:0] mem3d [4:0][5:0]; + + integer i,j,k; + + // Four different test cases for out of bounds + // = + // <= + // Continuous assigns + // Output pin interconnect (also covers cont assigns) + // Each with both bit selects and array selects + + initial begin + mem1d[0] = 1'b0; + i=7; + mem1d[i] = 1'b1; + if (mem1d[0] !== 1'b0) $stop; + // + for (i=0; i<8; i=i+1) begin + for (j=0; j<8; j=j+1) begin + for (k=0; k<8; k=k+1) begin + mem1d[k] = k[0]; + mem2d[j][k] = j[0]+k[0]; + mem3d[i][j][k] = i[0]+j[0]+k[0]; + end + end + end + for (i=0; i<5; i=i+1) begin + for (j=0; j<6; j=j+1) begin + for (k=0; k<7; k=k+1) begin + if (mem1d[k] !== k[0]) $stop; + if (mem2d[j][k] !== j[0]+k[0]) $stop; + if (mem3d[i][j][k] !== i[0]+j[0]+k[0]) $stop; + end + end + end + end + + integer wi; + wire [31:0] wd = cyc; + reg [31:0] reg2d[6:0]; + always @ (posedge clk) reg2d[wi[2:0]] <= wd; + + always @ (posedge clk) begin +`ifdef TEST_VERBOSE + $write("[%0t] cyc==%0d reg2d[%0d]=%0x wd=%0x\n",$time, cyc, wi[2:0], reg2d[wi[2:0]], wd); +`endif + cyc <= cyc + 1; + if (cyc<10) begin + wi <= 0; + end + else if (cyc==10) begin + wi <= 1; + end + else if (cyc==11) begin + if (reg2d[0] !== 10) $stop; + wi <= 6; + end + else if (cyc==12) begin + if (reg2d[0] !== 10) $stop; + if (reg2d[1] !== 11) $stop; + wi <= 7; // Will be ignored + end + else if (cyc==13) begin + if (reg2d[0] !== 10) $stop; + if (reg2d[1] !== 11) $stop; + if (reg2d[6] !== 12) $stop; + end + else if (cyc==14) begin + if (reg2d[0] !== 10) $stop; + if (reg2d[1] !== 11) $stop; + if (reg2d[6] !== 12) $stop; + end + else if (cyc==99) begin + $write("*-* All Finished *-*\n"); + $finish; + end + end +endmodule diff --git a/test_regress/t/t_select_lhs_oob2.pl b/test_regress/t/t_select_lhs_oob2.pl new file mode 100644 index 000000000..7058e622f --- /dev/null +++ b/test_regress/t/t_select_lhs_oob2.pl @@ -0,0 +1,18 @@ +#!/usr/bin/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. + +compile ( + ); + +execute ( + check_finished=>1, + ); + +ok(1); +1; diff --git a/test_regress/t/t_select_lhs_oob2.v b/test_regress/t/t_select_lhs_oob2.v new file mode 100644 index 000000000..1c33db94d --- /dev/null +++ b/test_regress/t/t_select_lhs_oob2.v @@ -0,0 +1,141 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2009 by Wilson Snyder. + +module t (/*AUTOARG*/ + // Inputs + clk + ); + input clk; + + integer cyc=0; + reg [63:0] crc; + reg [63:0] sum; + + // Take CRC data and apply to testblock inputs + wire [31:0] in = crc[31:0]; + + /*AUTOWIRE*/ + // Beginning of automatic wires (for undeclared instantiated-module outputs) + wire [63:0] out; // From test of Test.v + // End of automatics + + wire reset_l = ~(cyc<15); + wire [63:0] d = crc[63:0]; + wire [8:0] t_wa = crc[8:0]; + wire [8:0] t_addr = {crc[18:17],3'b0,crc[13:10]}; + + Test test (/*AUTOINST*/ + // Outputs + .out (out[63:0]), + // Inputs + .clk (clk), + .reset_l (reset_l), + .t_wa (t_wa[8:0]), + .d (d[63:0]), + .t_addr (t_addr[8:0])); + + // Aggregate outputs into a single result vector + wire [63:0] result = {out}; + + // Test loop + always @ (posedge clk) begin +`ifdef TEST_VERBOSE + $write("[%0t] cyc==%0d crc=%x result=%x\n",$time, cyc, crc, result); +`endif + cyc <= cyc + 1; + crc <= {crc[62:0], crc[63]^crc[2]^crc[0]}; + sum <= result ^ {sum[62:0],sum[63]^sum[2]^sum[0]}; + if (cyc==0) begin + // Setup + crc <= 64'h5aef0c8d_d70a4497; + sum <= 64'h0; + end + else if (cyc<10) begin + sum <= 64'h0; + end + else if (cyc<90) begin + end + else if (cyc==99) begin + $write("[%0t] cyc==%0d crc=%x sum=%x\n",$time, cyc, crc, sum); + if (crc !== 64'hc77bb9b3784ea091) $stop; + // What checksum will we end up with (above print should match) +`define EXPECTED_SUM 64'h421a41d1541ea652 + if (sum !== `EXPECTED_SUM) $stop; + $write("*-* All Finished *-*\n"); + $finish; + end + end + +endmodule + +module Test (/*AUTOARG*/ + // Outputs + out, + // Inputs + clk, reset_l, t_wa, d, t_addr + ); + input clk; + input reset_l; + + reg [63:0] m_w0 [47:0]; + reg [63:0] m_w1 [23:0]; + reg [63:0] m_w2 [23:0]; + reg [63:0] m_w3 [23:0]; + reg [63:0] m_w4 [23:0]; + reg [63:0] m_w5 [23:0]; + + input [8:0] t_wa; + input [63:0] d; + + always @ (posedge clk) begin + if (~reset_l) begin : blk + integer i; + + for (i=0; i<48; i=i+1) begin + m_w0[i] <= 64'h0; + end + + for (i=0; i<24; i=i+1) begin + m_w1[i] <= 64'h0; + m_w2[i] <= 64'h0; + m_w3[i] <= 64'h0; + m_w4[i] <= 64'h0; + m_w5[i] <= 64'h0; + end + end + else begin + casez (t_wa[8:6]) + 3'd0: m_w0[t_wa[5:0]] <= d; + 3'd1: m_w1[t_wa[4:0]] <= d; + 3'd2: m_w2[t_wa[4:0]] <= d; + 3'd3: m_w3[t_wa[4:0]] <= d; + 3'd4: m_w4[t_wa[4:0]] <= d; + default: m_w5[t_wa[4:0]] <= d; + endcase + end + end + + input [8:0] t_addr; + + wire [63:0] t_w0 = m_w0[t_addr[5:0]]; + wire [63:0] t_w1 = m_w1[t_addr[4:0]]; + wire [63:0] t_w2 = m_w2[t_addr[4:0]]; + wire [63:0] t_w3 = m_w3[t_addr[4:0]]; + wire [63:0] t_w4 = m_w4[t_addr[4:0]]; + wire [63:0] t_w5 = m_w5[t_addr[4:0]]; + + output reg [63:0] out; + always @* begin + casez (t_addr[8:6]) + 3'd0: out = t_w0; + 3'd1: out = t_w1; + 3'd2: out = t_w2; + 3'd3: out = t_w3; + 3'd4: out = t_w4; + default: out = t_w5; + endcase + end + +endmodule