Per https://gpuweb.github.io/gpuweb/wgsl/#bit-expr we need to report overflow when value goes into sign bits, while rust's checked_shl only reports overflow when its out of bits.
Originally posted by @sagudev in #6156 (comment)
Actually the issue is much worse, checked_shl only performs checks on rhs not for actual overflow.
Relevant tint code: https://github.com/google/dawn/blob/03b32d20940d1b0e6d979c30ee327c309de6ed01/src/tint/lang/core/constant/eval.cc#L1977
But I think this should make it work:
diff --git a/naga/src/proc/constant_evaluator.rs b/naga/src/proc/constant_evaluator.rs
index a79944a3f..c4ef04e25 100644
--- a/naga/src/proc/constant_evaluator.rs
+++ b/naga/src/proc/constant_evaluator.rs
@@ -1832,8 +1832,11 @@ impl<'a> ConstantEvaluator<'a> {
}),
(Literal::I32(a), Literal::U32(b)) => Literal::I32(match op {
BinaryOperator::ShiftLeft => a
- .checked_shl(b)
- .ok_or(ConstantEvaluatorError::ShiftedMoreThan32Bits)?,
+ .checked_mul(
+ 1i32.checked_shl(b)
+ .ok_or(ConstantEvaluatorError::ShiftedMoreThan32Bits)?,
+ )
+ .ok_or(ConstantEvaluatorError::Overflow("<<".to_string()))?,
BinaryOperator::ShiftRight => a
.checked_shr(b)
.ok_or(ConstantEvaluatorError::ShiftedMoreThan32Bits)?,
@@ -1859,8 +1862,11 @@ impl<'a> ConstantEvaluator<'a> {
BinaryOperator::ExclusiveOr => a ^ b,
BinaryOperator::InclusiveOr => a | b,
BinaryOperator::ShiftLeft => a
- .checked_shl(b)
- .ok_or(ConstantEvaluatorError::ShiftedMoreThan32Bits)?,
+ .checked_mul(
+ 1u32.checked_shl(b)
+ .ok_or(ConstantEvaluatorError::ShiftedMoreThan32Bits)?,
+ )
+ .ok_or(ConstantEvaluatorError::Overflow("<<".to_string()))?,
BinaryOperator::ShiftRight => a
.checked_shr(b)
.ok_or(ConstantEvaluatorError::ShiftedMoreThan32Bits)?,
Per https://gpuweb.github.io/gpuweb/wgsl/#bit-expr we need to report overflow when value goes into sign bits, while rust's
checked_shlonly reports overflow when its out of bits.Originally posted by @sagudev in #6156 (comment)
Actually the issue is much worse,
checked_shlonly performs checks on rhs not for actual overflow.Relevant tint code: https://github.com/google/dawn/blob/03b32d20940d1b0e6d979c30ee327c309de6ed01/src/tint/lang/core/constant/eval.cc#L1977
But I think this should make it work: