hlsl: Remove the sanitizer transform
Invoke the required transforms directly in the HLSL backend.
Change-Id: I9465fef375dd4dad6a91c1e7e16ede6401b9bfc0
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/63800
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
diff --git a/src/BUILD.gn b/src/BUILD.gn
index 9ad333d..ccbbfd6 100644
--- a/src/BUILD.gn
+++ b/src/BUILD.gn
@@ -681,8 +681,6 @@
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 8dd1896..51a8b91 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -486,8 +486,6 @@
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 @@
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 0fe35b5..0000000
--- 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 <utility>
-
-#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<Config>();
-
- // 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<FoldTrivialSingleUseLets>();
- manager.Add<LoopToForLoop>();
-
- if (!cfg || !cfg->disable_workgroup_init) {
- // ZeroInitWorkgroupMemory must come before CanonicalizeEntryPointIO as
- // ZeroInitWorkgroupMemory may inject new builtin parameters.
- manager.Add<ZeroInitWorkgroupMemory>();
- }
- manager.Add<CanonicalizeEntryPointIO>();
- manager.Add<InlinePointerLets>();
- // Simplify cleans up messy `*(&(expr))` expressions from InlinePointerLets.
- manager.Add<Simplify>();
- // 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<DecomposeMemoryAccess>();
- // CalculateArrayLength must come after DecomposeMemoryAccess, as
- // DecomposeMemoryAccess special-cases the arrayLength() intrinsic, which
- // will be transformed by CalculateArrayLength
- manager.Add<CalculateArrayLength>();
- manager.Add<ExternalTextureTransform>();
- manager.Add<PromoteInitializersToConstVar>();
- manager.Add<PadArrayElements>();
- manager.Add<AddEmptyEntryPoint>();
-
- data.Add<CanonicalizeEntryPointIO::Config>(
- 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 ea8a27e..0000000
--- 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<Hlsl, Transform> {
- public:
- /// Configuration options for the Hlsl sanitizer transform.
- struct Config : public Castable<Data, transform::Data> {
- /// 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 4389d7e..0000000
--- 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 259f0b6..d2ee601 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 Generate(const Program* program, const Options& options) {
Result result;
- // Run the HLSL sanitizer.
- transform::Hlsl sanitizer;
- transform::DataMap transform_input;
- transform_input.Add<transform::Hlsl::Config>(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<GeneratorImpl>(&output.program);
+ auto impl = std::make_unique<GeneratorImpl>(&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 d8e6dfd..c70e331 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 @@
} // 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<transform::FoldTrivialSingleUseLets>();
+ manager.Add<transform::LoopToForLoop>();
+
+ if (!disable_workgroup_init) {
+ // ZeroInitWorkgroupMemory must come before CanonicalizeEntryPointIO as
+ // ZeroInitWorkgroupMemory may inject new builtin parameters.
+ manager.Add<transform::ZeroInitWorkgroupMemory>();
+ }
+ manager.Add<transform::CanonicalizeEntryPointIO>();
+ manager.Add<transform::InlinePointerLets>();
+ // Simplify cleans up messy `*(&(expr))` expressions from InlinePointerLets.
+ manager.Add<transform::Simplify>();
+ // 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<transform::DecomposeMemoryAccess>();
+ // CalculateArrayLength must come after DecomposeMemoryAccess, as
+ // DecomposeMemoryAccess special-cases the arrayLength() intrinsic, which
+ // will be transformed by CalculateArrayLength
+ manager.Add<transform::CalculateArrayLength>();
+ manager.Add<transform::ExternalTextureTransform>();
+ manager.Add<transform::PromoteInitializersToConstVar>();
+ manager.Add<transform::PadArrayElements>();
+ manager.Add<transform::AddEmptyEntryPoint>();
+
+ data.Add<transform::CanonicalizeEntryPointIO::Config>(
+ 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<transform::Hlsl>()) {
- 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 7ec64ff..17a6674 100644
--- a/src/writer/hlsl/generator_impl.h
+++ b/src/writer/hlsl/generator_impl.h
@@ -48,6 +48,18 @@
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 cd01730..d4227f3 100644
--- a/src/writer/hlsl/generator_impl_test.cc
+++ b/src/writer/hlsl/generator_impl_test.cc
@@ -21,16 +21,6 @@
using HlslGeneratorImplTest = TestHelper;
-TEST_F(HlslGeneratorImplTest, ErrorIfSanitizerNotRun) {
- auto program = std::make_unique<Program>(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 7daca49..7523c82 100644
--- a/src/writer/hlsl/test_helper.h
+++ b/src/writer/hlsl/test_helper.h
@@ -20,7 +20,6 @@
#include <utility>
#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 @@
if (gen_) {
return *gen_;
}
- // Fake that the HLSL sanitizer has been applied, so that we can unit test
- // the writer without it erroring.
- SetTransformApplied<transform::Hlsl>();
[&]() {
ASSERT_TRUE(IsValid()) << "Builder program is not valid\n"
<< diag::Formatter().format(Diagnostics());
@@ -60,7 +56,7 @@
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 @@
<< 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::Config>(
transform::Renamer::Target::kHlslKeywords);
transform_manager.Add<tint::transform::Renamer>();
- transform_manager.Add<tint::transform::Hlsl>();
- 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 8d3ee1f..203b853 100644
--- a/test/BUILD.gn
+++ b/test/BUILD.gn
@@ -586,7 +586,6 @@
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",
