Revert "tint: rhs of left shift is always u32 or vector of u32"

This reverts commit cc9ae5cae3.

Reason for revert: Kokoro failed on this CL

Original change's description:
> tint: rhs of left shift is always u32 or vector of u32
>
> Implements recent spec change:
> https://github.com/gpuweb/gpuweb/pull/3516
>
> Bug: tint:1713
> Change-Id: I678e3c15ec8ab010dca43fc814cf0bc79486f4b6
> Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/106721
> Commit-Queue: Antonio Maiorano <amaiorano@google.com>
> Reviewed-by: Dan Sinclair <dsinclair@chromium.org>

TBR=dsinclair@chromium.org,amaiorano@google.com,noreply+kokoro@google.com,dawn-scoped@luci-project-accounts.iam.gserviceaccount.com

Change-Id: I9d2337b77e2853c9e26300d0bfdbfba744c54c48
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: tint:1713
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/106847
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: dan sinclair <dsinclair@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
This commit is contained in:
Antonio Maiorano 2022-10-24 20:30:12 +00:00 committed by Dawn LUCI CQ
parent a3325fa94d
commit 087c355e08
11 changed files with 4610 additions and 4670 deletions

View File

@ -972,8 +972,8 @@ op || (bool, bool) -> bool
@const op << <T: iu32>(T, u32) -> T @const op << <T: iu32>(T, u32) -> T
@const op << <T: iu32, N: num> (vec<N, T>, vec<N, u32>) -> vec<N, T> @const op << <T: iu32, N: num> (vec<N, T>, vec<N, u32>) -> vec<N, T>
@const op << <T: ia>(T, u32) -> T @const op << <T: ia>(T, ia) -> T
@const op << <T: ia, N: num> (vec<N, T>, vec<N, u32>) -> vec<N, T> @const op << <T: ia, N: num> (vec<N, T>, vec<N, ia>) -> vec<N, T>
op >> <T: iu32>(T, u32) -> T op >> <T: iu32>(T, u32) -> T
op >> <T: iu32, N: num> (vec<N, T>, vec<N, u32>) -> vec<N, T> op >> <T: iu32, N: num> (vec<N, T>, vec<N, u32>) -> vec<N, T>

View File

@ -54,7 +54,7 @@ T First(T&& first, ...) {
} }
/// Helper that calls `f` passing in the value of all `cs`. /// Helper that calls `f` passing in the value of all `cs`.
/// Calls `f` with all constants cast to the type of the first `cs` argument. /// Assumes all `cs` are of the same type.
template <typename F, typename... CONSTANTS> template <typename F, typename... CONSTANTS>
auto Dispatch_ia_iu32(F&& f, CONSTANTS&&... cs) { auto Dispatch_ia_iu32(F&& f, CONSTANTS&&... cs) {
return Switch( return Switch(
@ -65,7 +65,7 @@ auto Dispatch_ia_iu32(F&& f, CONSTANTS&&... cs) {
} }
/// Helper that calls `f` passing in the value of all `cs`. /// Helper that calls `f` passing in the value of all `cs`.
/// Calls `f` with all constants cast to the type of the first `cs` argument. /// Assumes all `cs` are of the same type.
template <typename F, typename... CONSTANTS> template <typename F, typename... CONSTANTS>
auto Dispatch_ia_iu32_bool(F&& f, CONSTANTS&&... cs) { auto Dispatch_ia_iu32_bool(F&& f, CONSTANTS&&... cs) {
return Switch( return Switch(
@ -77,7 +77,7 @@ auto Dispatch_ia_iu32_bool(F&& f, CONSTANTS&&... cs) {
} }
/// Helper that calls `f` passing in the value of all `cs`. /// Helper that calls `f` passing in the value of all `cs`.
/// Calls `f` with all constants cast to the type of the first `cs` argument. /// Assumes all `cs` are of the same type.
template <typename F, typename... CONSTANTS> template <typename F, typename... CONSTANTS>
auto Dispatch_fia_fi32_f16(F&& f, CONSTANTS&&... cs) { auto Dispatch_fia_fi32_f16(F&& f, CONSTANTS&&... cs) {
return Switch( return Switch(
@ -90,7 +90,7 @@ auto Dispatch_fia_fi32_f16(F&& f, CONSTANTS&&... cs) {
} }
/// Helper that calls `f` passing in the value of all `cs`. /// Helper that calls `f` passing in the value of all `cs`.
/// Calls `f` with all constants cast to the type of the first `cs` argument. /// Assumes all `cs` are of the same type.
template <typename F, typename... CONSTANTS> template <typename F, typename... CONSTANTS>
auto Dispatch_fia_fiu32_f16(F&& f, CONSTANTS&&... cs) { auto Dispatch_fia_fiu32_f16(F&& f, CONSTANTS&&... cs) {
return Switch( return Switch(
@ -104,7 +104,7 @@ auto Dispatch_fia_fiu32_f16(F&& f, CONSTANTS&&... cs) {
} }
/// Helper that calls `f` passing in the value of all `cs`. /// Helper that calls `f` passing in the value of all `cs`.
/// Calls `f` with all constants cast to the type of the first `cs` argument. /// Assumes all `cs` are of the same type.
template <typename F, typename... CONSTANTS> template <typename F, typename... CONSTANTS>
auto Dispatch_fia_fiu32_f16_bool(F&& f, CONSTANTS&&... cs) { auto Dispatch_fia_fiu32_f16_bool(F&& f, CONSTANTS&&... cs) {
return Switch( return Switch(
@ -119,7 +119,7 @@ auto Dispatch_fia_fiu32_f16_bool(F&& f, CONSTANTS&&... cs) {
} }
/// Helper that calls `f` passing in the value of all `cs`. /// Helper that calls `f` passing in the value of all `cs`.
/// Calls `f` with all constants cast to the type of the first `cs` argument. /// Assumes all `cs` are of the same type.
template <typename F, typename... CONSTANTS> template <typename F, typename... CONSTANTS>
auto Dispatch_fa_f32_f16(F&& f, CONSTANTS&&... cs) { auto Dispatch_fa_f32_f16(F&& f, CONSTANTS&&... cs) {
return Switch( return Switch(
@ -130,7 +130,7 @@ auto Dispatch_fa_f32_f16(F&& f, CONSTANTS&&... cs) {
} }
/// Helper that calls `f` passing in the value of all `cs`. /// Helper that calls `f` passing in the value of all `cs`.
/// Calls `f` with all constants cast to the type of the first `cs` argument. /// Assumes all `cs` are of the same type.
template <typename F, typename... CONSTANTS> template <typename F, typename... CONSTANTS>
auto Dispatch_bool(F&& f, CONSTANTS&&... cs) { auto Dispatch_bool(F&& f, CONSTANTS&&... cs) {
return f(cs->template As<bool>()...); return f(cs->template As<bool>()...);
@ -1450,6 +1450,13 @@ ConstEval::Result ConstEval::OpShiftLeft(const sem::Type* ty,
UT e2u = static_cast<UT>(e2); UT e2u = static_cast<UT>(e2);
if constexpr (IsAbstract<NumberT>) { if constexpr (IsAbstract<NumberT>) {
// NOTE: Concrete shift left requires an unsigned rhs, so this check only applies
// for abstracts.
if (e2 < 0) {
AddError("cannot shift left by a negative value", source);
return nullptr;
}
// The e2 + 1 most significant bits of e1 must have the same bit value, otherwise // The e2 + 1 most significant bits of e1 must have the same bit value, otherwise
// sign change (overflow) would occur. // sign change (overflow) would occur.
// Check sign change only if e2 is less than bit width of e1. If e1 is larger // Check sign change only if e2 is less than bit width of e1. If e1 is larger
@ -1513,12 +1520,6 @@ ConstEval::Result ConstEval::OpShiftLeft(const sem::Type* ty,
return Dispatch_ia_iu32(create, c0, c1); return Dispatch_ia_iu32(create, c0, c1);
}; };
if (!sem::Type::DeepestElementOf(args[1]->Type())->Is<sem::U32>()) {
TINT_ICE(Resolver, builder.Diagnostics())
<< "Element type of rhs of ShiftLeft must be a u32";
return nullptr;
}
auto r = TransformElements(builder, ty, transform, args[0], args[1]); auto r = TransformElements(builder, ty, transform, args[0], args[1]);
if (builder.Diagnostics().contains_errors()) { if (builder.Diagnostics().contains_errors()) {
return utils::Failure; return utils::Failure;

View File

@ -636,21 +636,21 @@ std::vector<Case> ShiftLeftCases() {
Vec(T{0b0010'1000'0000}, T{0b0101'0000'0000}, T{0b1010'0000'0000})), // Vec(T{0b0010'1000'0000}, T{0b0101'0000'0000}, T{0b1010'0000'0000})), //
}; };
// Abstract 0 can be shifted by any u32 value (0 to 2^32), whereas concrete 0 (or any number) // Only abstract 0 can be shifted left as much as we like. For concrete 0 (and any number), it
// can only by shifted by a value less than the number of bits of the lhs. // cannot be shifted equal or more than the number of bits of the lhs (see
// (see ResolverConstEvalShiftLeftConcreteGeqBitWidthError for negative tests) // ResolverConstEvalShiftLeftConcreteGeqBitWidthError for negative tests)
ConcatIntoIf<IsAbstract<T>>( // ConcatIntoIf<IsAbstract<T>>( //
r, std::vector<Case>{ r, std::vector<Case>{
C(T{0}, ST{64}, T{0}), // C(T{0}, ST{64}, T{0}),
C(T{0}, ST{65}, T{0}), // C(T{0}, ST{65}, T{0}),
C(T{0}, ST{65}, T{0}), // C(T{0}, ST{65}, T{0}),
C(T{0}, ST{10000}, T{0}), // C(T{0}, ST{10000}, T{0}),
C(T{0}, ST{u32::Highest()}, T{0}), // C(T{0}, T::Highest(), T{0}),
C(Negate(T{0}), ST{64}, Negate(T{0})), // C(Negate(T{0}), ST{64}, Negate(T{0})),
C(Negate(T{0}), ST{65}, Negate(T{0})), // C(Negate(T{0}), ST{65}, Negate(T{0})),
C(Negate(T{0}), ST{65}, Negate(T{0})), // C(Negate(T{0}), ST{65}, Negate(T{0})),
C(Negate(T{0}), ST{10000}, Negate(T{0})), // C(Negate(T{0}), ST{10000}, Negate(T{0})),
C(Negate(T{0}), ST{u32::Highest()}, Negate(T{0})), // C(Negate(T{0}), T::Highest(), Negate(T{0})),
}); });
// Cases that are fine for signed values (no sign change), but would overflow unsigned values. // Cases that are fine for signed values (no sign change), but would overflow unsigned values.
@ -898,29 +898,9 @@ INSTANTIATE_TEST_SUITE_P(
// AInt left shift negative value -> error // AInt left shift negative value -> error
TEST_F(ResolverConstEvalTest, BinaryAbstractShiftLeftByNegativeValue_Error) { TEST_F(ResolverConstEvalTest, BinaryAbstractShiftLeftByNegativeValue_Error) {
GlobalConst("c", Shl(Expr(1_a), Expr(Source{{1, 1}}, -1_a))); GlobalConst("c", Shl(Source{{1, 1}}, Expr(1_a), Expr(-1_a)));
EXPECT_FALSE(r()->Resolve()); EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), "1:1 error: value -1 cannot be represented as 'u32'"); EXPECT_EQ(r()->error(), "1:1 error: cannot shift left by a negative value");
}
// AInt left shift by AInt or u32 always results in an AInt
TEST_F(ResolverConstEvalTest, BinaryAbstractShiftLeftRemainsAbstract) {
auto* expr1 = Shl(Expr(1_a), Expr(1_u));
GlobalConst("c1", expr1);
auto* expr2 = Shl(Expr(1_a), Expr(1_a));
GlobalConst("c2", expr2);
EXPECT_TRUE(r()->Resolve()) << r()->error();
auto* sem1 = Sem().Get(expr1);
ASSERT_NE(sem1, nullptr);
auto* sem2 = Sem().Get(expr2);
ASSERT_NE(sem2, nullptr);
auto aint_ty = create<sem::AbstractInt>();
EXPECT_EQ(sem1->Type(), aint_ty);
EXPECT_EQ(sem2->Type(), aint_ty);
} }
// i32/u32 left shift by >= 32 -> error // i32/u32 left shift by >= 32 -> error

File diff suppressed because it is too large Load Diff

View File

@ -1,4 +0,0 @@
const a = 1 << 30u; // First shift, result should be abstract
const b = a << 10u; // Valid only if 'a' is abstract
const c = 5000000000 << 3u; // Valid only if result is abtract

View File

@ -1,5 +0,0 @@
[numthreads(1, 1, 1)]
void unused_entry_point() {
return;
}

View File

@ -1,5 +0,0 @@
[numthreads(1, 1, 1)]
void unused_entry_point() {
return;
}

View File

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

View File

@ -1,3 +0,0 @@
#include <metal_stdlib>
using namespace metal;

View File

@ -1,16 +0,0 @@
; SPIR-V
; Version: 1.3
; Generator: Google Tint Compiler; 0
; Bound: 5
; 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"
%void = OpTypeVoid
%1 = OpTypeFunction %void
%unused_entry_point = OpFunction %void None %1
%4 = OpLabel
OpReturn
OpFunctionEnd

View File

@ -1,5 +0,0 @@
const a = (1 << 30u);
const b = (a << 10u);
const c = (5000000000 << 3u);