Skip to content

Commit ee6d194

Browse files
ivanstepanovftwalexrp
authored andcommitted
spirv: fix signed overflow detection for safe subtraction
The overflow check for safe signed subtraction was using the formula (rhs < 0) == (lhs > result). This logic is flawed and incorrectly reports an overflow when the right-hand side is zero. For the expression 42 - 0, this check evaluated to (0 < 0) == (42 > 42), which is false == false, resulting in true. This caused the generated SPIR-V to incorrectly branch to an OpUnreachable instruction, preventing the result from being stored. Fixes #24281.
1 parent b8ac740 commit ee6d194

File tree

1 file changed

+23
-18
lines changed

1 file changed

+23
-18
lines changed

src/codegen/spirv.zig

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3661,6 +3661,7 @@ const NavGen = struct {
36613661
comptime ucmp: CmpPredicate,
36623662
comptime scmp: CmpPredicate,
36633663
) !?IdRef {
3664+
_ = scmp;
36643665
// Note: OpIAddCarry and OpISubBorrow are not really useful here: For unsigned numbers,
36653666
// there is in both cases only one extra operation required. For signed operations,
36663667
// the overflow bit is set then going from 0x80.. to 0x00.., but this doesn't actually
@@ -3689,26 +3690,30 @@ const NavGen = struct {
36893690
// Overflow happened if the result is smaller than either of the operands. It doesn't matter which.
36903691
// For subtraction the conditions need to be swapped.
36913692
.unsigned => try self.buildCmp(ucmp, result, lhs),
3692-
// For addition, overflow happened if:
3693-
// - rhs is negative and value > lhs
3694-
// - rhs is positive and value < lhs
3695-
// This can be shortened to:
3696-
// (rhs < 0 and value > lhs) or (rhs >= 0 and value <= lhs)
3697-
// = (rhs < 0) == (value > lhs)
3698-
// = (rhs < 0) == (lhs < value)
3699-
// Note that signed overflow is also wrapping in spir-v.
3700-
// For subtraction, overflow happened if:
3701-
// - rhs is negative and value < lhs
3702-
// - rhs is positive and value > lhs
3703-
// This can be shortened to:
3704-
// (rhs < 0 and value < lhs) or (rhs >= 0 and value >= lhs)
3705-
// = (rhs < 0) == (value < lhs)
3706-
// = (rhs < 0) == (lhs > value)
3693+
// For signed operations, we check the signs of the operands and the result.
37073694
.signed => blk: {
3695+
// Signed overflow detection using the sign bits of the operands and the result.
3696+
// For addition (a + b), overflow occurs if the operands have the same sign
3697+
// and the result's sign is different from the operands' sign.
3698+
// (sign(a) == sign(b)) && (sign(a) != sign(result))
3699+
// For subtraction (a - b), overflow occurs if the operands have different signs
3700+
// and the result's sign is different from the minuend's (a's) sign.
3701+
// (sign(a) != sign(b)) && (sign(a) != sign(result))
37083702
const zero = Temporary.init(rhs.ty, try self.constInt(rhs.ty, 0));
3709-
const rhs_lt_zero = try self.buildCmp(.s_lt, rhs, zero);
3710-
const result_gt_lhs = try self.buildCmp(scmp, lhs, result);
3711-
break :blk try self.buildCmp(.l_eq, rhs_lt_zero, result_gt_lhs);
3703+
3704+
const lhs_is_neg = try self.buildCmp(.s_lt, lhs, zero);
3705+
const rhs_is_neg = try self.buildCmp(.s_lt, rhs, zero);
3706+
const result_is_neg = try self.buildCmp(.s_lt, result, zero);
3707+
3708+
const signs_match = try self.buildCmp(.l_eq, lhs_is_neg, rhs_is_neg);
3709+
const result_sign_differs = try self.buildCmp(.l_ne, lhs_is_neg, result_is_neg);
3710+
3711+
const overflow_condition = if (add == .i_add)
3712+
signs_match
3713+
else // .i_sub
3714+
try self.buildUnary(.l_not, signs_match);
3715+
3716+
break :blk try self.buildBinary(.l_and, overflow_condition, result_sign_differs);
37123717
},
37133718
};
37143719

0 commit comments

Comments
 (0)