tint: optimize compile time for const_eval_*_test files The reason for slow compile times is because the very large variants of builder::Value<T>s combined with the many std::visits over these variants result in many combinatorial instantiations of the visit callbacks. To address this, I added a polymorphic base class ValueBase to Value<T>, and replaced most of the std::visit-based compile time code with runtime virtual calls. For the two heaviest users of std::visit over the large variants, compiles times dropped more than half (clang-10, debug): const_eval_binary_op_test.cc: 19.079s to 7.736s const_eval_unary_op_test.cc: 10.021s to 4.789s Bug: tint:1711 Change-Id: Iba05e6ae1004ef0814250e2a8ea50aa2b26b85f2 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/105782 Reviewed-by: Ben Clayton <bclayton@google.com> Kokoro: Kokoro <noreply+kokoro@google.com> Commit-Queue: Antonio Maiorano <amaiorano@google.com>
diff --git a/src/tint/resolver/const_eval_binary_op_test.cc b/src/tint/resolver/const_eval_binary_op_test.cc index 18b28da..5f43499 100644 --- a/src/tint/resolver/const_eval_binary_op_test.cc +++ b/src/tint/resolver/const_eval_binary_op_test.cc
@@ -54,47 +54,39 @@ auto op = std::get<0>(GetParam()); auto& c = std::get<1>(GetParam()); - std::visit( - [&](auto&& expected) { - using T = typename std::decay_t<decltype(expected)>::ElementType; - if constexpr (std::is_same_v<T, AInt> || std::is_same_v<T, AFloat>) { - if (c.overflow) { - // Overflow is not allowed for abstract types. This is tested separately. - return; - } - } + auto* expected = ToValueBase(c.expected); + if (expected->IsAbstract() && c.overflow) { + // Overflow is not allowed for abstract types. This is tested separately. + return; + } - auto* lhs_expr = std::visit([&](auto&& value) { return value.Expr(*this); }, c.lhs); - auto* rhs_expr = std::visit([&](auto&& value) { return value.Expr(*this); }, c.rhs); - auto* expr = create<ast::BinaryExpression>(op, lhs_expr, rhs_expr); + auto* lhs = ToValueBase(c.lhs); + auto* rhs = ToValueBase(c.rhs); - GlobalConst("C", expr); - auto* expected_expr = expected.Expr(*this); - GlobalConst("E", expected_expr); - ASSERT_TRUE(r()->Resolve()) << r()->error(); + auto* lhs_expr = lhs->Expr(*this); + auto* rhs_expr = rhs->Expr(*this); + auto* expr = create<ast::BinaryExpression>(op, lhs_expr, rhs_expr); + GlobalConst("C", expr); + ASSERT_TRUE(r()->Resolve()) << r()->error(); - auto* sem = Sem().Get(expr); - const sem::Constant* value = sem->ConstantValue(); - ASSERT_NE(value, nullptr); - EXPECT_TYPE(value->Type(), sem->Type()); + auto* sem = Sem().Get(expr); + const sem::Constant* value = sem->ConstantValue(); + ASSERT_NE(value, nullptr); + EXPECT_TYPE(value->Type(), sem->Type()); - auto* expected_sem = Sem().Get(expected_expr); - const sem::Constant* expected_value = expected_sem->ConstantValue(); - ASSERT_NE(expected_value, nullptr); - EXPECT_TYPE(expected_value->Type(), expected_sem->Type()); - - ForEachElemPair(value, expected_value, - [&](const sem::Constant* a, const sem::Constant* b) { - EXPECT_EQ(a->As<T>(), b->As<T>()); - if constexpr (IsIntegral<T>) { - // Check that the constant's integer doesn't contain unexpected - // data in the MSBs that are outside of the bit-width of T. - EXPECT_EQ(a->As<AInt>(), b->As<AInt>()); - } - return HasFailure() ? Action::kStop : Action::kContinue; - }); - }, - c.expected); + auto values_flat = ScalarArgsFrom(value); + auto expected_values_flat = expected->Args(); + ASSERT_EQ(values_flat.values.Length(), expected_values_flat.values.Length()); + for (size_t i = 0; i < values_flat.values.Length(); ++i) { + auto& a = values_flat.values[i]; + auto& b = expected_values_flat.values[i]; + EXPECT_EQ(a, b); + if (expected->IsIntegral()) { + // Check that the constant's integer doesn't contain unexpected + // data in the MSBs that are outside of the bit-width of T. + EXPECT_EQ(builder::As<AInt>(a), builder::As<AInt>(b)); + } + } } INSTANTIATE_TEST_SUITE_P(MixedAbstractArgs, @@ -658,21 +650,15 @@ TEST_P(ResolverConstEvalBinaryOpTest_Overflow, Test) { Enable(ast::Extension::kF16); auto& c = GetParam(); - auto* lhs_expr = std::visit([&](auto&& value) { return value.Expr(*this); }, c.lhs); - auto* rhs_expr = std::visit([&](auto&& value) { return value.Expr(*this); }, c.rhs); + auto* lhs = ToValueBase(c.lhs); + auto* rhs = ToValueBase(c.rhs); + auto* lhs_expr = lhs->Expr(*this); + auto* rhs_expr = rhs->Expr(*this); auto* expr = create<ast::BinaryExpression>(Source{{1, 1}}, c.op, lhs_expr, rhs_expr); GlobalConst("C", expr); ASSERT_FALSE(r()->Resolve()); - - std::string type_name = std::visit( - [&](auto&& value) { - using ValueType = std::decay_t<decltype(value)>; - return builder::FriendlyName<ValueType>(); - }, - c.lhs); - EXPECT_THAT(r()->error(), HasSubstr("1:1 error: '")); - EXPECT_THAT(r()->error(), HasSubstr("' cannot be represented as '" + type_name + "'")); + EXPECT_THAT(r()->error(), HasSubstr("' cannot be represented as '" + lhs->TypeName() + "'")); } INSTANTIATE_TEST_SUITE_P( Test, @@ -854,10 +840,8 @@ using ResolverConstEvalShiftLeftConcreteGeqBitWidthError = ResolverTestWithParam<std::tuple<Types, Types>>; TEST_P(ResolverConstEvalShiftLeftConcreteGeqBitWidthError, Test) { - auto* lhs_expr = - std::visit([&](auto&& value) { return value.Expr(*this); }, std::get<0>(GetParam())); - auto* rhs_expr = - std::visit([&](auto&& value) { return value.Expr(*this); }, std::get<1>(GetParam())); + auto* lhs_expr = ToValueBase(std::get<0>(GetParam()))->Expr(*this); + auto* rhs_expr = ToValueBase(std::get<1>(GetParam()))->Expr(*this); GlobalConst("c", Shl(Source{{1, 1}}, lhs_expr, rhs_expr)); EXPECT_FALSE(r()->Resolve()); EXPECT_EQ( @@ -880,10 +864,8 @@ // AInt left shift results in sign change error using ResolverConstEvalShiftLeftSignChangeError = ResolverTestWithParam<std::tuple<Types, Types>>; TEST_P(ResolverConstEvalShiftLeftSignChangeError, Test) { - auto* lhs_expr = - std::visit([&](auto&& value) { return value.Expr(*this); }, std::get<0>(GetParam())); - auto* rhs_expr = - std::visit([&](auto&& value) { return value.Expr(*this); }, std::get<1>(GetParam())); + auto* lhs_expr = ToValueBase(std::get<0>(GetParam()))->Expr(*this); + auto* rhs_expr = ToValueBase(std::get<1>(GetParam()))->Expr(*this); GlobalConst("c", Shl(Source{{1, 1}}, lhs_expr, rhs_expr)); EXPECT_FALSE(r()->Resolve()); EXPECT_EQ(r()->error(), "1:1 error: shift left operation results in sign change");
diff --git a/src/tint/resolver/const_eval_builtin_test.cc b/src/tint/resolver/const_eval_builtin_test.cc index 3936fba..86223ba 100644 --- a/src/tint/resolver/const_eval_builtin_test.cc +++ b/src/tint/resolver/const_eval_builtin_test.cc
@@ -83,54 +83,57 @@ std::visit([&](auto&& v) { args.Push(v.Expr(*this)); }, a); } - std::visit( - [&](auto&& expected) { - using T = typename std::decay_t<decltype(expected)>::ElementType; - auto* expr = Call(sem::str(builtin), std::move(args)); + auto* expected = ToValueBase(c.expected); + auto* expr = Call(sem::str(builtin), std::move(args)); - GlobalConst("C", expr); - auto* expected_expr = expected.Expr(*this); - GlobalConst("E", expected_expr); + GlobalConst("C", expr); + auto* expected_expr = expected->Expr(*this); + GlobalConst("E", expected_expr); - EXPECT_TRUE(r()->Resolve()) << r()->error(); + EXPECT_TRUE(r()->Resolve()) << r()->error(); - auto* sem = Sem().Get(expr); - const sem::Constant* value = sem->ConstantValue(); - ASSERT_NE(value, nullptr); - EXPECT_TYPE(value->Type(), sem->Type()); + auto* sem = Sem().Get(expr); + const sem::Constant* value = sem->ConstantValue(); + ASSERT_NE(value, nullptr); + EXPECT_TYPE(value->Type(), sem->Type()); - auto* expected_sem = Sem().Get(expected_expr); - const sem::Constant* expected_value = expected_sem->ConstantValue(); - ASSERT_NE(expected_value, nullptr); - EXPECT_TYPE(expected_value->Type(), expected_sem->Type()); + auto* expected_sem = Sem().Get(expected_expr); + const sem::Constant* expected_value = expected_sem->ConstantValue(); + ASSERT_NE(expected_value, nullptr); + EXPECT_TYPE(expected_value->Type(), expected_sem->Type()); - ForEachElemPair(value, expected_value, - [&](const sem::Constant* a, const sem::Constant* b) { - auto v = a->As<T>(); - auto e = b->As<T>(); - if constexpr (std::is_same_v<bool, T>) { - EXPECT_EQ(v, e); - } else if constexpr (IsFloatingPoint<T>) { - if (std::isnan(e)) { - EXPECT_TRUE(std::isnan(v)); - } else { - auto vf = (c.expected_pos_or_neg ? Abs(v) : v); - if (c.float_compare) { - EXPECT_FLOAT_EQ(vf, e); - } else { - EXPECT_EQ(vf, e); - } - } - } else { - EXPECT_EQ((c.expected_pos_or_neg ? Abs(v) : v), e); - // Check that the constant's integer doesn't contain unexpected - // data in the MSBs that are outside of the bit-width of T. - EXPECT_EQ(a->As<AInt>(), b->As<AInt>()); - } - return HasFailure() ? Action::kStop : Action::kContinue; - }); - }, - c.expected); + // @TODO(amaiorano): Rewrite using ScalarArgsFrom() + ForEachElemPair(value, expected_value, [&](const sem::Constant* a, const sem::Constant* b) { + std::visit( + [&](auto&& ct_expected) { + using T = typename std::decay_t<decltype(ct_expected)>::ElementType; + + auto v = a->As<T>(); + auto e = b->As<T>(); + if constexpr (std::is_same_v<bool, T>) { + EXPECT_EQ(v, e); + } else if constexpr (IsFloatingPoint<T>) { + if (std::isnan(e)) { + EXPECT_TRUE(std::isnan(v)); + } else { + auto vf = (c.expected_pos_or_neg ? Abs(v) : v); + if (c.float_compare) { + EXPECT_FLOAT_EQ(vf, e); + } else { + EXPECT_EQ(vf, e); + } + } + } else { + EXPECT_EQ((c.expected_pos_or_neg ? Abs(v) : v), e); + // Check that the constant's integer doesn't contain unexpected + // data in the MSBs that are outside of the bit-width of T. + EXPECT_EQ(a->As<AInt>(), b->As<AInt>()); + } + }, + c.expected); + + return HasFailure() ? Action::kStop : Action::kContinue; + }); } INSTANTIATE_TEST_SUITE_P( //
diff --git a/src/tint/resolver/const_eval_conversion_test.cc b/src/tint/resolver/const_eval_conversion_test.cc index 35657ee..7e6a6fc 100644 --- a/src/tint/resolver/const_eval_conversion_test.cc +++ b/src/tint/resolver/const_eval_conversion_test.cc
@@ -29,20 +29,7 @@ builder::Value<bool>>; static std::ostream& operator<<(std::ostream& o, const Scalar& scalar) { - std::visit( - [&](auto&& v) { - using ValueType = std::decay_t<decltype(v)>; - o << ValueType::DataType::Name() << "("; - for (auto& a : v.args.values) { - o << std::get<typename ValueType::ElementType>(a); - if (&a != &v.args.values.Back()) { - o << ", "; - } - } - o << ")"; - }, - scalar); - return o; + return ToValueBase(scalar)->Print(o); } enum class Kind { @@ -96,7 +83,7 @@ const auto& type = std::get<1>(GetParam()).type; const auto unrepresentable = std::get<1>(GetParam()).unrepresentable; - auto* input_val = std::visit([&](auto val) { return val.Expr(*this); }, input); + auto* input_val = ToValueBase(input)->Expr(*this); auto* expr = Construct(type.ast(*this), input_val); if (kind == Kind::kVector) { expr = Construct(ty.vec(nullptr, 3), expr); @@ -120,7 +107,7 @@ ASSERT_NE(sem->ConstantValue(), nullptr); EXPECT_TYPE(sem->ConstantValue()->Type(), target_sem_ty); - auto expected_values = std::visit([&](auto&& val) { return val.args; }, expected); + auto expected_values = ToValueBase(expected)->Args(); if (kind == Kind::kVector) { expected_values.values.Push(expected_values.values[0]); expected_values.values.Push(expected_values.values[0]);
diff --git a/src/tint/resolver/const_eval_test.h b/src/tint/resolver/const_eval_test.h index 3840540..2761daf 100644 --- a/src/tint/resolver/const_eval_test.h +++ b/src/tint/resolver/const_eval_test.h
@@ -41,6 +41,8 @@ inline void CollectScalarArgs(const sem::Constant* c, builder::ScalarArgs& args) { Switch( c->Type(), // + [&](const sem::AbstractInt*) { args.values.Push(c->As<AInt>()); }, + [&](const sem::AbstractFloat*) { args.values.Push(c->As<AFloat>()); }, [&](const sem::Bool*) { args.values.Push(c->As<bool>()); }, [&](const sem::I32*) { args.values.Push(c->As<i32>()); }, [&](const sem::U32*) { args.values.Push(c->As<u32>()); }, @@ -136,6 +138,7 @@ using builder::Mat; using builder::Val; using builder::Value; +using builder::ValueBase; using builder::Vec; using Types = std::variant< // @@ -188,21 +191,18 @@ // >; +/// Returns the current Value<T> in the `types` variant as a `ValueBase` pointer to use the +/// polymorphic API. This trades longer compile times using std::variant for longer runtime via +/// virtual function calls. +template <typename ValueVariant> +inline const ValueBase* ToValueBase(const ValueVariant& types) { + return std::visit( + [](auto&& t) -> const ValueBase* { return static_cast<const ValueBase*>(&t); }, types); +} + +/// Prints Types to ostream inline std::ostream& operator<<(std::ostream& o, const Types& types) { - std::visit( - [&](auto&& v) { - using ValueType = std::decay_t<decltype(v)>; - o << ValueType::DataType::Name() << "("; - for (auto& a : v.args.values) { - o << std::get<typename ValueType::ElementType>(a); - if (&a != &v.args.values.Back()) { - o << ", "; - } - } - o << ")"; - }, - types); - return o; + return ToValueBase(types)->Print(o); } // Calls `f` on deepest elements of both `a` and `b`. If function returns Action::kStop, it stops
diff --git a/src/tint/resolver/const_eval_unary_op_test.cc b/src/tint/resolver/const_eval_unary_op_test.cc index 80b8caa..fced490 100644 --- a/src/tint/resolver/const_eval_unary_op_test.cc +++ b/src/tint/resolver/const_eval_unary_op_test.cc
@@ -51,40 +51,34 @@ auto op = std::get<0>(GetParam()); auto& c = std::get<1>(GetParam()); - std::visit( - [&](auto&& expected) { - using T = typename std::decay_t<decltype(expected)>::ElementType; - auto* input_expr = std::visit([&](auto&& value) { return value.Expr(*this); }, c.input); - auto* expr = create<ast::UnaryOpExpression>(op, input_expr); + auto* expected = ToValueBase(c.expected); + auto* input = ToValueBase(c.input); - GlobalConst("C", expr); - auto* expected_expr = expected.Expr(*this); - GlobalConst("E", expected_expr); - ASSERT_TRUE(r()->Resolve()) << r()->error(); + auto* input_expr = input->Expr(*this); + auto* expr = create<ast::UnaryOpExpression>(op, input_expr); - auto* sem = Sem().Get(expr); - const sem::Constant* value = sem->ConstantValue(); - ASSERT_NE(value, nullptr); - EXPECT_TYPE(value->Type(), sem->Type()); + GlobalConst("C", expr); + ASSERT_TRUE(r()->Resolve()) << r()->error(); - auto* expected_sem = Sem().Get(expected_expr); - const sem::Constant* expected_value = expected_sem->ConstantValue(); - ASSERT_NE(expected_value, nullptr); - EXPECT_TYPE(expected_value->Type(), expected_sem->Type()); + auto* sem = Sem().Get(expr); + const sem::Constant* value = sem->ConstantValue(); + ASSERT_NE(value, nullptr); + EXPECT_TYPE(value->Type(), sem->Type()); - ForEachElemPair(value, expected_value, - [&](const sem::Constant* a, const sem::Constant* b) { - EXPECT_EQ(a->As<T>(), b->As<T>()); - if constexpr (IsIntegral<T>) { - // Check that the constant's integer doesn't contain unexpected - // data in the MSBs that are outside of the bit-width of T. - EXPECT_EQ(a->As<AInt>(), b->As<AInt>()); - } - return HasFailure() ? Action::kStop : Action::kContinue; - }); - }, - c.expected); + auto values_flat = ScalarArgsFrom(value); + auto expected_values_flat = expected->Args(); + ASSERT_EQ(values_flat.values.Length(), expected_values_flat.values.Length()); + for (size_t i = 0; i < values_flat.values.Length(); ++i) { + auto& a = values_flat.values[i]; + auto& b = expected_values_flat.values[i]; + EXPECT_EQ(a, b); + if (expected->IsIntegral()) { + // Check that the constant's integer doesn't contain unexpected + // data in the MSBs that are outside of the bit-width of T. + EXPECT_EQ(builder::As<AInt>(a), builder::As<AInt>(b)); + } + } } INSTANTIATE_TEST_SUITE_P(Complement, ResolverConstEvalUnaryOpTest,
diff --git a/src/tint/resolver/resolver_test_helper.h b/src/tint/resolver/resolver_test_helper.h index 923f3ff..57fe14a5 100644 --- a/src/tint/resolver/resolver_test_helper.h +++ b/src/tint/resolver/resolver_test_helper.h
@@ -206,6 +206,12 @@ utils::Vector<Storage, 16> values; }; +/// Returns current variant value in `s` cast to type `T` +template <typename T> +T As(ScalarArgs::Storage& s) { + return std::visit([](auto&& v) { return static_cast<T>(v); }, s); +} + /// @param o the std::ostream to write to /// @param args the ScalarArgs /// @return the std::ostream so calls can be chained @@ -750,10 +756,45 @@ DataType<T>::Name}; } +/// Base class for Value<T> +struct ValueBase { + /// Constructor + ValueBase() = default; + /// Destructor + virtual ~ValueBase() = default; + /// Move constructor + ValueBase(ValueBase&&) = default; + /// Copy constructor + ValueBase(const ValueBase&) = default; + /// Copy assignment operator + /// @returns this instance + ValueBase& operator=(const ValueBase&) = default; + /// Creates an `ast::Expression` for the type T passing in previously stored args + /// @param b the ProgramBuilder + /// @returns an expression node + virtual const ast::Expression* Expr(ProgramBuilder& b) const = 0; + /// @returns args used to create expression via `Expr` + virtual const ScalarArgs& Args() const = 0; + /// @returns true if element type is abstract + virtual bool IsAbstract() const = 0; + /// @returns true if element type is an integral + virtual bool IsIntegral() const = 0; + /// @returns element type name + virtual std::string TypeName() const = 0; + /// Prints this value to the output stream + /// @param o the output stream + /// @returns input argument `o` + virtual std::ostream& Print(std::ostream& o) const = 0; +}; + /// Value<T> is an instance of a value of type DataType<T>. Useful for storing values to create /// expressions with. template <typename T> -struct Value { +struct Value : ValueBase { + /// Constructor + /// @param a the scalar args + explicit Value(ScalarArgs a) : args(std::move(a)) {} + /// Alias to T using Type = T; /// Alias to DataType<T> @@ -764,15 +805,43 @@ /// Creates a Value<T> with `args` /// @param args the args that will be passed to the expression /// @returns a Value<T> - static Value Create(ScalarArgs args) { return Value{CreatePtrsFor<T>(), std::move(args)}; } + static Value Create(ScalarArgs args) { return Value{std::move(args)}; } /// Creates an `ast::Expression` for the type T passing in previously stored args /// @param b the ProgramBuilder /// @returns an expression node - const ast::Expression* Expr(ProgramBuilder& b) const { return (*create.expr)(b, args); } + const ast::Expression* Expr(ProgramBuilder& b) const override { + auto create = CreatePtrsFor<T>(); + return (*create.expr)(b, args); + } - /// functions to create values / types of the value - CreatePtrs create; + /// @returns args used to create expression via `Expr` + const ScalarArgs& Args() const override { return args; } + + /// @returns true if element type is abstract + bool IsAbstract() const override { return tint::IsAbstract<ElementType>; } + + /// @returns true if element type is an integral + bool IsIntegral() const override { return tint::IsIntegral<ElementType>; } + + /// @returns element type name + std::string TypeName() const override { return tint::FriendlyName<ElementType>(); } + + /// Prints this value to the output stream + /// @param o the output stream + /// @returns input argument `o` + std::ostream& Print(std::ostream& o) const override { + o << TypeName() << "("; + for (auto& a : args.values) { + o << std::get<ElementType>(a); + if (&a != &args.values.Back()) { + o << ", "; + } + } + o << ")"; + return o; + } + /// args to create expression with ScalarArgs args; };