commit 2b4df7889186d2e98e437a2d1bb3e083b4722edb
author Ben Clayton <bclayton@google.com> Wed Jun 29 11:31:41 2022 +0000
committer Dawn LUCI CQ <dawn-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed Jun 29 11:31:41 2022 +0000
tree 7620235a49674b2cda20db194f160268ccd0eeda
parent 368f95a078eeadc547a3b3d1d0a55847130f3404

tint/number: Fix CheckedConvert() logic

This function was attempting to pick a higher-precision type by using
the decltype() of FROM + TO. This doesn't work if FROM and TO are both
similar bit-widths, and of a different signness, as the picked type may
not be wide enough to hold both the signed and unsigned representation.

Just use AInt or AFloat (both 64-bit), which are the largest types
supported by WGSL.

Change-Id: Ic76475d98bad8def12a0283a1c83c62f2ed58b5d
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/95041
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@google.com>

src/tint/number.h

diff --git a/src/tint/number.h b/src/tint/number.h
index 7f0c2f1..4e3e6fd 100644
--- a/src/tint/number.h
+++ b/src/tint/number.h
@@ -282,7 +282,9 @@
 /// @returns the resulting value of the conversion, or a failure reason.
 template <typename TO, typename FROM>
 utils::Result<TO, ConversionFailure> CheckedConvert(Number<FROM> num) {
-    using T = decltype(UnwrapNumber<TO>() + num.value);
+    // Use the highest-precision integer or floating-point type to perform the comparisons.
+    using T = std::conditional_t<IsFloatingPoint<UnwrapNumber<TO>> || IsFloatingPoint<FROM>,
+                                 AFloat::type, AInt::type>;
     const auto value = static_cast<T>(num.value);
     if (value > static_cast<T>(TO::kHighest)) {
         return ConversionFailure::kExceedsPositiveLimit;

src/tint/number_test.cc

diff --git a/src/tint/number_test.cc b/src/tint/number_test.cc
index 52ba4ae..6a48f9d 100644
--- a/src/tint/number_test.cc
+++ b/src/tint/number_test.cc
@@ -84,6 +84,7 @@
 TEST(NumberTest, CheckedConvertLargestValue) {
     EXPECT_EQ(CheckedConvert<i32>(AInt(kHighestI32)), i32(kHighestI32));
     EXPECT_EQ(CheckedConvert<u32>(AInt(kHighestU32)), u32(kHighestU32));
+    EXPECT_EQ(CheckedConvert<u32>(i32(kHighestI32)), u32(kHighestI32));
     EXPECT_EQ(CheckedConvert<f32>(AFloat(kHighestF32)), f32(kHighestF32));
     EXPECT_EQ(CheckedConvert<f16>(AFloat(kHighestF16)), f16(kHighestF16));
 }
@@ -105,6 +106,14 @@
 TEST(NumberTest, CheckedConvertExceedsPositiveLimit) {
     EXPECT_EQ(CheckedConvert<i32>(AInt(kHighestI32 + 1)), ConversionFailure::kExceedsPositiveLimit);
     EXPECT_EQ(CheckedConvert<u32>(AInt(kHighestU32 + 1)), ConversionFailure::kExceedsPositiveLimit);
+    EXPECT_EQ(CheckedConvert<i32>(u32(kHighestU32)), ConversionFailure::kExceedsPositiveLimit);
+    EXPECT_EQ(CheckedConvert<i32>(u32(0x80000000)), ConversionFailure::kExceedsPositiveLimit);
+    EXPECT_EQ(CheckedConvert<u32>(f32(f32::kHighest)), ConversionFailure::kExceedsPositiveLimit);
+    EXPECT_EQ(CheckedConvert<i32>(f32(f32::kHighest)), ConversionFailure::kExceedsPositiveLimit);
+    EXPECT_EQ(CheckedConvert<u32>(AFloat(AFloat::kHighest)),
+              ConversionFailure::kExceedsPositiveLimit);
+    EXPECT_EQ(CheckedConvert<i32>(AFloat(AFloat::kHighest)),
+              ConversionFailure::kExceedsPositiveLimit);
     EXPECT_EQ(CheckedConvert<f32>(AFloat(kHighestF32NextULP)),
               ConversionFailure::kExceedsPositiveLimit);
     EXPECT_EQ(CheckedConvert<f16>(AFloat(kHighestF16NextULP)),
@@ -114,6 +123,14 @@
 TEST(NumberTest, CheckedConvertExceedsNegativeLimit) {
     EXPECT_EQ(CheckedConvert<i32>(AInt(kLowestI32 - 1)), ConversionFailure::kExceedsNegativeLimit);
     EXPECT_EQ(CheckedConvert<u32>(AInt(kLowestU32 - 1)), ConversionFailure::kExceedsNegativeLimit);
+    EXPECT_EQ(CheckedConvert<u32>(i32(-1)), ConversionFailure::kExceedsNegativeLimit);
+    EXPECT_EQ(CheckedConvert<u32>(i32(kLowestI32)), ConversionFailure::kExceedsNegativeLimit);
+    EXPECT_EQ(CheckedConvert<u32>(f32(f32::kLowest)), ConversionFailure::kExceedsNegativeLimit);
+    EXPECT_EQ(CheckedConvert<i32>(f32(f32::kLowest)), ConversionFailure::kExceedsNegativeLimit);
+    EXPECT_EQ(CheckedConvert<u32>(AFloat(AFloat::kLowest)),
+              ConversionFailure::kExceedsNegativeLimit);
+    EXPECT_EQ(CheckedConvert<i32>(AFloat(AFloat::kLowest)),
+              ConversionFailure::kExceedsNegativeLimit);
     EXPECT_EQ(CheckedConvert<f32>(AFloat(kLowestF32NextULP)),
               ConversionFailure::kExceedsNegativeLimit);
     EXPECT_EQ(CheckedConvert<f16>(AFloat(kLowestF16NextULP)),