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>
This commit is contained in:
Ben Clayton 2022-06-29 11:31:41 +00:00 committed by Dawn LUCI CQ
parent 368f95a078
commit 2b4df78891
2 changed files with 20 additions and 1 deletions

View File

@ -282,7 +282,9 @@ std::ostream& operator<<(std::ostream& out, ConversionFailure failure);
/// @returns the resulting value of the conversion, or a failure reason. /// @returns the resulting value of the conversion, or a failure reason.
template <typename TO, typename FROM> template <typename TO, typename FROM>
utils::Result<TO, ConversionFailure> CheckedConvert(Number<FROM> num) { 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); const auto value = static_cast<T>(num.value);
if (value > static_cast<T>(TO::kHighest)) { if (value > static_cast<T>(TO::kHighest)) {
return ConversionFailure::kExceedsPositiveLimit; return ConversionFailure::kExceedsPositiveLimit;

View File

@ -84,6 +84,7 @@ TEST(NumberTest, CheckedConvertIdentity) {
TEST(NumberTest, CheckedConvertLargestValue) { TEST(NumberTest, CheckedConvertLargestValue) {
EXPECT_EQ(CheckedConvert<i32>(AInt(kHighestI32)), i32(kHighestI32)); EXPECT_EQ(CheckedConvert<i32>(AInt(kHighestI32)), i32(kHighestI32));
EXPECT_EQ(CheckedConvert<u32>(AInt(kHighestU32)), u32(kHighestU32)); 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<f32>(AFloat(kHighestF32)), f32(kHighestF32));
EXPECT_EQ(CheckedConvert<f16>(AFloat(kHighestF16)), f16(kHighestF16)); EXPECT_EQ(CheckedConvert<f16>(AFloat(kHighestF16)), f16(kHighestF16));
} }
@ -105,6 +106,14 @@ TEST(NumberTest, CheckedConvertSmallestValue) {
TEST(NumberTest, CheckedConvertExceedsPositiveLimit) { TEST(NumberTest, CheckedConvertExceedsPositiveLimit) {
EXPECT_EQ(CheckedConvert<i32>(AInt(kHighestI32 + 1)), ConversionFailure::kExceedsPositiveLimit); EXPECT_EQ(CheckedConvert<i32>(AInt(kHighestI32 + 1)), ConversionFailure::kExceedsPositiveLimit);
EXPECT_EQ(CheckedConvert<u32>(AInt(kHighestU32 + 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)), EXPECT_EQ(CheckedConvert<f32>(AFloat(kHighestF32NextULP)),
ConversionFailure::kExceedsPositiveLimit); ConversionFailure::kExceedsPositiveLimit);
EXPECT_EQ(CheckedConvert<f16>(AFloat(kHighestF16NextULP)), EXPECT_EQ(CheckedConvert<f16>(AFloat(kHighestF16NextULP)),
@ -114,6 +123,14 @@ TEST(NumberTest, CheckedConvertExceedsPositiveLimit) {
TEST(NumberTest, CheckedConvertExceedsNegativeLimit) { TEST(NumberTest, CheckedConvertExceedsNegativeLimit) {
EXPECT_EQ(CheckedConvert<i32>(AInt(kLowestI32 - 1)), ConversionFailure::kExceedsNegativeLimit); EXPECT_EQ(CheckedConvert<i32>(AInt(kLowestI32 - 1)), ConversionFailure::kExceedsNegativeLimit);
EXPECT_EQ(CheckedConvert<u32>(AInt(kLowestU32 - 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)), EXPECT_EQ(CheckedConvert<f32>(AFloat(kLowestF32NextULP)),
ConversionFailure::kExceedsNegativeLimit); ConversionFailure::kExceedsNegativeLimit);
EXPECT_EQ(CheckedConvert<f16>(AFloat(kLowestF16NextULP)), EXPECT_EQ(CheckedConvert<f16>(AFloat(kLowestF16NextULP)),