diff options
| author | Ivan Stepanov <ivanstepanovftw@gmail.com> | 2025-07-01 14:25:04 +0200 |
|---|---|---|
| committer | Alex Rønne Petersen <alex@alexrp.com> | 2025-07-01 19:26:21 +0200 |
| commit | ee6d19480da53b3a351749e2a7b18c45ad072018 (patch) | |
| tree | 22309b2a5cb5f383d0892c2da8908876341fdb9d /src/codegen/spirv.zig | |
| parent | b8ac740a1b072168c24785cf8d1c8315ed6db8b0 (diff) | |
| download | zig-ee6d19480da53b3a351749e2a7b18c45ad072018.tar.gz zig-ee6d19480da53b3a351749e2a7b18c45ad072018.zip | |
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.
Diffstat (limited to 'src/codegen/spirv.zig')
| -rw-r--r-- | src/codegen/spirv.zig | 41 |
1 files changed, 23 insertions, 18 deletions
diff --git a/src/codegen/spirv.zig b/src/codegen/spirv.zig index 858c04d53d..8a782c54aa 100644 --- a/src/codegen/spirv.zig +++ b/src/codegen/spirv.zig @@ -3661,6 +3661,7 @@ const NavGen = struct { comptime ucmp: CmpPredicate, comptime scmp: CmpPredicate, ) !?IdRef { + _ = scmp; // Note: OpIAddCarry and OpISubBorrow are not really useful here: For unsigned numbers, // there is in both cases only one extra operation required. For signed operations, // the overflow bit is set then going from 0x80.. to 0x00.., but this doesn't actually @@ -3689,26 +3690,30 @@ const NavGen = struct { // Overflow happened if the result is smaller than either of the operands. It doesn't matter which. // For subtraction the conditions need to be swapped. .unsigned => try self.buildCmp(ucmp, result, lhs), - // For addition, overflow happened if: - // - rhs is negative and value > lhs - // - rhs is positive and value < lhs - // This can be shortened to: - // (rhs < 0 and value > lhs) or (rhs >= 0 and value <= lhs) - // = (rhs < 0) == (value > lhs) - // = (rhs < 0) == (lhs < value) - // Note that signed overflow is also wrapping in spir-v. - // For subtraction, overflow happened if: - // - rhs is negative and value < lhs - // - rhs is positive and value > lhs - // This can be shortened to: - // (rhs < 0 and value < lhs) or (rhs >= 0 and value >= lhs) - // = (rhs < 0) == (value < lhs) - // = (rhs < 0) == (lhs > value) + // For signed operations, we check the signs of the operands and the result. .signed => blk: { + // Signed overflow detection using the sign bits of the operands and the result. + // For addition (a + b), overflow occurs if the operands have the same sign + // and the result's sign is different from the operands' sign. + // (sign(a) == sign(b)) && (sign(a) != sign(result)) + // For subtraction (a - b), overflow occurs if the operands have different signs + // and the result's sign is different from the minuend's (a's) sign. + // (sign(a) != sign(b)) && (sign(a) != sign(result)) const zero = Temporary.init(rhs.ty, try self.constInt(rhs.ty, 0)); - const rhs_lt_zero = try self.buildCmp(.s_lt, rhs, zero); - const result_gt_lhs = try self.buildCmp(scmp, lhs, result); - break :blk try self.buildCmp(.l_eq, rhs_lt_zero, result_gt_lhs); + + const lhs_is_neg = try self.buildCmp(.s_lt, lhs, zero); + const rhs_is_neg = try self.buildCmp(.s_lt, rhs, zero); + const result_is_neg = try self.buildCmp(.s_lt, result, zero); + + const signs_match = try self.buildCmp(.l_eq, lhs_is_neg, rhs_is_neg); + const result_sign_differs = try self.buildCmp(.l_ne, lhs_is_neg, result_is_neg); + + const overflow_condition = if (add == .i_add) + signs_match + else // .i_sub + try self.buildUnary(.l_not, signs_match); + + break :blk try self.buildBinary(.l_and, overflow_condition, result_sign_differs); }, }; |
