From bc44620d68bc66b1e664269b7365ea162a08b45c Mon Sep 17 00:00:00 2001 From: Antonio Maiorano Date: Wed, 21 Dec 2022 21:15:03 +0000 Subject: [PATCH] tint: implement short-circuiting of const eval bitcast Bug: tint:1581 Change-Id: I6058dee593bf97f9dee202a80ced7b2551da0ba9 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/115220 Kokoro: Kokoro Commit-Queue: Antonio Maiorano Reviewed-by: James Price --- src/tint/resolver/const_eval.cc | 9 ++--- src/tint/resolver/const_eval.h | 5 +-- .../resolver/const_eval_binary_op_test.cc | 36 ++++++++++--------- src/tint/resolver/const_eval_bitcast_test.cc | 3 +- src/tint/resolver/resolver.cc | 28 +++++++-------- 5 files changed, 42 insertions(+), 39 deletions(-) diff --git a/src/tint/resolver/const_eval.cc b/src/tint/resolver/const_eval.cc index 13cb213d47..b52f4aa1a2 100644 --- a/src/tint/resolver/const_eval.cc +++ b/src/tint/resolver/const_eval.cc @@ -1330,13 +1330,10 @@ ConstEval::Result ConstEval::Swizzle(const type::Type* ty, return builder.create(ty, std::move(values)); } -ConstEval::Result ConstEval::Bitcast(const type::Type* ty, const sem::Expression* expr) { - auto* value = expr->ConstantValue(); - if (!value) { - return nullptr; - } +ConstEval::Result ConstEval::Bitcast(const type::Type* ty, + const constant::Value* value, + const Source& source) { auto* el_ty = type::Type::DeepestElementOf(ty); - auto& source = expr->Declaration()->source; auto transform = [&](const constant::Value* c0) { auto create = [&](auto e) { return Switch( diff --git a/src/tint/resolver/const_eval.h b/src/tint/resolver/const_eval.h index d75f5de2d4..dbb1c2e3b9 100644 --- a/src/tint/resolver/const_eval.h +++ b/src/tint/resolver/const_eval.h @@ -80,10 +80,11 @@ class ConstEval { Result ArrayOrStructInit(const type::Type* ty, utils::VectorRef args); /// @param ty the target type - /// @param expr the input expression + /// @param value the value being converted + /// @param source the source location /// @return the bit-cast of the given expression to the given type, or null if the value cannot /// be calculated - Result Bitcast(const type::Type* ty, const sem::Expression* expr); + Result Bitcast(const type::Type* ty, const constant::Value* value, const Source& source); /// @param obj the object being indexed /// @param idx the index expression diff --git a/src/tint/resolver/const_eval_binary_op_test.cc b/src/tint/resolver/const_eval_binary_op_test.cc index e7e306af20..7ea998a593 100644 --- a/src/tint/resolver/const_eval_binary_op_test.cc +++ b/src/tint/resolver/const_eval_binary_op_test.cc @@ -1775,8 +1775,7 @@ TEST_F(ResolverConstEvalTest, ShortCircuit_Or_Error_Index) { // Short-Circuit Bitcast //////////////////////////////////////////////// -// @TODO(crbug.com/tint/1581): Enable once const eval of bitcast is implemented -TEST_F(ResolverConstEvalTest, DISABLED_ShortCircuit_And_Invalid_Bitcast) { +TEST_F(ResolverConstEvalTest, ShortCircuit_And_Invalid_Bitcast) { // const one = 1; // const a = 0x7F800000; // const result = (one == 0) && (bitcast(a) == 0.0); @@ -1791,8 +1790,7 @@ TEST_F(ResolverConstEvalTest, DISABLED_ShortCircuit_And_Invalid_Bitcast) { ValidateAnd(Sem(), binary); } -// @TODO(crbug.com/tint/1581): Enable once const eval of bitcast is implemented -TEST_F(ResolverConstEvalTest, DISABLED_NonShortCircuit_And_Invalid_Bitcast) { +TEST_F(ResolverConstEvalTest, NonShortCircuit_And_Invalid_Bitcast) { // const one = 1; // const a = 0x7F800000; // const result = (one == 1) && (bitcast(a) == 0.0); @@ -1804,11 +1802,10 @@ TEST_F(ResolverConstEvalTest, DISABLED_NonShortCircuit_And_Invalid_Bitcast) { GlobalConst("result", binary); EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ(r()->error(), "12:34 error: value not representable as f32 message here"); + EXPECT_EQ(r()->error(), "12:34 error: value inf cannot be represented as 'f32'"); } -// @TODO(crbug.com/tint/1581): Enable once const eval of bitcast is implemented -TEST_F(ResolverConstEvalTest, DISABLED_ShortCircuit_And_Error_Bitcast) { +TEST_F(ResolverConstEvalTest, ShortCircuit_And_Error_Bitcast) { // const one = 1; // const a = 0x7F800000; // const result = (one == 0) && (bitcast(a) == 0i); @@ -1820,11 +1817,15 @@ TEST_F(ResolverConstEvalTest, DISABLED_ShortCircuit_And_Error_Bitcast) { GlobalConst("result", binary); EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ(r()->error(), R"(12:34 error: no matching overload message here)"); + EXPECT_EQ(r()->error(), R"(12:34 error: no matching overload for operator == (f32, i32) + +2 candidate operators: + operator == (T, T) -> bool where: T is abstract-int, abstract-float, f32, f16, i32, u32 or bool + operator == (vecN, vecN) -> vecN where: T is abstract-int, abstract-float, f32, f16, i32, u32 or bool +)"); } -// @TODO(crbug.com/tint/1581): Enable once const eval of bitcast is implemented -TEST_F(ResolverConstEvalTest, DISABLED_ShortCircuit_Or_Invalid_Bitcast) { +TEST_F(ResolverConstEvalTest, ShortCircuit_Or_Invalid_Bitcast) { // const one = 1; // const a = 0x7F800000; // const result = (one == 1) || (bitcast(a) == 0.0); @@ -1839,8 +1840,7 @@ TEST_F(ResolverConstEvalTest, DISABLED_ShortCircuit_Or_Invalid_Bitcast) { ValidateOr(Sem(), binary); } -// @TODO(crbug.com/tint/1581): Enable once const eval of bitcast is implemented -TEST_F(ResolverConstEvalTest, DISABLED_NonShortCircuit_Or_Invalid_Bitcast) { +TEST_F(ResolverConstEvalTest, NonShortCircuit_Or_Invalid_Bitcast) { // const one = 1; // const a = 0x7F800000; // const result = (one == 0) || (bitcast(a) == 0.0); @@ -1852,11 +1852,10 @@ TEST_F(ResolverConstEvalTest, DISABLED_NonShortCircuit_Or_Invalid_Bitcast) { GlobalConst("result", binary); EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ(r()->error(), "12:34 error: value not representable as f32 message here"); + EXPECT_EQ(r()->error(), "12:34 error: value inf cannot be represented as 'f32'"); } -// @TODO(crbug.com/tint/1581): Enable once const eval of bitcast is implemented -TEST_F(ResolverConstEvalTest, DISABLED_ShortCircuit_Or_Error_Bitcast) { +TEST_F(ResolverConstEvalTest, ShortCircuit_Or_Error_Bitcast) { // const one = 1; // const a = 0x7F800000; // const result = (one == 1) || (bitcast(a) == 0i); @@ -1868,7 +1867,12 @@ TEST_F(ResolverConstEvalTest, DISABLED_ShortCircuit_Or_Error_Bitcast) { GlobalConst("result", binary); EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ(r()->error(), R"(12:34 error: no matching overload message here)"); + EXPECT_EQ(r()->error(), R"(12:34 error: no matching overload for operator == (f32, i32) + +2 candidate operators: + operator == (T, T) -> bool where: T is abstract-int, abstract-float, f32, f16, i32, u32 or bool + operator == (vecN, vecN) -> vecN where: T is abstract-int, abstract-float, f32, f16, i32, u32 or bool +)"); } //////////////////////////////////////////////// diff --git a/src/tint/resolver/const_eval_bitcast_test.cc b/src/tint/resolver/const_eval_bitcast_test.cc index 34fc25e29c..50d4693d87 100644 --- a/src/tint/resolver/const_eval_bitcast_test.cc +++ b/src/tint/resolver/const_eval_bitcast_test.cc @@ -67,7 +67,7 @@ TEST_P(ResolverConstEvalBitcastTest, Test) { auto* target_ty = target_create_ptrs.ast(*this); ASSERT_NE(target_ty, nullptr); auto* input_val = input.Expr(*this); - const ast::Expression* expr = Bitcast(target_ty, input_val); + const ast::Expression* expr = Bitcast(Source{{12, 34}}, target_ty, input_val); WrapInFunction(expr); @@ -87,6 +87,7 @@ TEST_P(ResolverConstEvalBitcastTest, Test) { EXPECT_EQ(expected_values, got_values); } else { ASSERT_FALSE(r()->Resolve()); + EXPECT_THAT(r()->error(), testing::StartsWith("12:34 error:")); EXPECT_THAT(r()->error(), testing::HasSubstr("cannot be represented as")); } } diff --git a/src/tint/resolver/resolver.cc b/src/tint/resolver/resolver.cc index 42910094d1..cec6e5a2c3 100644 --- a/src/tint/resolver/resolver.cc +++ b/src/tint/resolver/resolver.cc @@ -1960,24 +1960,24 @@ sem::Expression* Resolver::Bitcast(const ast::BitcastExpression* expr) { if (!validator_.Bitcast(expr, ty)) { return nullptr; } - // - const constant::Value* val = nullptr; - sem::EvaluationStage stage = sem::EvaluationStage::kRuntime; - // TODO(crbug.com/tint/1582): short circuit 'expr' once const eval of Bitcast is implemented. - if (auto r = const_eval_.Bitcast(ty, inner)) { - val = r.Get(); - if (val) { - stage = sem::EvaluationStage::kConstant; - } - } else { - return nullptr; + auto stage = inner->Stage(); + if (stage == sem::EvaluationStage::kConstant && skip_const_eval_.Contains(expr)) { + stage = sem::EvaluationStage::kNotEvaluated; } + + const constant::Value* value = nullptr; + if (stage == sem::EvaluationStage::kConstant) { + if (auto r = const_eval_.Bitcast(ty, inner->ConstantValue(), expr->source)) { + value = r.Get(); + } else { + return nullptr; + } + } + auto* sem = builder_->create(expr, ty, stage, current_statement_, - std::move(val), inner->HasSideEffects()); - + std::move(value), inner->HasSideEffects()); sem->Behaviors() = inner->Behaviors(); - return sem; }