From 2f04dc94ce25e91e40dc088d787e148bd499ba65 Mon Sep 17 00:00:00 2001 From: James Price Date: Thu, 1 Apr 2021 22:07:37 +0000 Subject: [PATCH] [validator] Remove requirement to have an entry point The SPIR-V and HLSL sanitizing transforms add an empty one if necessary. Fixed: tint:679 Change-Id: Ic98ff3109d7381b1fbc2de68d95d57e15c7a67c0 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46700 Commit-Queue: Ben Clayton Auto-Submit: James Price Reviewed-by: Ben Clayton --- src/transform/hlsl.cc | 13 ++++++++++++ src/transform/hlsl.h | 2 ++ src/transform/hlsl_test.cc | 14 +++++++++++++ src/transform/spirv.cc | 13 ++++++++++++ src/transform/spirv.h | 2 ++ src/transform/spirv_test.cc | 14 +++++++++++++ src/validator/validator_function_test.cc | 25 ++---------------------- src/validator/validator_impl.cc | 8 -------- src/validator/validator_test.cc | 2 +- 9 files changed, 61 insertions(+), 32 deletions(-) diff --git a/src/transform/hlsl.cc b/src/transform/hlsl.cc index 0a08e24a30..918d6e3ddb 100644 --- a/src/transform/hlsl.cc +++ b/src/transform/hlsl.cc @@ -16,6 +16,7 @@ #include +#include "src/ast/stage_decoration.h" #include "src/ast/variable_decl_statement.h" #include "src/program_builder.h" #include "src/semantic/expression.h" @@ -32,6 +33,7 @@ Transform::Output Hlsl::Run(const Program* in, const DataMap&) { ProgramBuilder out; CloneContext ctx(&out, in); PromoteArrayInitializerToConstVar(ctx); + AddEmptyEntryPoint(ctx); ctx.Clone(); return Output{Program(std::move(out))}; } @@ -105,5 +107,16 @@ void Hlsl::PromoteArrayInitializerToConstVar(CloneContext& ctx) const { } } +void Hlsl::AddEmptyEntryPoint(CloneContext& ctx) const { + for (auto* func : ctx.src->AST().Functions()) { + if (func->IsEntryPoint()) { + return; + } + } + ctx.dst->Func( + "_tint_unused_entry_point", {}, ctx.dst->ty.void_(), {}, + {ctx.dst->create(ast::PipelineStage::kVertex)}); +} + } // namespace transform } // namespace tint diff --git a/src/transform/hlsl.h b/src/transform/hlsl.h index 53ad35f235..df903a75dd 100644 --- a/src/transform/hlsl.h +++ b/src/transform/hlsl.h @@ -44,6 +44,8 @@ class Hlsl : public Transform { /// the array usage statement. /// See crbug.com/tint/406 for more details void PromoteArrayInitializerToConstVar(CloneContext& ctx) const; + /// Add an empty shader entry point if none exist in the module. + void AddEmptyEntryPoint(CloneContext& ctx) const; }; } // namespace transform diff --git a/src/transform/hlsl_test.cc b/src/transform/hlsl_test.cc index bc6b264b46..4ce2259350 100644 --- a/src/transform/hlsl_test.cc +++ b/src/transform/hlsl_test.cc @@ -143,6 +143,20 @@ fn main() -> void { EXPECT_EQ(expect, str(got)); } +TEST_F(HlslTest, AddEmptyEntryPoint) { + auto* src = R"()"; + + auto* expect = R"( +[[stage(vertex)]] +fn _tint_unused_entry_point() -> void { +} +)"; + + auto got = Run(src); + + EXPECT_EQ(expect, str(got)); +} + } // namespace } // namespace transform } // namespace tint diff --git a/src/transform/spirv.cc b/src/transform/spirv.cc index d29cfa4460..bbbff3f4f7 100644 --- a/src/transform/spirv.cc +++ b/src/transform/spirv.cc @@ -19,6 +19,7 @@ #include "src/ast/call_statement.h" #include "src/ast/return_statement.h" +#include "src/ast/stage_decoration.h" #include "src/program_builder.h" #include "src/semantic/function.h" #include "src/semantic/statement.h" @@ -42,6 +43,7 @@ Transform::Output Spirv::Run(const Program* in, const DataMap&) { ProgramBuilder out2; CloneContext ctx2(&out2, &tmp); HandleSampleMaskBuiltins(ctx2); + AddEmptyEntryPoint(ctx2); ctx2.Clone(); return Output{Program(std::move(out2))}; @@ -234,6 +236,17 @@ void Spirv::HandleSampleMaskBuiltins(CloneContext& ctx) const { } } +void Spirv::AddEmptyEntryPoint(CloneContext& ctx) const { + for (auto* func : ctx.src->AST().Functions()) { + if (func->IsEntryPoint()) { + return; + } + } + ctx.dst->Func( + "_tint_unused_entry_point", {}, ctx.dst->ty.void_(), {}, + {ctx.dst->create(ast::PipelineStage::kCompute)}); +} + Symbol Spirv::HoistToInputVariables( CloneContext& ctx, const ast::Function* func, diff --git a/src/transform/spirv.h b/src/transform/spirv.h index 40bc54ee8d..7ac53e1699 100644 --- a/src/transform/spirv.h +++ b/src/transform/spirv.h @@ -47,6 +47,8 @@ class Spirv : public Transform { void HandleEntryPointIOTypes(CloneContext& ctx) const; /// Change type of sample mask builtin variables to single element arrays. void HandleSampleMaskBuiltins(CloneContext& ctx) const; + /// Add an empty shader entry point if none exist in the module. + void AddEmptyEntryPoint(CloneContext& ctx) const; /// Recursively create module-scope input variables for `ty` and add /// function-scope variables for structs to `func`. diff --git a/src/transform/spirv_test.cc b/src/transform/spirv_test.cc index 9df9ed29f1..bb21aea0c9 100644 --- a/src/transform/spirv_test.cc +++ b/src/transform/spirv_test.cc @@ -833,6 +833,20 @@ fn main() -> void { EXPECT_EQ(expect, str(got)); } +TEST_F(SpirvTest, AddEmptyEntryPoint) { + auto* src = R"()"; + + auto* expect = R"( +[[stage(compute)]] +fn _tint_unused_entry_point() -> void { +} +)"; + + auto got = Run(src); + + EXPECT_EQ(expect, str(got)); +} + // Test that different transforms within the sanitizer interact correctly. TEST_F(SpirvTest, MultipleTransforms) { auto* src = R"( diff --git a/src/validator/validator_function_test.cc b/src/validator/validator_function_test.cc index d74e10377f..0d3d54972c 100644 --- a/src/validator/validator_function_test.cc +++ b/src/validator/validator_function_test.cc @@ -78,25 +78,7 @@ TEST_F(ValidateFunctionTest, PipelineStage_MustBeUnique_Fail) { "12:34 v-0020: only one stage decoration permitted per entry point"); } -TEST_F(ValidateFunctionTest, OnePipelineStageFunctionMustBePresent_Pass) { - // [[stage(vertex)]] - // fn vtx_func() -> void { return; } - - Func("vtx_func", ast::VariableList{}, ty.void_(), - ast::StatementList{ - create(), - }, - ast::DecorationList{ - create(ast::PipelineStage::kVertex), - }); - - ValidatorImpl& v = Build(); - - EXPECT_TRUE(v.Validate()) << v.error(); -} - -TEST_F(ValidateFunctionTest, OnePipelineStageFunctionMustBePresent_Fail) { - // fn vtx_func() -> void { return; } +TEST_F(ValidateFunctionTest, NoPipelineEntryPoints) { Func("vtx_func", ast::VariableList{}, ty.void_(), ast::StatementList{ create(), @@ -105,10 +87,7 @@ TEST_F(ValidateFunctionTest, OnePipelineStageFunctionMustBePresent_Fail) { ValidatorImpl& v = Build(); - EXPECT_FALSE(v.Validate()); - EXPECT_EQ(v.error(), - "v-0003: At least one of vertex, fragment or compute shader must " - "be present"); + EXPECT_TRUE(v.Validate()) << v.error(); } TEST_F(ValidateFunctionTest, FunctionVarInitWithParam) { diff --git a/src/validator/validator_impl.cc b/src/validator/validator_impl.cc index 6b109219e6..acc10faf0b 100644 --- a/src/validator/validator_impl.cc +++ b/src/validator/validator_impl.cc @@ -138,10 +138,8 @@ bool ValidatorImpl::ValidateGlobalVariable(const ast::Variable* var) { } bool ValidatorImpl::ValidateEntryPoint(const ast::FunctionList& funcs) { - auto shader_is_present = false; for (auto* func : funcs) { if (func->IsEntryPoint()) { - shader_is_present = true; auto stage_deco_count = 0; for (auto* deco : func->decorations()) { if (deco->Is()) { @@ -158,12 +156,6 @@ bool ValidatorImpl::ValidateEntryPoint(const ast::FunctionList& funcs) { } } } - if (!shader_is_present) { - add_error(Source{}, "v-0003", - "At least one of vertex, fragment or compute shader must " - "be present"); - return false; - } return true; } diff --git a/src/validator/validator_test.cc b/src/validator/validator_test.cc index c79eff1d75..4a69b34a67 100644 --- a/src/validator/validator_test.cc +++ b/src/validator/validator_test.cc @@ -65,7 +65,7 @@ TEST_F(ValidatorTest, GlobalConstNoStorageClass_Pass) { ValidatorImpl& v = Build(); - EXPECT_FALSE(v.Validate()) << v.error(); + EXPECT_TRUE(v.Validate()) << v.error(); } TEST_F(ValidatorTest, GlobalVariableUnique_Pass) {