diff --git a/src/BUILD.gn b/src/BUILD.gn index 9ad333dc03..ccbbfd6c28 100644 --- a/src/BUILD.gn +++ b/src/BUILD.gn @@ -681,8 +681,6 @@ libtint_source_set("libtint_msl_writer_src") { libtint_source_set("libtint_hlsl_writer_src") { sources = [ - "transform/hlsl.cc", - "transform/hlsl.h", "writer/hlsl/generator.cc", "writer/hlsl/generator.h", "writer/hlsl/generator_impl.cc", diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 8dd18966c4..51a8b91b1f 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -486,8 +486,6 @@ endif() if(${TINT_BUILD_HLSL_WRITER}) list(APPEND TINT_LIB_SRCS - transform/hlsl.cc - transform/hlsl.h writer/hlsl/generator.cc writer/hlsl/generator.h writer/hlsl/generator_impl.cc @@ -986,9 +984,6 @@ if(${TINT_BUILD_TESTS}) endif() if (${TINT_BUILD_HLSL_WRITER}) - if(${TINT_BUILD_WGSL_READER} AND ${TINT_BUILD_WGSL_WRITER}) - list(APPEND TINT_TEST_SRCS transform/hlsl_test.cc) - endif() list(APPEND TINT_TEST_SRCS writer/hlsl/generator_impl_array_accessor_test.cc writer/hlsl/generator_impl_assign_test.cc diff --git a/src/transform/hlsl.cc b/src/transform/hlsl.cc deleted file mode 100644 index 0fe35b59a6..0000000000 --- a/src/transform/hlsl.cc +++ /dev/null @@ -1,97 +0,0 @@ -// Copyright 2021 The Tint Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "src/transform/hlsl.h" - -#include - -#include "src/program_builder.h" -#include "src/transform/add_empty_entry_point.h" -#include "src/transform/calculate_array_length.h" -#include "src/transform/canonicalize_entry_point_io.h" -#include "src/transform/decompose_memory_access.h" -#include "src/transform/external_texture_transform.h" -#include "src/transform/fold_trivial_single_use_lets.h" -#include "src/transform/inline_pointer_lets.h" -#include "src/transform/loop_to_for_loop.h" -#include "src/transform/manager.h" -#include "src/transform/pad_array_elements.h" -#include "src/transform/promote_initializers_to_const_var.h" -#include "src/transform/simplify.h" -#include "src/transform/zero_init_workgroup_memory.h" - -TINT_INSTANTIATE_TYPEINFO(tint::transform::Hlsl); -TINT_INSTANTIATE_TYPEINFO(tint::transform::Hlsl::Config); - -namespace tint { -namespace transform { - -Hlsl::Hlsl() = default; -Hlsl::~Hlsl() = default; - -Output Hlsl::Run(const Program* in, const DataMap& inputs) { - Manager manager; - DataMap data; - - auto* cfg = inputs.Get(); - - // Attempt to convert `loop`s into for-loops. This is to try and massage the - // output into something that will not cause FXC to choke or misbehave. - manager.Add(); - manager.Add(); - - if (!cfg || !cfg->disable_workgroup_init) { - // ZeroInitWorkgroupMemory must come before CanonicalizeEntryPointIO as - // ZeroInitWorkgroupMemory may inject new builtin parameters. - manager.Add(); - } - manager.Add(); - manager.Add(); - // Simplify cleans up messy `*(&(expr))` expressions from InlinePointerLets. - manager.Add(); - // DecomposeMemoryAccess must come after InlinePointerLets as we cannot take - // the address of calls to DecomposeMemoryAccess::Intrinsic. Must also come - // after Simplify, as we need to fold away the address-of and defers of - // `*(&(intrinsic_load()))` expressions. - manager.Add(); - // CalculateArrayLength must come after DecomposeMemoryAccess, as - // DecomposeMemoryAccess special-cases the arrayLength() intrinsic, which - // will be transformed by CalculateArrayLength - manager.Add(); - manager.Add(); - manager.Add(); - manager.Add(); - manager.Add(); - - data.Add( - CanonicalizeEntryPointIO::ShaderStyle::kHlsl); - auto out = manager.Run(in, data); - if (!out.program.IsValid()) { - return out; - } - - ProgramBuilder builder; - CloneContext ctx(&builder, &out.program); - // TODO(jrprice): Move the sanitizer into the backend. - ctx.Clone(); - builder.SetTransformApplied(this); - return Output{Program(std::move(builder))}; -} - -Hlsl::Config::Config(bool disable_wi) : disable_workgroup_init(disable_wi) {} -Hlsl::Config::Config(const Config&) = default; -Hlsl::Config::~Config() = default; - -} // namespace transform -} // namespace tint diff --git a/src/transform/hlsl.h b/src/transform/hlsl.h deleted file mode 100644 index ea8a27ec90..0000000000 --- a/src/transform/hlsl.h +++ /dev/null @@ -1,63 +0,0 @@ -// Copyright 2021 The Tint Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#ifndef SRC_TRANSFORM_HLSL_H_ -#define SRC_TRANSFORM_HLSL_H_ - -#include "src/transform/transform.h" - -namespace tint { - -// Forward declarations -class CloneContext; - -namespace transform { - -/// Hlsl is a transform used to sanitize a Program for use with the Hlsl writer. -/// Passing a non-sanitized Program to the Hlsl writer will result in undefined -/// behavior. -class Hlsl : public Castable { - public: - /// Configuration options for the Hlsl sanitizer transform. - struct Config : public Castable { - /// Constructor - /// @param disable_workgroup_init `true` to disable workgroup memory zero - /// initialization - explicit Config(bool disable_workgroup_init = false); - - /// Copy constructor - Config(const Config&); - - /// Destructor - ~Config() override; - - /// Set to `true` to disable workgroup memory zero initialization - bool disable_workgroup_init = false; - }; - - /// Constructor - Hlsl(); - ~Hlsl() override; - - /// Runs the transform on `program`, returning the transformation result. - /// @param program the source program to transform - /// @param data optional extra transform-specific data - /// @returns the transformation result - Output Run(const Program* program, const DataMap& data = {}) override; -}; - -} // namespace transform -} // namespace tint - -#endif // SRC_TRANSFORM_HLSL_H_ diff --git a/src/transform/hlsl_test.cc b/src/transform/hlsl_test.cc deleted file mode 100644 index 4389d7e3d9..0000000000 --- a/src/transform/hlsl_test.cc +++ /dev/null @@ -1,29 +0,0 @@ -// Copyright 2021 The Tint Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "src/transform/hlsl.h" - -#include "src/transform/test_helper.h" - -namespace tint { -namespace transform { -namespace { - -using HlslTest = TransformTest; - -// TODO(jrprice): Remove this file when we remove the sanitizer transforms. - -} // namespace -} // namespace transform -} // namespace tint diff --git a/src/writer/hlsl/generator.cc b/src/writer/hlsl/generator.cc index 259f0b6b0b..d2ee6011ad 100644 --- a/src/writer/hlsl/generator.cc +++ b/src/writer/hlsl/generator.cc @@ -14,7 +14,6 @@ #include "src/writer/hlsl/generator.h" -#include "src/transform/hlsl.h" #include "src/writer/hlsl/generator_impl.h" namespace tint { @@ -28,27 +27,24 @@ Result::Result(const Result&) = default; Result Generate(const Program* program, const Options& options) { Result result; - // Run the HLSL sanitizer. - transform::Hlsl sanitizer; - transform::DataMap transform_input; - transform_input.Add(options.disable_workgroup_init); - auto output = sanitizer.Run(program, transform_input); - if (!output.program.IsValid()) { + // Sanitize the program. + auto sanitized_result = Sanitize(program, options.disable_workgroup_init); + if (!sanitized_result.program.IsValid()) { result.success = false; - result.error = output.program.Diagnostics().str(); + result.error = sanitized_result.program.Diagnostics().str(); return result; } // Generate the HLSL code. - auto impl = std::make_unique(&output.program); + auto impl = std::make_unique(&sanitized_result.program); result.success = impl->Generate(); result.error = impl->error(); result.hlsl = impl->result(); // Collect the list of entry points in the sanitized program. - for (auto* func : output.program.AST().Functions()) { + for (auto* func : sanitized_result.program.AST().Functions()) { if (func->IsEntryPoint()) { - auto name = output.program.Symbols().NameFor(func->symbol()); + auto name = sanitized_result.program.Symbols().NameFor(func->symbol()); result.entry_points.push_back({name, func->pipeline_stage()}); } } diff --git a/src/writer/hlsl/generator_impl.cc b/src/writer/hlsl/generator_impl.cc index d8e6dfd392..c70e331c46 100644 --- a/src/writer/hlsl/generator_impl.cc +++ b/src/writer/hlsl/generator_impl.cc @@ -42,8 +42,19 @@ #include "src/sem/storage_texture_type.h" #include "src/sem/struct.h" #include "src/sem/variable.h" +#include "src/transform/add_empty_entry_point.h" #include "src/transform/calculate_array_length.h" -#include "src/transform/hlsl.h" +#include "src/transform/canonicalize_entry_point_io.h" +#include "src/transform/decompose_memory_access.h" +#include "src/transform/external_texture_transform.h" +#include "src/transform/fold_trivial_single_use_lets.h" +#include "src/transform/inline_pointer_lets.h" +#include "src/transform/loop_to_for_loop.h" +#include "src/transform/manager.h" +#include "src/transform/pad_array_elements.h" +#include "src/transform/promote_initializers_to_const_var.h" +#include "src/transform/simplify.h" +#include "src/transform/zero_init_workgroup_memory.h" #include "src/utils/defer.h" #include "src/utils/get_or_create.h" #include "src/utils/scoped_assignment.h" @@ -102,19 +113,51 @@ std::ostream& operator<<(std::ostream& s, const RegisterAndSpace& rs) { } // namespace +SanitizedResult Sanitize(const Program* in, bool disable_workgroup_init) { + transform::Manager manager; + transform::DataMap data; + + // Attempt to convert `loop`s into for-loops. This is to try and massage the + // output into something that will not cause FXC to choke or misbehave. + manager.Add(); + manager.Add(); + + if (!disable_workgroup_init) { + // ZeroInitWorkgroupMemory must come before CanonicalizeEntryPointIO as + // ZeroInitWorkgroupMemory may inject new builtin parameters. + manager.Add(); + } + manager.Add(); + manager.Add(); + // Simplify cleans up messy `*(&(expr))` expressions from InlinePointerLets. + manager.Add(); + // DecomposeMemoryAccess must come after InlinePointerLets as we cannot take + // the address of calls to DecomposeMemoryAccess::Intrinsic. Must also come + // after Simplify, as we need to fold away the address-of and defers of + // `*(&(intrinsic_load()))` expressions. + manager.Add(); + // CalculateArrayLength must come after DecomposeMemoryAccess, as + // DecomposeMemoryAccess special-cases the arrayLength() intrinsic, which + // will be transformed by CalculateArrayLength + manager.Add(); + manager.Add(); + manager.Add(); + manager.Add(); + manager.Add(); + + data.Add( + transform::CanonicalizeEntryPointIO::ShaderStyle::kHlsl); + + SanitizedResult result; + result.program = std::move(manager.Run(in, data).program); + return result; +} + GeneratorImpl::GeneratorImpl(const Program* program) : TextGenerator(program) {} GeneratorImpl::~GeneratorImpl() = default; bool GeneratorImpl::Generate() { - if (!builder_.HasTransformApplied()) { - diagnostics_.add_error( - diag::System::Writer, - "HLSL writer requires the transform::Hlsl sanitizer to have been " - "applied to the input program"); - return false; - } - const TypeInfo* last_kind = nullptr; size_t last_padding_line = 0; diff --git a/src/writer/hlsl/generator_impl.h b/src/writer/hlsl/generator_impl.h index 7ec64ff70e..17a667458a 100644 --- a/src/writer/hlsl/generator_impl.h +++ b/src/writer/hlsl/generator_impl.h @@ -48,6 +48,18 @@ class Intrinsic; namespace writer { namespace hlsl { +/// The result of sanitizing a program for generation. +struct SanitizedResult { + /// The sanitized program. + Program program; +}; + +/// Sanitize a program in preparation for generating HLSL. +/// @param disable_workgroup_init `true` to disable workgroup memory zero +/// @returns the sanitized program and any supplementary information +SanitizedResult Sanitize(const Program* program, + bool disable_workgroup_init = false); + /// Implementation class for HLSL generator class GeneratorImpl : public TextGenerator { public: diff --git a/src/writer/hlsl/generator_impl_test.cc b/src/writer/hlsl/generator_impl_test.cc index cd01730cb4..d4227f359b 100644 --- a/src/writer/hlsl/generator_impl_test.cc +++ b/src/writer/hlsl/generator_impl_test.cc @@ -21,16 +21,6 @@ namespace { using HlslGeneratorImplTest = TestHelper; -TEST_F(HlslGeneratorImplTest, ErrorIfSanitizerNotRun) { - auto program = std::make_unique(std::move(*this)); - GeneratorImpl gen(program.get()); - EXPECT_FALSE(gen.Generate()); - EXPECT_EQ( - gen.error(), - "error: HLSL writer requires the transform::Hlsl sanitizer to have been " - "applied to the input program"); -} - TEST_F(HlslGeneratorImplTest, Generate) { Func("my_func", ast::VariableList{}, ty.void_(), ast::StatementList{}, ast::DecorationList{}); diff --git a/src/writer/hlsl/test_helper.h b/src/writer/hlsl/test_helper.h index 7daca490ca..7523c82cbb 100644 --- a/src/writer/hlsl/test_helper.h +++ b/src/writer/hlsl/test_helper.h @@ -20,7 +20,6 @@ #include #include "gtest/gtest.h" -#include "src/transform/hlsl.h" #include "src/transform/manager.h" #include "src/transform/renamer.h" #include "src/writer/hlsl/generator_impl.h" @@ -44,9 +43,6 @@ class TestHelperBase : public BODY, public ProgramBuilder { if (gen_) { return *gen_; } - // Fake that the HLSL sanitizer has been applied, so that we can unit test - // the writer without it erroring. - SetTransformApplied(); [&]() { ASSERT_TRUE(IsValid()) << "Builder program is not valid\n" << diag::Formatter().format(Diagnostics()); @@ -60,7 +56,7 @@ class TestHelperBase : public BODY, public ProgramBuilder { return *gen_; } - /// Builds the program, runs the program through the transform::Hlsl sanitizer + /// Builds the program, runs the program through the HLSL sanitizer /// and returns a GeneratorImpl from the sanitized program. /// @note The generator is only built once. Multiple calls to Build() will /// return the same GeneratorImpl without rebuilding. @@ -80,13 +76,19 @@ class TestHelperBase : public BODY, public ProgramBuilder { << formatter.format(program->Diagnostics()); }(); + auto sanitized_result = Sanitize(program.get()); + [&]() { + ASSERT_TRUE(sanitized_result.program.IsValid()) + << formatter.format(sanitized_result.program.Diagnostics()); + }(); + transform::Manager transform_manager; transform::DataMap transform_data; transform_data.Add( transform::Renamer::Target::kHlslKeywords); transform_manager.Add(); - transform_manager.Add(); - auto result = transform_manager.Run(program.get(), transform_data); + auto result = + transform_manager.Run(&sanitized_result.program, transform_data); [&]() { ASSERT_TRUE(result.program.IsValid()) << formatter.format(result.program.Diagnostics()); diff --git a/test/BUILD.gn b/test/BUILD.gn index 8d3ee1f078..203b853828 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -586,7 +586,6 @@ tint_unittests_source_set("tint_unittests_msl_writer_src") { tint_unittests_source_set("tint_unittests_hlsl_writer_src") { sources = [ - "../src/transform/hlsl_test.cc", "../src/writer/hlsl/generator_impl_array_accessor_test.cc", "../src/writer/hlsl/generator_impl_assign_test.cc", "../src/writer/hlsl/generator_impl_binary_test.cc",