From bebb1cb8f143e9c1e8218e8f640f90b59d106837 Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Wed, 7 Apr 2021 16:30:41 +0000 Subject: [PATCH] writer/hlsl: Fix zero-init of vectors float3(0.0f) is not legal HLSL Change-Id: I2fd20e9718f394c896c9515c4c9b88e490b2b758 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46874 Commit-Queue: Ben Clayton Kokoro: Kokoro Reviewed-by: James Price --- src/writer/hlsl/generator_impl.cc | 9 +- .../hlsl/generator_impl_constructor_test.cc | 125 ++++++++++++------ ...rator_impl_variable_decl_statement_test.cc | 2 +- 3 files changed, 95 insertions(+), 41 deletions(-) diff --git a/src/writer/hlsl/generator_impl.cc b/src/writer/hlsl/generator_impl.cc index affba86491..90ffaf332d 100644 --- a/src/writer/hlsl/generator_impl.cc +++ b/src/writer/hlsl/generator_impl.cc @@ -1994,7 +1994,14 @@ bool GeneratorImpl::EmitZeroValue(std::ostream& out, type::Type* type) { } else if (type->Is()) { out << "0u"; } else if (auto* vec = type->As()) { - return EmitZeroValue(out, vec->type()); + for (uint32_t i = 0; i < vec->size(); i++) { + if (i != 0) { + out << ", "; + } + if (!EmitZeroValue(out, vec->type())) { + return false; + } + } } else if (auto* mat = type->As()) { for (uint32_t i = 0; i < (mat->rows() * mat->columns()); i++) { if (i != 0) { diff --git a/src/writer/hlsl/generator_impl_constructor_test.cc b/src/writer/hlsl/generator_impl_constructor_test.cc index 59593542fa..2acbedbb68 100644 --- a/src/writer/hlsl/generator_impl_constructor_test.cc +++ b/src/writer/hlsl/generator_impl_constructor_test.cc @@ -25,94 +25,114 @@ using ::testing::HasSubstr; using HlslGeneratorImplTest_Constructor = TestHelper; TEST_F(HlslGeneratorImplTest_Constructor, EmitConstructor_Bool) { - auto* expr = Expr(false); + WrapInFunction(Expr(false)); GeneratorImpl& gen = Build(); - ASSERT_TRUE(gen.EmitConstructor(pre, out, expr)) << gen.error(); - EXPECT_EQ(result(), "false"); + ASSERT_TRUE(gen.Generate(out)) << gen.error(); + EXPECT_THAT(result(), HasSubstr("false")); + + Validate(); } TEST_F(HlslGeneratorImplTest_Constructor, EmitConstructor_Int) { - auto* expr = Expr(-12345); + WrapInFunction(Expr(-12345)); GeneratorImpl& gen = Build(); - ASSERT_TRUE(gen.EmitConstructor(pre, out, expr)) << gen.error(); - EXPECT_EQ(result(), "-12345"); + ASSERT_TRUE(gen.Generate(out)) << gen.error(); + EXPECT_THAT(result(), HasSubstr("-12345")); + + Validate(); } TEST_F(HlslGeneratorImplTest_Constructor, EmitConstructor_UInt) { - auto* expr = Expr(56779u); + WrapInFunction(Expr(56779u)); GeneratorImpl& gen = Build(); - ASSERT_TRUE(gen.EmitConstructor(pre, out, expr)) << gen.error(); - EXPECT_EQ(result(), "56779u"); + ASSERT_TRUE(gen.Generate(out)) << gen.error(); + EXPECT_THAT(result(), HasSubstr("56779u")); + + Validate(); } TEST_F(HlslGeneratorImplTest_Constructor, EmitConstructor_Float) { // Use a number close to 1<<30 but whose decimal representation ends in 0. - auto* expr = Expr(static_cast((1 << 30) - 4)); + WrapInFunction(Expr(static_cast((1 << 30) - 4))); GeneratorImpl& gen = Build(); - ASSERT_TRUE(gen.EmitConstructor(pre, out, expr)) << gen.error(); - EXPECT_EQ(result(), "1073741824.0f"); + ASSERT_TRUE(gen.Generate(out)) << gen.error(); + EXPECT_THAT(result(), HasSubstr("1073741824.0f")); + + Validate(); } TEST_F(HlslGeneratorImplTest_Constructor, EmitConstructor_Type_Float) { - auto* expr = Construct(-1.2e-5f); + WrapInFunction(Construct(-1.2e-5f)); GeneratorImpl& gen = Build(); - ASSERT_TRUE(gen.EmitConstructor(pre, out, expr)) << gen.error(); - EXPECT_EQ(result(), "float(-0.000012f)"); + ASSERT_TRUE(gen.Generate(out)) << gen.error(); + EXPECT_THAT(result(), HasSubstr("float(-0.000012f)")); + + Validate(); } TEST_F(HlslGeneratorImplTest_Constructor, EmitConstructor_Type_Bool) { - auto* expr = Construct(true); + WrapInFunction(Construct(true)); GeneratorImpl& gen = Build(); - ASSERT_TRUE(gen.EmitConstructor(pre, out, expr)) << gen.error(); - EXPECT_EQ(result(), "bool(true)"); + ASSERT_TRUE(gen.Generate(out)) << gen.error(); + EXPECT_THAT(result(), HasSubstr("bool(true)")); + + Validate(); } TEST_F(HlslGeneratorImplTest_Constructor, EmitConstructor_Type_Int) { - auto* expr = Construct(-12345); + WrapInFunction(Construct(-12345)); GeneratorImpl& gen = Build(); - ASSERT_TRUE(gen.EmitConstructor(pre, out, expr)) << gen.error(); - EXPECT_EQ(result(), "int(-12345)"); + ASSERT_TRUE(gen.Generate(out)) << gen.error(); + EXPECT_THAT(result(), HasSubstr("int(-12345)")); + + Validate(); } TEST_F(HlslGeneratorImplTest_Constructor, EmitConstructor_Type_Uint) { - auto* expr = Construct(12345u); + WrapInFunction(Construct(12345u)); GeneratorImpl& gen = Build(); - ASSERT_TRUE(gen.EmitConstructor(pre, out, expr)) << gen.error(); - EXPECT_EQ(result(), "uint(12345u)"); + ASSERT_TRUE(gen.Generate(out)) << gen.error(); + EXPECT_THAT(result(), HasSubstr("uint(12345u)")); + + Validate(); } TEST_F(HlslGeneratorImplTest_Constructor, EmitConstructor_Type_Vec) { - auto* expr = vec3(1.f, 2.f, 3.f); + WrapInFunction(vec3(1.f, 2.f, 3.f)); GeneratorImpl& gen = Build(); - ASSERT_TRUE(gen.EmitConstructor(pre, out, expr)) << gen.error(); - EXPECT_EQ(result(), "float3(1.0f, 2.0f, 3.0f)"); + ASSERT_TRUE(gen.Generate(out)) << gen.error(); + EXPECT_THAT(result(), HasSubstr("float3(1.0f, 2.0f, 3.0f)")); + + Validate(); } TEST_F(HlslGeneratorImplTest_Constructor, EmitConstructor_Type_Vec_Empty) { - auto* expr = vec3(); + WrapInFunction(vec3()); GeneratorImpl& gen = Build(); - ASSERT_TRUE(gen.EmitConstructor(pre, out, expr)) << gen.error(); - EXPECT_EQ(result(), "float3(0.0f)"); + ASSERT_TRUE(gen.Generate(out)) << gen.error(); + EXPECT_THAT(result(), HasSubstr("float3(0.0f, 0.0f, 0.0f)")); + + Validate(); } TEST_F(HlslGeneratorImplTest_Constructor, EmitConstructor_Type_Mat) { @@ -131,21 +151,48 @@ TEST_F(HlslGeneratorImplTest_Constructor, EmitConstructor_Type_Mat) { Validate(); } -TEST_F(HlslGeneratorImplTest_Constructor, EmitConstructor_Type_Array) { - auto* expr = Construct(ty.array(ty.vec3(), 3), vec3(1.f, 2.f, 3.f), - vec3(4.f, 5.f, 6.f), vec3(7.f, 8.f, 9.f)); +TEST_F(HlslGeneratorImplTest_Constructor, EmitConstructor_Type_Mat_Empty) { + WrapInFunction(mat2x3()); GeneratorImpl& gen = Build(); - ASSERT_TRUE(gen.EmitConstructor(pre, out, expr)) << gen.error(); - EXPECT_EQ(result(), - "{float3(1.0f, 2.0f, 3.0f), float3(4.0f, 5.0f, 6.0f)," - " float3(7.0f, 8.0f, 9.0f)}"); + ASSERT_TRUE(gen.Generate(out)) << gen.error(); + + EXPECT_THAT(result(), + HasSubstr("float2x3(0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f)")); + + Validate(); } -// TODO(dsinclair): Add struct constructor test. +TEST_F(HlslGeneratorImplTest_Constructor, EmitConstructor_Type_Array) { + WrapInFunction(Construct(ty.array(ty.vec3(), 3), + vec3(1.f, 2.f, 3.f), vec3(4.f, 5.f, 6.f), + vec3(7.f, 8.f, 9.f))); + + GeneratorImpl& gen = Build(); + + ASSERT_TRUE(gen.Generate(out)) << gen.error(); + EXPECT_THAT(result(), + HasSubstr("{float3(1.0f, 2.0f, 3.0f), float3(4.0f, 5.0f, 6.0f)," + " float3(7.0f, 8.0f, 9.0f)}")); + + Validate(); +} + +// TODO(bclayton): Zero-init arrays TEST_F(HlslGeneratorImplTest_Constructor, - DISABLED_EmitConstructor_Type_Struct) {} + DISABLED_EmitConstructor_Type_Array_Empty) { + WrapInFunction(Construct(ty.array(ty.vec3(), 3))); + + GeneratorImpl& gen = Build(); + + ASSERT_TRUE(gen.Generate(out)) << gen.error(); + EXPECT_THAT(result(), + HasSubstr("{float3(0.0f, 0.0f, 0.0f), float3(0.0f, 0.0f, 0.0f)," + " float3(0.0f, 0.0f, 0.0f)}")); + + Validate(); +} } // namespace } // namespace hlsl diff --git a/src/writer/hlsl/generator_impl_variable_decl_statement_test.cc b/src/writer/hlsl/generator_impl_variable_decl_statement_test.cc index 5dae91bddd..7fa67cc779 100644 --- a/src/writer/hlsl/generator_impl_variable_decl_statement_test.cc +++ b/src/writer/hlsl/generator_impl_variable_decl_statement_test.cc @@ -115,7 +115,7 @@ TEST_F(HlslGeneratorImplTest_VariableDecl, GeneratorImpl& gen = Build(); ASSERT_TRUE(gen.EmitStatement(out, stmt)) << gen.error(); - EXPECT_EQ(result(), R"(float3 a = float3(0.0f); + EXPECT_EQ(result(), R"(float3 a = float3(0.0f, 0.0f, 0.0f); )"); }