From 43cf5693d1f772a9d33c9d46ef77caf9e408ac17 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Sun, 14 Feb 2021 11:15:12 -0500 Subject: [PATCH] Fix shifts by > 32 bit values (#2785). --- Changes | 2 + include/verilated.h | 33 +++++- src/V3Ast.h | 2 +- src/V3AstNodes.h | 8 +- test_regress/t/t_math_shift.v | 207 ++++++++++++++++++++++++++-------- 5 files changed, 197 insertions(+), 55 deletions(-) diff --git a/Changes b/Changes index 6471e8a93..b4dff84d3 100644 --- a/Changes +++ b/Changes @@ -11,6 +11,8 @@ The contributors that suggested a given feature are shown in []. Thanks! **** Fix class extends with VM_PARALLEL_BUILDS (#2775). [Iru Cai] +**** Fix shifts by > 32 bit values (#2785). [qrq992] + **** Fix examples not flushing vcd (#2787). [Richard E George] diff --git a/include/verilated.h b/include/verilated.h index 6abb68521..2188f124b 100644 --- a/include/verilated.h +++ b/include/verilated.h @@ -2145,6 +2145,12 @@ static inline WDataOutP VL_SHIFTL_WWW(int obits, int lbits, int rbits, WDataOutP } return VL_SHIFTL_WWI(obits, lbits, 32, owp, lwp, rwp[0]); } +static inline WDataOutP VL_SHIFTL_WWQ(int obits, int lbits, int rbits, WDataOutP owp, WDataInP lwp, + QData rd) VL_MT_SAFE { + WData rwp[VL_WQ_WORDS_E]; + VL_SET_WQ(rwp, rd); + return VL_SHIFTL_WWW(obits, lbits, rbits, owp, lwp, rwp); +} static inline IData VL_SHIFTL_IIW(int obits, int, int rbits, IData lhs, WDataInP rwp) VL_MT_SAFE { for (int i = 1; i < VL_WORDS_I(rbits); ++i) { if (VL_UNLIKELY(rwp[i])) { // Huge shift 1>>32 or more @@ -2153,6 +2159,11 @@ static inline IData VL_SHIFTL_IIW(int obits, int, int rbits, IData lhs, WDataInP } return VL_CLEAN_II(obits, obits, lhs << rwp[0]); } +static inline IData VL_SHIFTL_IIQ(int obits, int lbits, int rbits, IData lhs, + QData rhs) VL_MT_SAFE { + if (VL_UNLIKELY(rhs >= VL_IDATASIZE)) return 0; + return VL_CLEAN_II(obits, obits, lhs << rhs); +} static inline QData VL_SHIFTL_QQW(int obits, int, int rbits, QData lhs, WDataInP rwp) VL_MT_SAFE { for (int i = 1; i < VL_WORDS_I(rbits); ++i) { if (VL_UNLIKELY(rwp[i])) { // Huge shift 1>>32 or more @@ -2162,6 +2173,11 @@ static inline QData VL_SHIFTL_QQW(int obits, int, int rbits, QData lhs, WDataInP // Above checks rwp[1]==0 so not needed in below shift return VL_CLEAN_QQ(obits, obits, lhs << (static_cast(rwp[0]))); } +static inline QData VL_SHIFTL_QQQ(int obits, int lbits, int rbits, QData lhs, + QData rhs) VL_MT_SAFE { + if (VL_UNLIKELY(rhs >= VL_QUADSIZE)) return 0; + return VL_CLEAN_QQ(obits, obits, lhs << rhs); +} // EMIT_RULE: VL_SHIFTR: oclean=lclean; rclean==clean; // Important: Unlike most other funcs, the shift might well be a computed @@ -2223,6 +2239,14 @@ static inline QData VL_SHIFTR_QQW(int obits, int, int rbits, QData lhs, WDataInP // Above checks rwp[1]==0 so not needed in below shift return VL_CLEAN_QQ(obits, obits, lhs >> (static_cast(rwp[0]))); } +static inline IData VL_SHIFTR_IIQ(int obits, int, int rbits, IData lhs, QData rhs) VL_MT_SAFE { + if (VL_UNLIKELY(rhs >= VL_IDATASIZE)) return 0; + return VL_CLEAN_QQ(obits, obits, lhs >> rhs); +} +static inline QData VL_SHIFTR_QQQ(int obits, int, int rbits, QData lhs, QData rhs) VL_MT_SAFE { + if (VL_UNLIKELY(rhs >= VL_QUADSIZE)) return 0; + return VL_CLEAN_QQ(obits, obits, lhs >> rhs); +} // EMIT_RULE: VL_SHIFTRS: oclean=false; lclean=clean, rclean==clean; static inline IData VL_SHIFTRS_III(int obits, int lbits, int, IData lhs, IData rhs) VL_PURE { @@ -2277,7 +2301,7 @@ static inline WDataOutP VL_SHIFTRS_WWW(int obits, int lbits, int rbits, WDataOut WDataInP lwp, WDataInP rwp) VL_MT_SAFE { EData overshift = 0; // Huge shift 1>>32 or more for (int i = 1; i < VL_WORDS_I(rbits); ++i) overshift |= rwp[i]; - if (VL_UNLIKELY(overshift)) { + if (VL_UNLIKELY(overshift || rwp[0] >= obits)) { int lmsw = VL_WORDS_I(obits) - 1; EData sign = VL_SIGNONES_E(lbits, lwp[lmsw]); for (int j = 0; j <= lmsw; ++j) owp[j] = sign; @@ -2296,7 +2320,7 @@ static inline IData VL_SHIFTRS_IIW(int obits, int lbits, int rbits, IData lhs, WDataInP rwp) VL_MT_SAFE { EData overshift = 0; // Huge shift 1>>32 or more for (int i = 1; i < VL_WORDS_I(rbits); ++i) overshift |= rwp[i]; - if (VL_UNLIKELY(overshift)) { + if (VL_UNLIKELY(overshift || rwp[0] >= obits)) { IData sign = -(lhs >> (lbits - 1)); // ffff_ffff if negative return VL_CLEAN_II(obits, obits, sign); } @@ -2306,13 +2330,14 @@ static inline QData VL_SHIFTRS_QQW(int obits, int lbits, int rbits, QData lhs, WDataInP rwp) VL_MT_SAFE { EData overshift = 0; // Huge shift 1>>32 or more for (int i = 1; i < VL_WORDS_I(rbits); ++i) overshift |= rwp[i]; - if (VL_UNLIKELY(overshift)) { + if (VL_UNLIKELY(overshift || rwp[0] >= obits)) { QData sign = -(lhs >> (lbits - 1)); // ffff_ffff if negative return VL_CLEAN_QQ(obits, obits, sign); } return VL_SHIFTRS_QQI(obits, lbits, 32, lhs, rwp[0]); } -static inline IData VL_SHIFTRS_IIQ(int obits, int lbits, int rbits, IData lhs, QData rhs) VL_PURE { +static inline IData VL_SHIFTRS_IIQ(int obits, int lbits, int rbits, IData lhs, + QData rhs) VL_MT_SAFE { WData rwp[VL_WQ_WORDS_E]; VL_SET_WQ(rwp, rhs); return VL_SHIFTRS_IIW(obits, lbits, rbits, lhs, rwp); diff --git a/src/V3Ast.h b/src/V3Ast.h index 2295ff743..1718191e5 100644 --- a/src/V3Ast.h +++ b/src/V3Ast.h @@ -1922,7 +1922,7 @@ public: virtual string emitVerilog() = 0; /// Format string for verilog writing; see V3EmitV // For documentation on emitC format see EmitCStmts::emitOpName virtual string emitC() = 0; - virtual string emitSimpleOperator() { return ""; } + virtual string emitSimpleOperator() { return ""; } // "" means not ok to use virtual bool emitCheckMaxWords() { return false; } // Check VL_MULS_MAX_WORDS virtual bool cleanOut() const = 0; // True if output has extra upper bits zero // Someday we will generically support data types on every math node diff --git a/src/V3AstNodes.h b/src/V3AstNodes.h index feebe0714..eacbdbf8b 100644 --- a/src/V3AstNodes.h +++ b/src/V3AstNodes.h @@ -7305,7 +7305,9 @@ public: } virtual string emitVerilog() override { return "%k(%l %f<< %r)"; } virtual string emitC() override { return "VL_SHIFTL_%nq%lq%rq(%nw,%lw,%rw, %P, %li, %ri)"; } - virtual string emitSimpleOperator() override { return "<<"; } + virtual string emitSimpleOperator() override { + return (rhsp()->isWide() || rhsp()->isQuad()) ? "" : "<<"; + } virtual bool cleanOut() const override { return false; } virtual bool cleanLhs() const override { return false; } virtual bool cleanRhs() const override { return true; } @@ -7327,7 +7329,9 @@ public: } virtual string emitVerilog() override { return "%k(%l %f>> %r)"; } virtual string emitC() override { return "VL_SHIFTR_%nq%lq%rq(%nw,%lw,%rw, %P, %li, %ri)"; } - virtual string emitSimpleOperator() override { return ">>"; } + virtual string emitSimpleOperator() override { + return (rhsp()->isWide() || rhsp()->isQuad()) ? "" : ">>"; + } virtual bool cleanOut() const override { return false; } virtual bool cleanLhs() const override { return true; } virtual bool cleanRhs() const override { return true; } diff --git a/test_regress/t/t_math_shift.v b/test_regress/t/t_math_shift.v index cd071be78..e8f9379c4 100644 --- a/test_regress/t/t_math_shift.v +++ b/test_regress/t/t_math_shift.v @@ -24,30 +24,86 @@ module t (/*AUTOARG*/ localparam [3:0] PBIG29 = 4'b1 << 33'h100000000; // verilator lint_on WIDTH - reg [31:0] right; - reg [31:0] left; + reg [31:0] iright; + reg signed [31:0] irights; + reg [31:0] ileft; reg [P64-1:0] qright; + reg signed [P64-1:0] qrights; reg [P64-1:0] qleft; - reg [31:0] amt; + reg [95:0] wright; + reg signed [95:0] wrights; + reg [95:0] wleft; + + reg [31:0] q_iright; + reg signed [31:0] q_irights; + reg [31:0] q_ileft; + reg [P64-1:0] q_qright; + reg signed [P64-1:0] q_qrights; + reg [P64-1:0] q_qleft; + reg [95:0] q_wright; + reg signed [95:0] q_wrights; + reg [95:0] q_wleft; + + + reg [31:0] w_iright; + reg signed [31:0] w_irights; + reg [31:0] w_ileft; + reg [P64-1:0] w_qright; + reg signed [P64-1:0] w_qrights; + reg [P64-1:0] w_qleft; + reg [95:0] w_wright; + reg signed [95:0] w_wrights; + reg [95:0] w_wleft; + + reg [31:0] iamt; + reg [63:0] qamt; + reg [95:0] wamt; assign ign = {31'h0, clk} >>> 4'bx; // bug760 - assign ign2 = {amt[1:0] >> {22{amt[5:2]}}, amt[1:0] << (0 <<< amt[5:2])}; // bug1174 - assign ign3 = {amt[1:0] >> {22{amt[5:2]}}, - amt[1:0] >> {11{amt[5:2]}}, - $signed(amt[1:0]) >>> {22{amt[5:2]}}, - $signed(amt[1:0]) >>> {11{amt[5:2]}}, - amt[1:0] << {22{amt[5:2]}}, - amt[1:0] << {11{amt[5:2]}}}; + assign ign2 = {iamt[1:0] >> {22{iamt[5:2]}}, iamt[1:0] << (0 <<< iamt[5:2])}; // bug1174 + assign ign3 = {iamt[1:0] >> {22{iamt[5:2]}}, + iamt[1:0] >> {11{iamt[5:2]}}, + $signed(iamt[1:0]) >>> {22{iamt[5:2]}}, + $signed(iamt[1:0]) >>> {11{iamt[5:2]}}, + iamt[1:0] << {22{iamt[5:2]}}, + iamt[1:0] << {11{iamt[5:2]}}}; - wire [95:0] wamt = {amt,amt,amt}; - output wire [95:0] ign4 = wamt >> {11{amt[5:2]}}; - output wire signed [95:0] ign4s = $signed(wamt) >>> {11{amt[5:2]}}; + wire [95:0] wamtt = {iamt,iamt,iamt}; + output wire [95:0] ign4; + assign ign4 = wamtt >> {11{iamt[5:2]}}; + output wire signed [95:0] ign4s; + assign ign4s = $signed(wamtt) >>> {11{iamt[5:2]}}; always @* begin - right = 32'h819b018a >> amt; - left = 32'h819b018a << amt; - qright = 64'hf784bf8f_12734089 >> amt; - qleft = 64'hf784bf8f_12734089 >> amt; + iright = 32'h819b018a >> iamt; + irights = 32'sh819b018a >>> signed'(iamt); + ileft = 32'h819b018a << iamt; + qright = 64'hf784bf8f_12734089 >> iamt; + qrights = 64'shf784bf8f_12734089 >>> signed'(iamt); + qleft = 64'hf784bf8f_12734089 << iamt; + wright = 96'hf784bf8f_12734089_190abe48 >> iamt; + wrights = 96'shf784bf8f_12734089_190abe48 >>> signed'(iamt); + wleft = 96'hf784bf8f_12734089_190abe48 << iamt; + + q_iright = 32'h819b018a >> qamt; + q_irights = 32'sh819b018a >>> signed'(qamt); + q_ileft = 32'h819b018a << qamt; + q_qright = 64'hf784bf8f_12734089 >> qamt; + q_qrights = 64'shf784bf8f_12734089 >>> signed'(qamt); + q_qleft = 64'hf784bf8f_12734089 << qamt; + q_wright = 96'hf784bf8f_12734089_190abe48 >> qamt; + q_wrights = 96'shf784bf8f_12734089_190abe48 >>> signed'(qamt); + q_wleft = 96'hf784bf8f_12734089_190abe48 << qamt; + + w_iright = 32'h819b018a >> wamt; + w_irights = 32'sh819b018a >>> signed'(wamt); + w_ileft = 32'h819b018a << wamt; + w_qright = 64'hf784bf8f_12734089 >> wamt; + w_qrights = 64'shf784bf8f_12734089 >>> signed'(wamt); + w_qleft = 64'hf784bf8f_12734089 << wamt; + w_wright = 96'hf784bf8f_12734089_190abe48 >> wamt; + w_wrights = 96'shf784bf8f_12734089_190abe48 >>> signed'(wamt); + w_wleft = 96'hf784bf8f_12734089_190abe48 << wamt; end integer cyc; initial cyc=1; @@ -55,10 +111,12 @@ module t (/*AUTOARG*/ if (cyc!=0) begin cyc <= cyc + 1; `ifdef TEST_VERBOSE - $write("%d %x %x %x %x\n", cyc, left, right, qleft, qright); + $write("%d %x %x %x %x %x %x\n", cyc, ileft, iright, qleft, qright, wleft, wright); `endif if (cyc==1) begin - amt <= 32'd0; + iamt <= 0; + qamt <= 0; + wamt <= 0; if (P64 != 64) $stop; if (5'b10110>>2 != 5'b00101) $stop; if (5'b10110>>>2 != 5'b00101) $stop; // Note it cares about sign-ness @@ -72,57 +130,110 @@ module t (/*AUTOARG*/ if ((64'sh458c2de282e30f8b >> 68'sh4) !== 64'sh0458c2de282e30f8) $stop; end if (cyc==2) begin - amt <= 32'd28; - if (left != 32'h819b018a) $stop; - if (right != 32'h819b018a) $stop; - if (qleft != 64'hf784bf8f_12734089) $stop; - if (qright != 64'hf784bf8f_12734089) $stop; + iamt <= 28; + qamt <= 28; + wamt <= 28; + if (ileft != 32'h819b018a) $stop; + if (iright != 32'h819b018a) $stop; + if (irights != 32'h819b018a) $stop; + if (qleft != 64'hf784bf8f_12734089) $stop; + if (qright != 64'hf784bf8f_12734089) $stop; + if (qrights != 64'hf784bf8f_12734089) $stop; + if (wleft != 96'hf784bf8f12734089190abe48) $stop; + if (wright != 96'hf784bf8f12734089190abe48) $stop; + if (wrights != 96'hf784bf8f12734089190abe48) $stop; end if (cyc==3) begin - amt <= 32'd31; - if (left != 32'ha0000000) $stop; - if (right != 32'h8) $stop; - if (qleft != 64'h0000000f784bf8f1) $stop; + iamt <= 31; + qamt <= 31; + wamt <= 31; + if (ileft != 32'ha0000000) $stop; + if (iright != 32'h8) $stop; + if (irights != 32'hfffffff8) $stop; + if (qleft != 64'hf127340890000000) $stop; if (qright != 64'h0000000f784bf8f1) $stop; + if (qrights != 64'hffffffff784bf8f1) $stop; + if (wleft != 96'hf12734089190abe480000000) $stop; + if (wright != 96'h0000000f784bf8f127340891) $stop; + if (wrights != 96'hffffffff784bf8f127340891) $stop; end if (cyc==4) begin - amt <= 32'd32; - if (left != 32'h0) $stop; - if (right != 32'h1) $stop; - if (qleft != 64'h00000001ef097f1e) $stop; + iamt <= 32; + qamt <= 32; + wamt <= 32; + if (ileft != 32'h0) $stop; + if (iright != 32'h1) $stop; + if (qleft != 64'h8939a04480000000) $stop; if (qright != 64'h00000001ef097f1e) $stop; end if (cyc==5) begin - amt <= 32'd33; - if (left != 32'h0) $stop; - if (right != 32'h0) $stop; - if (qleft != 64'h00000000f784bf8f) $stop; + iamt <= 33; + qamt <= 33; + wamt <= 33; + if (ileft != 32'h0) $stop; + if (iright != 32'h0) $stop; + if (qleft != 64'h1273408900000000) $stop; if (qright != 64'h00000000f784bf8f) $stop; end if (cyc==6) begin - amt <= 32'd64; - if (left != 32'h0) $stop; - if (right != 32'h0) $stop; - if (qleft != 64'h000000007bc25fc7) $stop; + iamt <= 64; + qamt <= 64; + wamt <= 64; + if (ileft != 32'h0) $stop; + if (iright != 32'h0) $stop; + if (qleft != 64'h24e6811200000000) $stop; if (qright != 64'h000000007bc25fc7) $stop; end if (cyc==7) begin - amt <= 32'd128; - if (left != 32'h0) $stop; - if (right != 32'h0) $stop; + iamt <= 128; + qamt <= 128; + wamt <= 128; + if (ileft != 32'h0) $stop; + if (iright != 32'h0) $stop; if (qleft != 64'h0) $stop; if (qright != 64'h0) $stop; end if (cyc==8) begin - if (left != 32'h0) $stop; - if (right != 32'h0) $stop; - if (qleft != 64'h0) $stop; - if (qright != 64'h0) $stop; + iamt <= 100; + qamt <= {32'h10, 32'h0}; + wamt <= {32'h10, 64'h0}; + if (ileft != '0) $stop; + if (iright != '0) $stop; + if (irights != '1) $stop; + if (qleft != '0) $stop; + if (qright != '0) $stop; + if (qrights != '1) $stop; + if (wleft != '0) $stop; + if (wright != '0) $stop; + if (wrights != '1) $stop; end - if (cyc==9) begin + if (cyc==19) begin $write("*-* All Finished *-*\n"); $finish; end + + // General rule to test all q's + if (cyc != 0) begin + if (ileft != q_ileft) $stop; + if (iright != q_iright) $stop; + if (irights != q_irights) $stop; + if (qleft != q_qleft) $stop; + if (qright != q_qright) $stop; + if (qrights != q_qrights) $stop; + if (wleft != q_wleft) $stop; + if (wright != q_wright) $stop; + if (wrights != q_wrights) $stop; + + if (ileft != w_ileft) $stop; + if (iright != w_iright) $stop; + if (irights != w_irights) $stop; + if (qleft != w_qleft) $stop; + if (qright != w_qright) $stop; + if (qrights != w_qrights) $stop; + if (wleft != w_wleft) $stop; + if (wright != w_wright) $stop; + if (wrights != w_wrights) $stop; + end end end endmodule