From 7f142c449fb17aa0e47375785b46528a0f85b15d Mon Sep 17 00:00:00 2001 From: James Price Date: Wed, 14 Jul 2021 13:23:45 +0000 Subject: [PATCH] inspector: Remove legacy shader IO support This is a reland of the CL: https://dawn-review.googlesource.com/c/tint/+/55402 Now that sanitizers are no longer exposed externally, the Inspector no longer needs to handle this post-sanitizer world. Bug: tint:697 Change-Id: Ic02ebb9c529aa132a238285bdd0d0df8686e219b Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/57104 Kokoro: Kokoro Reviewed-by: Ben Clayton --- src/inspector/inspector.cc | 43 +--- src/inspector/inspector_test.cc | 329 ------------------------ src/inspector/test_inspector_builder.cc | 54 ---- 3 files changed, 4 insertions(+), 422 deletions(-) diff --git a/src/inspector/inspector.cc b/src/inspector/inspector.cc index cd8546d31f..f8b3b5986a 100644 --- a/src/inspector/inspector.cc +++ b/src/inspector/inspector.cc @@ -172,46 +172,11 @@ std::vector Inspector::GetEntryPoints() { auto* decl = var->Declaration(); auto name = program_->Symbols().NameFor(decl->symbol()); - if (ast::HasDecoration(decl->decorations())) { - continue; - } - // TODO(crbug.com/tint/697): Remove this. - { - StageVariable stage_variable; - stage_variable.name = name; - - auto* type = var->Type()->UnwrapRef(); - std::tie(stage_variable.component_type, - stage_variable.composition_type) = - CalculateComponentAndComposition(type); - - auto* location_decoration = - ast::GetDecoration(decl->decorations()); - if (location_decoration) { - stage_variable.has_location_decoration = true; - stage_variable.location_decoration = location_decoration->value(); - } else { - stage_variable.has_location_decoration = false; - } - - std::tie(stage_variable.interpolation_type, - stage_variable.interpolation_sampling) = - CalculateInterpolationData(type, decl->decorations()); - - if (var->StorageClass() == ast::StorageClass::kInput) { - entry_point.input_variables.push_back(stage_variable); - } else if (var->StorageClass() == ast::StorageClass::kOutput) { - entry_point.output_variables.push_back(stage_variable); - } - } - - { - if (var->IsPipelineConstant()) { - OverridableConstant overridable_constant; - overridable_constant.name = name; - entry_point.overridable_constants.push_back(overridable_constant); - } + if (var->IsPipelineConstant()) { + OverridableConstant overridable_constant; + overridable_constant.name = name; + entry_point.overridable_constants.push_back(overridable_constant); } } diff --git a/src/inspector/inspector_test.cc b/src/inspector/inspector_test.cc index 70d5de6f72..f52b93b130 100644 --- a/src/inspector/inspector_test.cc +++ b/src/inspector/inspector_test.cc @@ -534,335 +534,6 @@ TEST_F(InspectorGetEntryPointTest, MixInOutVariablesAndStruct) { EXPECT_EQ(ComponentType::kUInt, result[0].output_variables[1].component_type); } -// TODO(crbug.com/tint/697): Remove this. -TEST_F(InspectorGetEntryPointTest, EntryPointInOutVariables_Legacy) { - AddInOutVariables({{"in_var", "out_var"}}); - - MakeInOutVariableBodyFunction("foo", {{"in_var", "out_var"}}, - ast::DecorationList{ - Stage(ast::PipelineStage::kFragment), - }); - - Inspector& inspector = Build(); - - auto result = inspector.GetEntryPoints(); - ASSERT_FALSE(inspector.has_error()) << inspector.error(); - - ASSERT_EQ(1u, result.size()); - - ASSERT_EQ(1u, result[0].input_variables.size()); - EXPECT_EQ("in_var", result[0].input_variables[0].name); - EXPECT_TRUE(result[0].input_variables[0].has_location_decoration); - EXPECT_EQ(0u, result[0].input_variables[0].location_decoration); - EXPECT_EQ(ComponentType::kUInt, result[0].input_variables[0].component_type); - - ASSERT_EQ(1u, result[0].output_variables.size()); - EXPECT_EQ("out_var", result[0].output_variables[0].name); - EXPECT_TRUE(result[0].output_variables[0].has_location_decoration); - EXPECT_EQ(1u, result[0].output_variables[0].location_decoration); - EXPECT_EQ(ComponentType::kUInt, result[0].output_variables[0].component_type); -} - -// TODO(crbug.com/tint/697): Remove this. -TEST_F(InspectorGetEntryPointTest, FunctionInOutVariables_Legacy) { - AddInOutVariables({{"in_var", "out_var"}}); - - MakeInOutVariableBodyFunction("func", {{"in_var", "out_var"}}, {}); - - MakeCallerBodyFunction("foo", {"func"}, - ast::DecorationList{ - Stage(ast::PipelineStage::kFragment), - }); - - Inspector& inspector = Build(); - - auto result = inspector.GetEntryPoints(); - ASSERT_FALSE(inspector.has_error()) << inspector.error(); - - ASSERT_EQ(1u, result.size()); - - ASSERT_EQ(1u, result[0].input_variables.size()); - EXPECT_EQ("in_var", result[0].input_variables[0].name); - EXPECT_TRUE(result[0].input_variables[0].has_location_decoration); - EXPECT_EQ(0u, result[0].input_variables[0].location_decoration); - EXPECT_EQ(ComponentType::kUInt, result[0].input_variables[0].component_type); - - ASSERT_EQ(1u, result[0].output_variables.size()); - EXPECT_EQ("out_var", result[0].output_variables[0].name); - EXPECT_TRUE(result[0].output_variables[0].has_location_decoration); - EXPECT_EQ(1u, result[0].output_variables[0].location_decoration); - EXPECT_EQ(ComponentType::kUInt, result[0].output_variables[0].component_type); -} - -// TODO(crbug.com/tint/697): Remove this. -TEST_F(InspectorGetEntryPointTest, RepeatedInOutVariables_Legacy) { - AddInOutVariables({{"in_var", "out_var"}}); - - MakeInOutVariableBodyFunction("func", {{"in_var", "out_var"}}, {}); - - MakeInOutVariableCallerBodyFunction("foo", "func", {{"in_var", "out_var"}}, - ast::DecorationList{ - Stage(ast::PipelineStage::kFragment), - }); - - Inspector& inspector = Build(); - - auto result = inspector.GetEntryPoints(); - ASSERT_FALSE(inspector.has_error()) << inspector.error(); - - ASSERT_EQ(1u, result.size()); - - ASSERT_EQ(1u, result[0].input_variables.size()); - EXPECT_EQ("in_var", result[0].input_variables[0].name); - EXPECT_TRUE(result[0].input_variables[0].has_location_decoration); - EXPECT_EQ(0u, result[0].input_variables[0].location_decoration); - EXPECT_EQ(ComponentType::kUInt, result[0].input_variables[0].component_type); - - ASSERT_EQ(1u, result[0].output_variables.size()); - EXPECT_EQ("out_var", result[0].output_variables[0].name); - EXPECT_TRUE(result[0].output_variables[0].has_location_decoration); - EXPECT_EQ(1u, result[0].output_variables[0].location_decoration); - EXPECT_EQ(ComponentType::kUInt, result[0].output_variables[0].component_type); -} - -// TODO(crbug.com/tint/697): Remove this. -TEST_F(InspectorGetEntryPointTest, EntryPointMultipleInOutVariables_Legacy) { - AddInOutVariables({{"in_var", "out_var"}, {"in2_var", "out2_var"}}); - - MakeInOutVariableBodyFunction( - "foo", {{"in_var", "out_var"}, {"in2_var", "out2_var"}}, - ast::DecorationList{ - Stage(ast::PipelineStage::kFragment), - }); - - Inspector& inspector = Build(); - - auto result = inspector.GetEntryPoints(); - ASSERT_FALSE(inspector.has_error()) << inspector.error(); - - ASSERT_EQ(1u, result.size()); - - ASSERT_EQ(2u, result[0].input_variables.size()); - EXPECT_TRUE(ContainsName(result[0].input_variables, "in_var")); - EXPECT_TRUE(ContainsName(result[0].input_variables, "in2_var")); - EXPECT_TRUE(result[0].input_variables[0].has_location_decoration); - EXPECT_EQ(0u, result[0].input_variables[0].location_decoration); - EXPECT_EQ(ComponentType::kUInt, result[0].input_variables[0].component_type); - EXPECT_TRUE(result[0].input_variables[1].has_location_decoration); - EXPECT_EQ(2u, result[0].input_variables[1].location_decoration); - EXPECT_EQ(ComponentType::kUInt, result[0].input_variables[1].component_type); - - ASSERT_EQ(2u, result[0].output_variables.size()); - EXPECT_TRUE(ContainsName(result[0].output_variables, "out_var")); - EXPECT_TRUE(ContainsName(result[0].output_variables, "out2_var")); - EXPECT_TRUE(result[0].output_variables[0].has_location_decoration); - EXPECT_EQ(1u, result[0].output_variables[0].location_decoration); - EXPECT_EQ(ComponentType::kUInt, result[0].output_variables[0].component_type); - EXPECT_TRUE(result[0].output_variables[1].has_location_decoration); - EXPECT_EQ(3u, result[0].output_variables[1].location_decoration); - EXPECT_EQ(ComponentType::kUInt, result[0].output_variables[1].component_type); -} - -// TODO(crbug.com/tint/697): Remove this. -TEST_F(InspectorGetEntryPointTest, FunctionMultipleInOutVariables_Legacy) { - AddInOutVariables({{"in_var", "out_var"}, {"in2_var", "out2_var"}}); - - MakeInOutVariableBodyFunction( - "func", {{"in_var", "out_var"}, {"in2_var", "out2_var"}}, {}); - - MakeCallerBodyFunction("foo", {"func"}, - ast::DecorationList{ - Stage(ast::PipelineStage::kFragment), - }); - - Inspector& inspector = Build(); - - auto result = inspector.GetEntryPoints(); - ASSERT_FALSE(inspector.has_error()) << inspector.error(); - - ASSERT_EQ(1u, result.size()); - - ASSERT_EQ(2u, result[0].input_variables.size()); - EXPECT_TRUE(ContainsName(result[0].input_variables, "in_var")); - EXPECT_TRUE(ContainsName(result[0].input_variables, "in2_var")); - EXPECT_TRUE(result[0].input_variables[0].has_location_decoration); - EXPECT_EQ(0u, result[0].input_variables[0].location_decoration); - EXPECT_EQ(ComponentType::kUInt, result[0].input_variables[0].component_type); - EXPECT_TRUE(result[0].input_variables[1].has_location_decoration); - EXPECT_EQ(2u, result[0].input_variables[1].location_decoration); - EXPECT_EQ(ComponentType::kUInt, result[0].input_variables[1].component_type); - - ASSERT_EQ(2u, result[0].output_variables.size()); - EXPECT_TRUE(ContainsName(result[0].output_variables, "out_var")); - EXPECT_TRUE(ContainsName(result[0].output_variables, "out2_var")); - EXPECT_TRUE(result[0].output_variables[0].has_location_decoration); - EXPECT_EQ(1u, result[0].output_variables[0].location_decoration); - EXPECT_EQ(ComponentType::kUInt, result[0].output_variables[0].component_type); - EXPECT_TRUE(result[0].output_variables[1].has_location_decoration); - EXPECT_EQ(3u, result[0].output_variables[1].location_decoration); - EXPECT_EQ(ComponentType::kUInt, result[0].output_variables[1].component_type); -} - -// TODO(crbug.com/tint/697): Remove this. -TEST_F(InspectorGetEntryPointTest, MultipleEntryPointsInOutVariables_Legacy) { - AddInOutVariables({{"in_var", "out_var"}, {"in2_var", "out2_var"}}); - - MakeInOutVariableBodyFunction("foo", {{"in_var", "out2_var"}}, - ast::DecorationList{ - Stage(ast::PipelineStage::kFragment), - }); - - MakeInOutVariableBodyFunction( - "bar", {{"in2_var", "out_var"}}, - ast::DecorationList{Stage(ast::PipelineStage::kCompute), - WorkgroupSize(1)}); - - // TODO(dsinclair): Update to run the namer transform when - // available. - - Inspector& inspector = Build(); - - auto result = inspector.GetEntryPoints(); - ASSERT_FALSE(inspector.has_error()) << inspector.error(); - - ASSERT_EQ(2u, result.size()); - - ASSERT_EQ("foo", result[0].name); - ASSERT_EQ("foo", result[0].remapped_name); - - ASSERT_EQ(1u, result[0].input_variables.size()); - EXPECT_EQ("in_var", result[0].input_variables[0].name); - EXPECT_TRUE(result[0].input_variables[0].has_location_decoration); - EXPECT_EQ(0u, result[0].input_variables[0].location_decoration); - EXPECT_EQ(ComponentType::kUInt, result[0].input_variables[0].component_type); - - ASSERT_EQ(1u, result[0].output_variables.size()); - EXPECT_EQ("out2_var", result[0].output_variables[0].name); - EXPECT_TRUE(result[0].output_variables[0].has_location_decoration); - EXPECT_EQ(3u, result[0].output_variables[0].location_decoration); - EXPECT_EQ(ComponentType::kUInt, result[0].output_variables[0].component_type); - - ASSERT_EQ("bar", result[1].name); - ASSERT_EQ("bar", result[1].remapped_name); - - ASSERT_EQ(1u, result[1].input_variables.size()); - EXPECT_EQ("in2_var", result[1].input_variables[0].name); - EXPECT_TRUE(result[1].input_variables[0].has_location_decoration); - EXPECT_EQ(2u, result[1].input_variables[0].location_decoration); - EXPECT_EQ(ComponentType::kUInt, result[1].input_variables[0].component_type); - - ASSERT_EQ(1u, result[1].output_variables.size()); - EXPECT_EQ("out_var", result[1].output_variables[0].name); - EXPECT_TRUE(result[1].output_variables[0].has_location_decoration); - EXPECT_EQ(1u, result[1].output_variables[0].location_decoration); - EXPECT_EQ(ComponentType::kUInt, result[1].output_variables[0].component_type); -} - -// TODO(crbug.com/tint/697): Remove this. -TEST_F(InspectorGetEntryPointTest, - MultipleEntryPointsSharedInOutVariables_Legacy) { - AddInOutVariables({{"in_var", "out_var"}, {"in2_var", "out2_var"}}); - - MakeInOutVariableBodyFunction("func", {{"in2_var", "out2_var"}}, {}); - - MakeInOutVariableCallerBodyFunction("foo", "func", {{"in_var", "out_var"}}, - ast::DecorationList{ - Stage(ast::PipelineStage::kFragment), - }); - - MakeCallerBodyFunction( - "bar", {"func"}, - ast::DecorationList{Stage(ast::PipelineStage::kCompute), - WorkgroupSize(1)}); - - // TODO(dsinclair): Update to run the namer transform when - // available. - - Inspector& inspector = Build(); - - auto result = inspector.GetEntryPoints(); - ASSERT_FALSE(inspector.has_error()) << inspector.error(); - - ASSERT_EQ(2u, result.size()); - - ASSERT_EQ("foo", result[0].name); - ASSERT_EQ("foo", result[0].remapped_name); - - ASSERT_EQ(2u, result[0].input_variables.size()); - EXPECT_TRUE(ContainsName(result[0].input_variables, "in_var")); - EXPECT_TRUE(ContainsName(result[0].input_variables, "in2_var")); - EXPECT_TRUE(result[0].input_variables[0].has_location_decoration); - EXPECT_EQ(0u, result[0].input_variables[0].location_decoration); - EXPECT_EQ(ComponentType::kUInt, result[0].input_variables[0].component_type); - EXPECT_TRUE(result[0].input_variables[1].has_location_decoration); - EXPECT_EQ(2u, result[0].input_variables[1].location_decoration); - EXPECT_EQ(ComponentType::kUInt, result[0].input_variables[1].component_type); - - ASSERT_EQ(2u, result[0].output_variables.size()); - EXPECT_TRUE(ContainsName(result[0].output_variables, "out_var")); - EXPECT_TRUE(ContainsName(result[0].output_variables, "out2_var")); - EXPECT_TRUE(result[0].output_variables[0].has_location_decoration); - EXPECT_EQ(1u, result[0].output_variables[0].location_decoration); - EXPECT_EQ(ComponentType::kUInt, result[0].output_variables[0].component_type); - EXPECT_TRUE(result[0].output_variables[0].has_location_decoration); - EXPECT_EQ(3u, result[0].output_variables[1].location_decoration); - EXPECT_EQ(ComponentType::kUInt, result[0].output_variables[1].component_type); - - ASSERT_EQ("bar", result[1].name); - ASSERT_EQ("bar", result[1].remapped_name); - - ASSERT_EQ(1u, result[1].input_variables.size()); - EXPECT_EQ("in2_var", result[1].input_variables[0].name); - EXPECT_TRUE(result[1].input_variables[0].has_location_decoration); - EXPECT_EQ(2u, result[1].input_variables[0].location_decoration); - EXPECT_EQ(ComponentType::kUInt, result[1].input_variables[0].component_type); - - ASSERT_EQ(1u, result[1].output_variables.size()); - EXPECT_EQ("out2_var", result[1].output_variables[0].name); - EXPECT_TRUE(result[1].output_variables[0].has_location_decoration); - EXPECT_EQ(3u, result[1].output_variables[0].location_decoration); - EXPECT_EQ(ComponentType::kUInt, result[1].output_variables[0].component_type); -} - -// TODO(crbug.com/tint/697): Remove this. -TEST_F(InspectorGetEntryPointTest, BuiltInsNotStageVariables_Legacy) { - Global("in_var", ty.u32(), ast::StorageClass::kInput, nullptr, - ast::DecorationList{ - Builtin(ast::Builtin::kPosition), - ASTNodes().Create( - ID(), ast::DisabledValidation::kIgnoreStorageClass)}); - Global("out_var", ty.u32(), ast::StorageClass::kOutput, nullptr, - ast::DecorationList{ - Location(0), - ASTNodes().Create( - ID(), ast::DisabledValidation::kIgnoreStorageClass)}); - - MakeInOutVariableBodyFunction("func", {{"in_var", "out_var"}}, {}); - - MakeCallerBodyFunction("foo", {"func"}, - ast::DecorationList{ - Stage(ast::PipelineStage::kFragment), - }); - - // TODO(dsinclair): Update to run the namer transform when available. - - Inspector& inspector = Build(); - - auto result = inspector.GetEntryPoints(); - ASSERT_FALSE(inspector.has_error()) << inspector.error(); - - ASSERT_EQ(1u, result.size()); - - ASSERT_EQ("foo", result[0].name); - ASSERT_EQ("foo", result[0].remapped_name); - EXPECT_EQ(0u, result[0].input_variables.size()); - EXPECT_EQ(1u, result[0].output_variables.size()); - EXPECT_TRUE(ContainsName(result[0].output_variables, "out_var")); - EXPECT_TRUE(result[0].output_variables[0].has_location_decoration); - EXPECT_EQ(0u, result[0].output_variables[0].location_decoration); - EXPECT_EQ(ComponentType::kUInt, result[0].output_variables[0].component_type); -} - TEST_F(InspectorGetEntryPointTest, OverridableConstantUnreferenced) { AddOverridableConstantWithoutID("foo", ty.f32(), nullptr); MakeEmptyBodyFunction( diff --git a/src/inspector/test_inspector_builder.cc b/src/inspector/test_inspector_builder.cc index 8e1912ca01..922f342918 100644 --- a/src/inspector/test_inspector_builder.cc +++ b/src/inspector/test_inspector_builder.cc @@ -60,60 +60,6 @@ ast::Struct* InspectorBuilder::MakeInOutStruct( return Structure(name, members); } -// TODO(crbug.com/tint/697): Remove this. -void InspectorBuilder::AddInOutVariables( - std::vector> inout_vars) { - uint32_t location = 0; - for (auto inout : inout_vars) { - std::string in, out; - std::tie(in, out) = inout; - - Global(in, ty.u32(), ast::StorageClass::kInput, nullptr, - ast::DecorationList{ - Location(location++), - ASTNodes().Create( - ID(), ast::DisabledValidation::kIgnoreStorageClass)}); - Global(out, ty.u32(), ast::StorageClass::kOutput, nullptr, - ast::DecorationList{ - Location(location++), - ASTNodes().Create( - ID(), ast::DisabledValidation::kIgnoreStorageClass)}); - } -} - -// TODO(crbug.com/tint/697): Remove this. -void InspectorBuilder::MakeInOutVariableBodyFunction( - std::string name, - std::vector> inout_vars, - ast::DecorationList decorations) { - ast::StatementList stmts; - for (auto inout : inout_vars) { - std::string in, out; - std::tie(in, out) = inout; - stmts.emplace_back(Assign(out, in)); - } - stmts.emplace_back(Return()); - Func(name, ast::VariableList(), ty.void_(), stmts, decorations); -} - -// TODO(crbug.com/tint/697): Remove this. -ast::Function* InspectorBuilder::MakeInOutVariableCallerBodyFunction( - std::string caller, - std::string callee, - std::vector> inout_vars, - ast::DecorationList decorations) { - ast::StatementList stmts; - for (auto inout : inout_vars) { - std::string in, out; - std::tie(in, out) = inout; - stmts.emplace_back(Assign(out, in)); - } - stmts.emplace_back(create(Call(callee))); - stmts.emplace_back(Return()); - - return Func(caller, ast::VariableList(), ty.void_(), stmts, decorations); -} - ast::Function* InspectorBuilder::MakeConstReferenceBodyFunction( std::string func, std::string var,