From a6f4dd031f50387ae0169490c6d8843b91dd1c07 Mon Sep 17 00:00:00 2001 From: spomatasmd <152479765+spomatasmd@users.noreply.github.com> Date: Tue, 28 Jul 2026 11:42:51 +0200 Subject: [PATCH] Fix DfgPeephole miscompile of nested shifts with overflowing shift amount (#7977) The REPLACE_SHIFTL_SHIFTL and REPLACE_SHIFTR_SHIFTR peephole optimizations fold '(a << b) << c' into 'a << (b + c)' (and likewise for '>>'), but computed 'b + c' in the width of the shift-amount operand. When 'b + c' overflows that width it wraps around, producing a too-small shift amount and a wrong result. Compute the sum one bit wider than the amounts so it cannot overflow. Fixes #7955 --- docs/CONTRIBUTORS | 1 + src/V3DfgPeephole.cpp | 16 ++++++++++++++-- test_regress/t/t_dfg_peephole.v | 7 +++++++ 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/docs/CONTRIBUTORS b/docs/CONTRIBUTORS index f7881b717..3cdf22e39 100644 --- a/docs/CONTRIBUTORS +++ b/docs/CONTRIBUTORS @@ -345,3 +345,4 @@ Zubin Jain Muzaffer Kal Yilin Li Shashvat Prabhu +spomatasmd diff --git a/src/V3DfgPeephole.cpp b/src/V3DfgPeephole.cpp index fa29339fe..159f7539c 100644 --- a/src/V3DfgPeephole.cpp +++ b/src/V3DfgPeephole.cpp @@ -2548,7 +2548,13 @@ class V3DfgPeephole final : public DfgVisitor { if (DfgShiftL* const lShiftLp = lhsp->cast()) { if (!lShiftLp->hasMultipleSinks() && rhsp->dtype() == lShiftLp->rhsp()->dtype()) { APPLYING(REPLACE_SHIFTL_SHIFTL) { - DfgAdd* const addp = make(rhsp, rhsp, lShiftLp->rhsp()); + // Fold '(a << b) << c' to 'a << (b + c)'. Compute 'b + c' + // one bit wider than the amounts so it cannot overflow. + FileLine* const flp = vtxp->fileline(); + const DfgDataType& sumDType = DfgDataType::packed(rhsp->width() + 1); + DfgVertex* const bp = make(flp, sumDType, lShiftLp->rhsp()); + DfgVertex* const cp = make(flp, sumDType, rhsp); + DfgAdd* const addp = make(flp, sumDType, bp, cp); replace(make(vtxp, lShiftLp->lhsp(), addp)); return; } @@ -2637,7 +2643,13 @@ class V3DfgPeephole final : public DfgVisitor { if (DfgShiftR* const lShiftRp = lhsp->cast()) { if (!lShiftRp->hasMultipleSinks() && rhsp->dtype() == lShiftRp->rhsp()->dtype()) { APPLYING(REPLACE_SHIFTR_SHIFTR) { - DfgAdd* const addp = make(rhsp, rhsp, lShiftRp->rhsp()); + // Fold '(a >> b) >> c' to 'a >> (b + c)'. Compute 'b + c' + // one bit wider than the amounts so it cannot overflow. + FileLine* const flp = vtxp->fileline(); + const DfgDataType& sumDType = DfgDataType::packed(rhsp->width() + 1); + DfgVertex* const bp = make(flp, sumDType, lShiftRp->rhsp()); + DfgVertex* const cp = make(flp, sumDType, rhsp); + DfgAdd* const addp = make(flp, sumDType, bp, cp); replace(make(vtxp, lShiftRp->lhsp(), addp)); return; } diff --git a/test_regress/t/t_dfg_peephole.v b/test_regress/t/t_dfg_peephole.v index 3e393a297..1a5237723 100644 --- a/test_regress/t/t_dfg_peephole.v +++ b/test_regress/t/t_dfg_peephole.v @@ -341,6 +341,13 @@ module t ( `signal(REPLACE_BITWISE_OF_REDUCTION_OF_SELS_WITH_REDUCTION_XOR_B, (^rand_a[12 +:2]) ^ (^rand_a[10 +: 2])); `signal(REPLACE_SHIFTL_SHIFTL, rand_a << 2 << 3); `signal(REPLACE_SHIFTR_SHIFTR, rand_a >> 2 >> 3); + // As above, but with 2-bit variable amounts whose sum can overflow the amount width (#7955). + wire [1:0] shl_shl_amt0 = rand_a[1:0]; + wire [1:0] shl_shl_amt1 = rand_b[1:0]; + `signal(REPLACE_SHIFTL_SHIFTL_OVERFLOW, (rand_a << shl_shl_amt0) << shl_shl_amt1); + wire [1:0] shr_shr_amt0 = rand_a[3:2]; + wire [1:0] shr_shr_amt1 = rand_b[3:2]; + `signal(REPLACE_SHIFTR_SHIFTR_OVERFLOW, (rand_a >> shr_shr_amt0) >> shr_shr_amt1); `signal(PUSH_COMMUTATIVE_BINARY_THROUGH_COND, 58'h1 + (rand_a[0] ? rand_b[1 +: 58] : ~rand_b[1 +: 58])); `signal(REMOVE_ADD_ZERO, rand_a + '0); `signal(REPLACE_ADD_WITH_COUNT_ONES_A, 4'(rand_a[63]) + 4'(rand_a[62]) + 4'(rand_a[61]));