luke/verilator
1
0
Fork 0
mirror of https://github.com/verilator/verilator.git synced 2026-10-02 16:13:34 +02:00
Code Issues Packages Projects Releases Wiki Activity

Fix constant oversized shifts in V3Premit

Browse Source 
- Arithmetic right shift used the MSB of the C++ type as the sign bit,
  instead of the MSB at the Verilog width.
- Constant shift amounts with their top bit set were treated as
  negative, so the oversized shift was emitted as undefined behavior.
- Shifts of an impure operand were replaced by zero, discarding the
  side effects of the operand.
This commit is contained in:
Geza Lore
2026-09-25 14:12:00 +01:00
parent f2dbf210a8
commit 84dec259c8
4 changed files with 88 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/V3Premit.cpp
+38 -24
View File
@@ -138,6 +138,22 @@ class PremitVisitor final : public VNVisitor {
return varp;
}
void replaceWithOvrShift(AstNodeBiop* nodep) {
FileLine* const flp = nodep->fileline();
AstNodeExpr* const lhsp = nodep->lhsp()->unlinkFrBack();
AstNodeExpr* const rhsp = nodep->rhsp()->unlinkFrBack();
AstNodeExpr* newp = nullptr;
if (VN_IS(nodep, ShiftL)) {
newp = new AstShiftLOvr{flp, lhsp, rhsp};
} else if (VN_IS(nodep, ShiftR)) {
newp = new AstShiftROvr{flp, lhsp, rhsp};
} else {
newp = new AstShiftRSOvr{flp, lhsp, rhsp};
}
nodep->replaceWithKeepDType(newp);
VL_DO_DANGLING(pushDeletep(nodep), nodep);
}
void visitShift(AstNodeBiop* nodep) {
UINFO(4, " ShiftFix " << nodep);
UASSERT_OBJ(VN_IS(nodep, ShiftL) || VN_IS(nodep, ShiftR) || VN_IS(nodep, ShiftRS), nodep,
@@ -150,39 +166,37 @@ class PremitVisitor final : public VNVisitor {
// Shift amount known to be constant. If oversized shift, replace with zero/msbs.
// Otherwise we can leave the original shifts which have better constant folding
// than the *Ovr versions.
const bool isOversized = shiftp->num().mostSetBitP1() > 32 //
|| (shiftp->num().toSQuad() >= nodep->width());
const bool isOversized
= shiftp->num().mostSetBitP1() > 32
|| (shiftp->num().toUInt() >= static_cast<uint32_t>(nodep->width()));
if (isOversized) {
AstNodeExpr* newp = nullptr;
// If signed shift, replace with replicated MSB
if (VN_IS(nodep, ShiftRS)) {
AstNodeExpr* const lhsp = nodep->lhsp()->unlinkFrBack();
AstNodeExpr* const msbp = new AstSel{flp, lhsp, nodep->width() - 1, 1};
newp = new AstExtendS{flp, msbp, nodep->width()};
} else {
newp = new AstConst{flp, AstConst::DTyped{}, nodep->dtypep()};
AstNodeExpr* const msbp = new AstSel{flp, lhsp, lhsp->widthMin() - 1, 1};
nodep->replaceWithKeepDType(new AstExtendS{flp, msbp, nodep->width()});
VL_DO_DANGLING(pushDeletep(nodep), nodep);
return;
}
// Unsigned. If pure, replace with zero
if (nodep->lhsp()->isPure()) {
nodep->replaceWithKeepDType(
new AstConst{flp, AstConst::DTyped{}, nodep->dtypep()});
VL_DO_DANGLING(pushDeletep(nodep), nodep);
return;
}
// Impure. Keep shift
if (!nodep->isWide()) {
replaceWithOvrShift(nodep);
return;
}
nodep->replaceWithKeepDType(newp);
VL_DO_DANGLING(pushDeletep(nodep), nodep);
return;
}
} else {
// Shift amount not known at compile time. Convert to *Ovr version. Don't need to do
// if it would use a wide operation which works correctly at runtime, of if the max
// value of the shift amount is less than the with of the shifted value.
if (nodep->widthMin() <= VL_QUADSIZE
&& (nodep->width() < (1LL << nodep->rhsp()->widthMin()))) {
AstNodeExpr* const lhsp = nodep->lhsp()->unlinkFrBack();
AstNodeExpr* const rhsp = nodep->rhsp()->unlinkFrBack();
AstNodeExpr* newp = nullptr;
if (VN_IS(nodep, ShiftL)) {
newp = new AstShiftLOvr{flp, lhsp, rhsp};
} else if (VN_IS(nodep, ShiftR)) {
newp = new AstShiftROvr{flp, lhsp, rhsp};
} else {
newp = new AstShiftRSOvr{flp, lhsp, rhsp};
}
nodep->replaceWithKeepDType(newp);
VL_DO_DANGLING(pushDeletep(nodep), nodep);
if (!nodep->isWide() && (nodep->width() < (1LL << nodep->rhsp()->widthMin()))) {
replaceWithOvrShift(nodep);
return;
}
}
test_regress/t/t_math_shift.v
+11
View File
@@ -115,6 +115,17 @@ module t (/*AUTOARG*/
c_wleft_32 = rand_96 << 32;
end
// Constant shift amount with its top bit set is still unsigned.
// Operand is impure so the oversized shift is not folded before V3Premit.
logic [7:0] bq[$] = '{8'h5a};
int iq[$] = '{32'h819b018a};
longint qq[$] = '{64'hf784bf8f_12734089};
initial begin
if ((bq.pop_front() << 7'd100) != 8'h0) $stop;
if ((iq.pop_front() >> 7'd100) != 32'h0) $stop;
if ((qq.pop_front() >> 8'd200) != 64'h0) $stop;
end
integer cyc; initial cyc=1;
always @ (posedge clk) begin
if (cyc!=0) begin
test_regress/t/t_math_shift_side.v
+16
View File
@@ -16,6 +16,9 @@ endclass
module t;
int i;
longint q;
int iq[$];
longint qq[$];
initial begin
Cls c;
@@ -33,6 +36,19 @@ module t;
i = 32'shffffffff >>> c.get_n_bytes();
if (i != 32'hffffffff) $stop;
// Oversized constant shift must still evaluate the shifted operand
iq = '{1, 2, 3};
i = iq.pop_front() << 8'd40;
if (i != 0) $stop;
if (iq.size() != 2) $stop;
i = iq.pop_front() >> 8'd40;
if (i != 0) $stop;
if (iq.size() != 1) $stop;
qq = '{1, 2};
q = qq.pop_front() << 8'd70;
if (q != 0) $stop;
if (qq.size() != 1) $stop;
$write("*-* All Finished *-*\n");
$finish;
end
test_regress/t/t_math_shiftrs2.v
+23 -2
View File
@@ -6,7 +6,7 @@
// verilog_format: off
`define stop $stop
`define checkd(gotv, expv) do if ((gotv) !== (expv)) begin $write("%%Error: %s:%0d: got=%0d exp=%0d\n", `__FILE__, `__LINE__, (gotv), (expv)); `stop; end while(0);
`define checkh(gotv, expv) do if ((gotv) !== (expv)) begin $write("%%Error: %s:%0d: got='h%x exp='h%x\n", `__FILE__, `__LINE__, (gotv), (expv)); `stop; end while(0);
// verilog_format: on
module top (
@@ -17,9 +17,30 @@ module top (
assign wire_4 = 3'b011;
assign out35 = (wire_4 >>> 36'hffff_ffff_f);
// Constant shift >= the width must fill with the sign bit
logic signed [6:0] n7;
logic signed [71:0] n72;
wire signed [6:0] n7_32 = n7 >>> 32;
wire signed [6:0] n7_100 = n7 >>> 100;
wire signed [71:0] n72_100 = n72 >>> 100;
wire signed [71:0] n72_200 = n72 >>> 200;
initial begin
n7 = -7'sd3;
n72 = -72'sd3;
#10;
`checkd(out35, '0);
`checkh(out35, '0);
`checkh(n7_32, 7'h7f);
`checkh(n7_100, 7'h7f);
`checkh(n72_100, 72'hff_ffffffff_ffffffff);
`checkh(n72_200, 72'hff_ffffffff_ffffffff);
n7 = 7'sd3;
n72 = 72'sd3;
#10;
`checkh(n7_32, 7'h00);
`checkh(n7_100, 7'h00);
`checkh(n72_100, 72'h0);
`checkh(n72_200, 72'h0);
$write("*-* All Finished *-*\n");
$finish;
end