tint: improve error message for const eval operator overflow

Bug: tint:1581
Change-Id: I112c7981d6b1998dd8133d1f80409fdc12a16a21
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/101042
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
This commit is contained in:
Antonio Maiorano 2022-09-02 23:01:00 +00:00 committed by Dawn LUCI CQ
parent 863d9edf59
commit 8478dfca56
2 changed files with 71 additions and 79 deletions

View File

@ -140,6 +140,14 @@ inline bool IsPositiveZero(T value) {
return Number<N>(value) == Number<N>(0); // Considers sign bit
}
template <typename NumberT>
std::string OverflowErrorMessage(NumberT lhs, const char* op, NumberT rhs) {
std::stringstream ss;
ss << "'" << lhs.value << " " << op << " " << rhs.value << "' cannot be represented as '"
<< FriendlyName<NumberT>() << "'";
return ss.str();
}
/// Constant inherits from sem::Constant to add an private implementation method for conversion.
struct Constant : public sem::Constant {
/// Convert attempts to convert the constant value to the given type. On error, Convert()
@ -515,28 +523,26 @@ ConstEval::ConstEval(ProgramBuilder& b) : builder(b) {}
template <typename NumberT>
utils::Result<NumberT> ConstEval::Add(NumberT a, NumberT b) {
using T = UnwrapNumber<NumberT>;
auto add_values = [](T lhs, T rhs) {
if constexpr (std::is_integral_v<T> && std::is_signed_v<T>) {
// Ensure no UB for signed overflow
using UT = std::make_unsigned_t<T>;
return static_cast<T>(static_cast<UT>(lhs) + static_cast<UT>(rhs));
} else {
return lhs + rhs;
}
};
NumberT result;
if constexpr (std::is_same_v<NumberT, AInt> || std::is_same_v<NumberT, AFloat>) {
// Check for over/underflow for abstract values
if (auto r = CheckedAdd(a, b)) {
result = r->value;
} else {
AddError("'" + std::to_string(add_values(a.value, b.value)) +
"' cannot be represented as '" + FriendlyName<NumberT>() + "'",
*current_source);
AddError(OverflowErrorMessage(a, "+", b), *current_source);
return utils::Failure;
}
} else {
using T = UnwrapNumber<NumberT>;
auto add_values = [](T lhs, T rhs) {
if constexpr (std::is_integral_v<T> && std::is_signed_v<T>) {
// Ensure no UB for signed overflow
using UT = std::make_unsigned_t<T>;
return static_cast<T>(static_cast<UT>(lhs) + static_cast<UT>(rhs));
} else {
return lhs + rhs;
}
};
result = add_values(a.value, b.value);
}
return result;
@ -545,27 +551,25 @@ utils::Result<NumberT> ConstEval::Add(NumberT a, NumberT b) {
template <typename NumberT>
utils::Result<NumberT> ConstEval::Mul(NumberT a, NumberT b) {
using T = UnwrapNumber<NumberT>;
auto mul_values = [](T lhs, T rhs) { //
if constexpr (std::is_integral_v<T> && std::is_signed_v<T>) {
// For signed integrals, avoid C++ UB by multiplying as unsigned
using UT = std::make_unsigned_t<T>;
return static_cast<T>(static_cast<UT>(lhs) * static_cast<UT>(rhs));
} else {
return lhs * rhs;
}
};
NumberT result;
if constexpr (std::is_same_v<NumberT, AInt> || std::is_same_v<NumberT, AFloat>) {
// Check for over/underflow for abstract values
if (auto r = CheckedMul(a, b)) {
result = r->value;
} else {
AddError("'" + std::to_string(mul_values(a.value, b.value)) +
"' cannot be represented as '" + FriendlyName<NumberT>() + "'",
*current_source);
AddError(OverflowErrorMessage(a, "*", b), *current_source);
return utils::Failure;
}
} else {
auto mul_values = [](T lhs, T rhs) {
if constexpr (std::is_integral_v<T> && std::is_signed_v<T>) {
// For signed integrals, avoid C++ UB by multiplying as unsigned
using UT = std::make_unsigned_t<T>;
return static_cast<T>(static_cast<UT>(lhs) * static_cast<UT>(rhs));
} else {
return lhs * rhs;
}
};
result = mul_values(a.value, b.value);
}
return result;
@ -966,37 +970,32 @@ ConstEval::ConstantResult ConstEval::OpPlus(const sem::Type*,
return r;
}
ConstEval::ConstantResult ConstEval::OpMinus(const sem::Type* ty,
ConstEval::ConstantResult ConstEval::OpMinus(const sem::Type*,
utils::VectorRef<const sem::Constant*> args,
const Source& source) {
auto transform = [&](const sem::Constant* c0, const sem::Constant* c1) {
auto create = [&](auto i, auto j) -> const Constant* {
using NumberT = decltype(i);
using T = UnwrapNumber<NumberT>;
auto subtract_values = [](T lhs, T rhs) {
if constexpr (std::is_integral_v<T> && std::is_signed_v<T>) {
// Ensure no UB for signed underflow
using UT = std::make_unsigned_t<T>;
return static_cast<T>(static_cast<UT>(lhs) - static_cast<UT>(rhs));
} else {
return lhs - rhs;
}
};
NumberT result;
if constexpr (std::is_same_v<NumberT, AInt> || std::is_same_v<NumberT, AFloat>) {
// Check for over/underflow for abstract values
if (auto r = CheckedSub(i, j)) {
result = r->value;
} else {
AddError("'" + std::to_string(subtract_values(i.value, j.value)) +
"' cannot be represented as '" +
ty->FriendlyName(builder.Symbols()) + "'",
source);
AddError(OverflowErrorMessage(i, "-", j), source);
return nullptr;
}
} else {
using T = UnwrapNumber<NumberT>;
auto subtract_values = [](T lhs, T rhs) {
if constexpr (std::is_integral_v<T> && std::is_signed_v<T>) {
// Ensure no UB for signed underflow
using UT = std::make_unsigned_t<T>;
return static_cast<T>(static_cast<UT>(lhs) - static_cast<UT>(rhs));
} else {
return lhs - rhs;
}
};
result = subtract_values(i.value, j.value);
}
return CreateElement(builder, c0->Type(), result);

View File

@ -3515,7 +3515,6 @@ struct OverflowCase {
ast::BinaryOp op;
Types lhs;
Types rhs;
std::string overflowed_result;
};
static std::ostream& operator<<(std::ostream& o, const OverflowCase& c) {
@ -3539,35 +3538,32 @@ TEST_P(ResolverConstEvalBinaryOpTest_Overflow, Test) {
},
c.lhs);
EXPECT_THAT(r()->error(), HasSubstr("1:1 error: '" + c.overflowed_result +
"' cannot be represented as '" + type_name + "'"));
EXPECT_THAT(r()->error(), HasSubstr("1:1 error: '"));
EXPECT_THAT(r()->error(), HasSubstr("' cannot be represented as '" + type_name + "'"));
}
INSTANTIATE_TEST_SUITE_P(
Test,
ResolverConstEvalBinaryOpTest_Overflow,
testing::Values( //
// scalar-scalar add
OverflowCase{ast::BinaryOp::kAdd, Val(AInt::Highest()), Val(1_a), "-9223372036854775808"},
OverflowCase{ast::BinaryOp::kAdd, Val(AInt::Lowest()), Val(-1_a), "9223372036854775807"},
OverflowCase{ast::BinaryOp::kAdd, Val(AFloat::Highest()), Val(AFloat::Highest()), "inf"},
OverflowCase{ast::BinaryOp::kAdd, Val(AFloat::Lowest()), Val(AFloat::Lowest()), "-inf"},
testing::Values(
// scalar-scalar add
OverflowCase{ast::BinaryOp::kAdd, Val(AInt::Highest()), Val(1_a)},
OverflowCase{ast::BinaryOp::kAdd, Val(AInt::Lowest()), Val(-1_a)},
OverflowCase{ast::BinaryOp::kAdd, Val(AFloat::Highest()), Val(AFloat::Highest())},
OverflowCase{ast::BinaryOp::kAdd, Val(AFloat::Lowest()), Val(AFloat::Lowest())},
// scalar-scalar subtract
OverflowCase{ast::BinaryOp::kSubtract, Val(AInt::Lowest()), Val(1_a),
"9223372036854775807"},
OverflowCase{ast::BinaryOp::kSubtract, Val(AInt::Highest()), Val(-1_a),
"-9223372036854775808"},
OverflowCase{ast::BinaryOp::kSubtract, Val(AFloat::Highest()), Val(AFloat::Lowest()),
"inf"},
OverflowCase{ast::BinaryOp::kSubtract, Val(AFloat::Lowest()), Val(AFloat::Highest()),
"-inf"},
OverflowCase{ast::BinaryOp::kSubtract, Val(AInt::Lowest()), Val(1_a)},
OverflowCase{ast::BinaryOp::kSubtract, Val(AInt::Highest()), Val(-1_a)},
OverflowCase{ast::BinaryOp::kSubtract, Val(AFloat::Highest()), Val(AFloat::Lowest())},
OverflowCase{ast::BinaryOp::kSubtract, Val(AFloat::Lowest()), Val(AFloat::Highest())},
// scalar-scalar multiply
OverflowCase{ast::BinaryOp::kMultiply, Val(AInt::Highest()), Val(2_a), "-2"},
OverflowCase{ast::BinaryOp::kMultiply, Val(AInt::Lowest()), Val(-2_a), "0"},
OverflowCase{ast::BinaryOp::kMultiply, Val(AInt::Highest()), Val(2_a)},
OverflowCase{ast::BinaryOp::kMultiply, Val(AInt::Lowest()), Val(-2_a)},
// scalar-vector multiply
OverflowCase{ast::BinaryOp::kMultiply, Val(AInt::Highest()), Vec(2_a, 1_a), "-2"},
OverflowCase{ast::BinaryOp::kMultiply, Val(AInt::Lowest()), Vec(-2_a, 1_a), "0"},
OverflowCase{ast::BinaryOp::kMultiply, Val(AInt::Highest()), Vec(2_a, 1_a)},
OverflowCase{ast::BinaryOp::kMultiply, Val(AInt::Lowest()), Vec(-2_a, 1_a)},
// vector-matrix multiply
@ -3577,8 +3573,7 @@ INSTANTIATE_TEST_SUITE_P(
OverflowCase{ast::BinaryOp::kMultiply, //
Vec(AFloat::Highest(), 1.0_a), //
Mat({2.0_a, 1.0_a}, //
{1.0_a, 1.0_a}), //
"inf"},
{1.0_a, 1.0_a})},
// Overflow from second multiplication of dot product of vector and matrix column 0
// i.e. (v[0] * m[0][0] + v[1] * m[0][1])
@ -3586,8 +3581,7 @@ INSTANTIATE_TEST_SUITE_P(
OverflowCase{ast::BinaryOp::kMultiply, //
Vec(1.0_a, AFloat::Highest()), //
Mat({1.0_a, 2.0_a}, //
{1.0_a, 1.0_a}), //
"inf"},
{1.0_a, 1.0_a})},
// Overflow from addition of dot product of vector and matrix column 0
// i.e. (v[0] * m[0][0] + v[1] * m[0][1])
@ -3595,8 +3589,7 @@ INSTANTIATE_TEST_SUITE_P(
OverflowCase{ast::BinaryOp::kMultiply, //
Vec(AFloat::Highest(), AFloat::Highest()), //
Mat({1.0_a, 1.0_a}, //
{1.0_a, 1.0_a}), //
"inf"},
{1.0_a, 1.0_a})},
// matrix-matrix multiply
@ -3607,8 +3600,7 @@ INSTANTIATE_TEST_SUITE_P(
Mat({AFloat::Highest(), 1.0_a}, //
{1.0_a, 1.0_a}), //
Mat({2.0_a, 1.0_a}, //
{1.0_a, 1.0_a}), //
"inf"},
{1.0_a, 1.0_a})},
// Overflow from second multiplication of dot product of lhs row 0 and rhs column 0
// i.e. m1[0][0] * m2[0][0] + m1[0][1] * m[1][0]
@ -3617,8 +3609,7 @@ INSTANTIATE_TEST_SUITE_P(
Mat({1.0_a, AFloat::Highest()}, //
{1.0_a, 1.0_a}), //
Mat({1.0_a, 1.0_a}, //
{2.0_a, 1.0_a}), //
"inf"},
{2.0_a, 1.0_a})},
// Overflow from addition of dot product of lhs row 0 and rhs column 0
// i.e. m1[0][0] * m2[0][0] + m1[0][1] * m[1][0]
@ -3627,8 +3618,7 @@ INSTANTIATE_TEST_SUITE_P(
Mat({AFloat::Highest(), 1.0_a}, //
{AFloat::Highest(), 1.0_a}), //
Mat({1.0_a, 1.0_a}, //
{1.0_a, 1.0_a}), //
"inf"}
{1.0_a, 1.0_a})}
));
@ -3636,26 +3626,29 @@ TEST_F(ResolverConstEvalTest, BinaryAbstractAddOverflow_AInt) {
GlobalConst("c", Add(Source{{1, 1}}, Expr(AInt::Highest()), 1_a));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"1:1 error: '-9223372036854775808' cannot be represented as 'abstract-int'");
"1:1 error: '9223372036854775807 + 1' cannot be represented as 'abstract-int'");
}
TEST_F(ResolverConstEvalTest, BinaryAbstractAddUnderflow_AInt) {
GlobalConst("c", Add(Source{{1, 1}}, Expr(AInt::Lowest()), -1_a));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"1:1 error: '9223372036854775807' cannot be represented as 'abstract-int'");
"1:1 error: '-9223372036854775808 + -1' cannot be represented as 'abstract-int'");
}
TEST_F(ResolverConstEvalTest, BinaryAbstractAddOverflow_AFloat) {
GlobalConst("c", Add(Source{{1, 1}}, Expr(AFloat::Highest()), AFloat::Highest()));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), "1:1 error: 'inf' cannot be represented as 'abstract-float'");
EXPECT_EQ(r()->error(),
"1:1 error: '1.79769e+308 + 1.79769e+308' cannot be represented as 'abstract-float'");
}
TEST_F(ResolverConstEvalTest, BinaryAbstractAddUnderflow_AFloat) {
GlobalConst("c", Add(Source{{1, 1}}, Expr(AFloat::Lowest()), AFloat::Lowest()));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), "1:1 error: '-inf' cannot be represented as 'abstract-float'");
EXPECT_EQ(
r()->error(),
"1:1 error: '-1.79769e+308 + -1.79769e+308' cannot be represented as 'abstract-float'");
}
// Mixed AInt and AFloat args to test implicit conversion to AFloat