tint: implement updated spec rules for shift left of concrete values

Rules for shift left of concrete values are now split between signed and
unsigned. Shifting unsigned values no longer fails with "sign change"
errors. Furthermore, shifting unsigned values must only shift out 0s.

See https://github.com/gpuweb/gpuweb/pull/3539.

Bug: tint:1701
Bug: tint:1717
Change-Id: Iba2799f4b02cdc77cc58a6c7c104aaa408f0f0f9
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/106381
Reviewed-by: Dan Sinclair <dsinclair@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>

src/tint/resolver/const_eval.cc

@@ -1491,13 +1491,25 @@ ConstEval::Result ConstEval::OpShiftLeft(const sem::Type* ty,
                     return nullptr;
                 }
 
-                // The e2 + 1 most significant bits of e1 must have the same bit value, otherwise
+                if constexpr (std::is_signed_v<T>) {
-                // sign change (overflow) would occur.
+                    // If T is a signed integer type, and the e2+1 most significant bits of e1 do
-                size_t must_match_msb = e2u + 1;
+                    // not have the same bit value, then error.
-                UT mask = ~UT{0} << (bit_width - must_match_msb);
+                    size_t must_match_msb = e2u + 1;
-                if ((e1u & mask) != 0 && (e1u & mask) != mask) {
+                    UT mask = ~UT{0} << (bit_width - must_match_msb);
-                    AddError("shift left operation results in sign change", source);
+                    if ((e1u & mask) != 0 && (e1u & mask) != mask) {
-                    return nullptr;
+                        AddError("shift left operation results in sign change", source);
+                        return nullptr;
+                    }
+                } else {
+                    // If T is an unsigned integer type, and any of the e2 most significant bits of
+                    // e1 are 1, then error.
+                    if (e2u > 0) {
+                        size_t must_be_zero_msb = e2u;
+                        UT mask = ~UT{0} << (bit_width - must_be_zero_msb);
+                        if ((e1u & mask) != 0) {
+                            AddError(OverflowErrorMessage(e1, "<<", e2), source);
+                        }
+                    }
                 }
             }
 

src/tint/resolver/const_eval_binary_op_test.cc

@@ -611,19 +611,16 @@ std::vector<Case> ShiftLeftCases() {
     using ST = std::conditional_t<IsAbstract<T>, T, u32>;
     using B = BitValues<T>;
     auto r = std::vector<Case>{
-        C(T{0b1010}, ST{0}, T{0b0000'0000'1010}),    //
+        C(T{0b1010}, ST{0}, T{0b0000'0000'1010}),  //
-        C(T{0b1010}, ST{1}, T{0b0000'0001'0100}),    //
+        C(T{0b1010}, ST{1}, T{0b0000'0001'0100}),  //
-        C(T{0b1010}, ST{2}, T{0b0000'0010'1000}),    //
+        C(T{0b1010}, ST{2}, T{0b0000'0010'1000}),  //
-        C(T{0b1010}, ST{3}, T{0b0000'0101'0000}),    //
+        C(T{0b1010}, ST{3}, T{0b0000'0101'0000}),  //
-        C(T{0b1010}, ST{4}, T{0b0000'1010'0000}),    //
+        C(T{0b1010}, ST{4}, T{0b0000'1010'0000}),  //
-        C(T{0b1010}, ST{5}, T{0b0001'0100'0000}),    //
+        C(T{0b1010}, ST{5}, T{0b0001'0100'0000}),  //
-        C(T{0b1010}, ST{6}, T{0b0010'1000'0000}),    //
+        C(T{0b1010}, ST{6}, T{0b0010'1000'0000}),  //
-        C(T{0b1010}, ST{7}, T{0b0101'0000'0000}),    //
+        C(T{0b1010}, ST{7}, T{0b0101'0000'0000}),  //
-        C(T{0b1010}, ST{8}, T{0b1010'0000'0000}),    //
+        C(T{0b1010}, ST{8}, T{0b1010'0000'0000}),  //
-        C(B::LeftMost, ST{0}, B::LeftMost),          //
+        C(B::LeftMost, ST{0}, B::LeftMost),        //
-        C(B::TwoLeftMost, ST{1}, B::LeftMost),       // No overflow
-        C(B::All, ST{1}, B::AllButRightMost),        // No overflow
-        C(B::All, ST{B::NumBits - 1}, B::LeftMost),  // No overflow
 
         C(Vec(T{0b1010}, T{0b1010}),                                            //
           Vec(ST{0}, ST{1}),                                                    //
@@ -641,7 +638,7 @@ std::vector<Case> ShiftLeftCases() {
 
     // Only abstract 0 can be shifted left as much as we like. For concrete 0 (and any number), it
     // cannot be shifted equal or more than the number of bits of the lhs (see
-    // ResolverConstEvalShiftLeftConcreteGeqBitWidthError)
+    // ResolverConstEvalShiftLeftConcreteGeqBitWidthError for negative tests)
     ConcatIntoIf<IsAbstract<T>>(  //
         r, std::vector<Case>{
                C(T{0}, ST{64}, T{0}),
@@ -656,6 +653,31 @@ std::vector<Case> ShiftLeftCases() {
                C(Negate(T{0}), T::Highest(), Negate(T{0})),
            });
 
+    // Cases that are fine for signed values (no sign change), but would overflow unsigned values.
+    // See ResolverConstEvalBinaryOpTest_Overflow for negative tests.
+    ConcatIntoIf<IsSignedIntegral<T>>(  //
+        r, std::vector<Case>{
+               C(B::TwoLeftMost, ST{1}, B::LeftMost),      //
+               C(B::All, ST{1}, B::AllButRightMost),       //
+               C(B::All, ST{B::NumBits - 1}, B::LeftMost)  //
+           });
+
+    // Cases that are fine for unsigned values, but would overflow (sign change) signed
+    // values. See ShiftLeftSignChangeErrorCases() for negative tests.
+    ConcatIntoIf<IsUnsignedIntegral<T>>(  //
+        r, std::vector<Case>{
+               C(T{0b0001}, ST{B::NumBits - 1}, B::Lsh(0b0001, B::NumBits - 1)),
+               C(T{0b0010}, ST{B::NumBits - 2}, B::Lsh(0b0010, B::NumBits - 2)),
+               C(T{0b0100}, ST{B::NumBits - 3}, B::Lsh(0b0100, B::NumBits - 3)),
+               C(T{0b1000}, ST{B::NumBits - 4}, B::Lsh(0b1000, B::NumBits - 4)),
+
+               C(T{0b0011}, ST{B::NumBits - 2}, B::Lsh(0b0011, B::NumBits - 2)),
+               C(T{0b0110}, ST{B::NumBits - 3}, B::Lsh(0b0110, B::NumBits - 3)),
+               C(T{0b1100}, ST{B::NumBits - 4}, B::Lsh(0b1100, B::NumBits - 4)),
+
+               C(B::AllButLeftMost, ST{1}, B::AllButRightMost),
+           });
+
     return r;
 }
 INSTANTIATE_TEST_SUITE_P(ShiftLeft,
@@ -795,7 +817,21 @@ INSTANTIATE_TEST_SUITE_P(
                      Val(AInt{BitValues<AInt>::NumBits + 1})},    //
         OverflowCase{ast::BinaryOp::kShiftLeft,                   //
                      Val(AInt{BitValues<AInt>::AllButLeftMost}),  //
-                     Val(AInt{BitValues<AInt>::NumBits + 1000})}
+                     Val(AInt{BitValues<AInt>::NumBits + 1000})},
+
+        // ShiftLeft of u32s that overflow (non-zero bits get shifted out)
+        OverflowCase{ast::BinaryOp::kShiftLeft, Val(0b00010_u), Val(31_u)},
+        OverflowCase{ast::BinaryOp::kShiftLeft, Val(0b00100_u), Val(30_u)},
+        OverflowCase{ast::BinaryOp::kShiftLeft, Val(0b01000_u), Val(29_u)},
+        OverflowCase{ast::BinaryOp::kShiftLeft, Val(0b10000_u), Val(28_u)},
+        // ...
+        OverflowCase{ast::BinaryOp::kShiftLeft, Val(u32(1u << 28)), Val(4_u)},
+        OverflowCase{ast::BinaryOp::kShiftLeft, Val(u32(1u << 29)), Val(3_u)},
+        OverflowCase{ast::BinaryOp::kShiftLeft, Val(u32(1u << 30)), Val(2_u)},
+        OverflowCase{ast::BinaryOp::kShiftLeft, Val(u32(1u << 31)), Val(1_u)},
+        // And some more
+        OverflowCase{ast::BinaryOp::kShiftLeft, Val(BitValues<u32>::All), Val(1_u)},
+        OverflowCase{ast::BinaryOp::kShiftLeft, Val(BitValues<u32>::AllButLeftMost), Val(2_u)}
 
         ));
 
@@ -945,8 +981,7 @@ INSTANTIATE_TEST_SUITE_P(Test,
                          ResolverConstEvalShiftLeftSignChangeError,
                          testing::ValuesIn(Concat(  //
                              ShiftLeftSignChangeErrorCases<AInt>(),
-                             ShiftLeftSignChangeErrorCases<i32>(),
+                             ShiftLeftSignChangeErrorCases<i32>())));
-                             ShiftLeftSignChangeErrorCases<u32>())));
 
 }  // namespace
 }  // namespace tint::resolver

src/tint/resolver/const_eval_test.h

@@ -281,7 +281,7 @@ struct BitValues {
     /// @returns the shifted value
     template <typename U, typename V>
     static constexpr NumberT Lsh(U val, V shiftBy) {
-        return NumberT{T{val} << T{shiftBy}};
+        return NumberT{static_cast<T>(val) << static_cast<T>(shiftBy)};
     }
 };
 

test/tint/bug/tint/1717.wgsl

@@ -0,0 +1,3 @@
+fn f() -> u32 {
+    return 0x1u << 31u;
+}

test/tint/bug/tint/1717.wgsl.expected.dxc.hlsl

@@ -0,0 +1,8 @@
+[numthreads(1, 1, 1)]
+void unused_entry_point() {
+  return;
+}
+
+uint f() {
+  return 2147483648u;
+}

test/tint/bug/tint/1717.wgsl.expected.fxc.hlsl

@@ -0,0 +1,8 @@
+[numthreads(1, 1, 1)]
+void unused_entry_point() {
+  return;
+}
+
+uint f() {
+  return 2147483648u;
+}

test/tint/bug/tint/1717.wgsl.expected.glsl

@@ -0,0 +1,10 @@
+#version 310 es
+
+layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in;
+void unused_entry_point() {
+  return;
+}
+uint f() {
+  return 2147483648u;
+}
+

test/tint/bug/tint/1717.wgsl.expected.msl

@@ -0,0 +1,7 @@
+#include <metal_stdlib>
+
+using namespace metal;
+uint f() {
+  return 2147483648u;
+}
+

test/tint/bug/tint/1717.wgsl.expected.spvasm

@@ -0,0 +1,24 @@
+; SPIR-V
+; Version: 1.3
+; Generator: Google Tint Compiler; 0
+; Bound: 10
+; Schema: 0
+               OpCapability Shader
+               OpMemoryModel Logical GLSL450
+               OpEntryPoint GLCompute %unused_entry_point "unused_entry_point"
+               OpExecutionMode %unused_entry_point LocalSize 1 1 1
+               OpName %unused_entry_point "unused_entry_point"
+               OpName %f "f"
+       %void = OpTypeVoid
+          %1 = OpTypeFunction %void
+       %uint = OpTypeInt 32 0
+          %5 = OpTypeFunction %uint
+%uint_2147483648 = OpConstant %uint 2147483648
+%unused_entry_point = OpFunction %void None %1
+          %4 = OpLabel
+               OpReturn
+               OpFunctionEnd
+          %f = OpFunction %uint None %5
+          %8 = OpLabel
+               OpReturnValue %uint_2147483648
+               OpFunctionEnd

test/tint/bug/tint/1717.wgsl.expected.wgsl

@@ -0,0 +1,3 @@
+fn f() -> u32 {
+  return (1u << 31u);
+}