spirv: Remove the sanitizer transform
Invoke the required transforms directly in the SPIR-V backend.
Change-Id: I78dc667d5c4c9c1d4da13ef5a99ece831c103982
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/63801
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
diff --git a/src/BUILD.gn b/src/BUILD.gn
index ccbbfd6..da64b3c 100644
--- a/src/BUILD.gn
+++ b/src/BUILD.gn
@@ -621,8 +621,6 @@
libtint_source_set("libtint_spv_writer_src") {
sources = [
- "transform/spirv.cc",
- "transform/spirv.h",
"writer/spirv/binary_writer.cc",
"writer/spirv/binary_writer.h",
"writer/spirv/builder.cc",
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 51a8b91..ecceb80 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -448,8 +448,6 @@
if(${TINT_BUILD_SPV_WRITER})
list(APPEND TINT_LIB_SRCS
- transform/spirv.cc
- transform/spirv.h
writer/spirv/binary_writer.cc
writer/spirv/binary_writer.h
writer/spirv/builder.cc
@@ -847,9 +845,6 @@
endif()
if(${TINT_BUILD_SPV_WRITER})
- if(${TINT_BUILD_WGSL_READER} AND ${TINT_BUILD_WGSL_WRITER})
- list(APPEND TINT_TEST_SRCS transform/spirv_test.cc)
- endif()
list(APPEND TINT_TEST_SRCS
writer/spirv/binary_writer_test.cc
writer/spirv/builder_accessor_expression_test.cc
diff --git a/src/transform/spirv.cc b/src/transform/spirv.cc
deleted file mode 100644
index 76fca09..0000000
--- a/src/transform/spirv.cc
+++ /dev/null
@@ -1,86 +0,0 @@
-// Copyright 2020 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/spirv.h"
-
-#include <string>
-#include <utility>
-
-#include "src/ast/stage_decoration.h"
-#include "src/program_builder.h"
-#include "src/sem/variable.h"
-#include "src/transform/add_empty_entry_point.h"
-#include "src/transform/canonicalize_entry_point_io.h"
-#include "src/transform/external_texture_transform.h"
-#include "src/transform/fold_constants.h"
-#include "src/transform/for_loop_to_loop.h"
-#include "src/transform/inline_pointer_lets.h"
-#include "src/transform/manager.h"
-#include "src/transform/simplify.h"
-#include "src/transform/zero_init_workgroup_memory.h"
-
-TINT_INSTANTIATE_TYPEINFO(tint::transform::Spirv);
-TINT_INSTANTIATE_TYPEINFO(tint::transform::Spirv::Config);
-
-namespace tint {
-namespace transform {
-
-Spirv::Spirv() = default;
-Spirv::~Spirv() = default;
-
-Output Spirv::Run(const Program* in, const DataMap& data) {
- auto* cfg = data.Get<Config>();
-
- Manager manager;
- DataMap internal_inputs;
- if (!cfg || !cfg->disable_workgroup_init) {
- manager.Add<ZeroInitWorkgroupMemory>();
- }
- manager.Add<InlinePointerLets>(); // Required for arrayLength()
- manager.Add<Simplify>(); // Required for arrayLength()
- manager.Add<FoldConstants>();
- manager.Add<ExternalTextureTransform>();
- manager.Add<ForLoopToLoop>(); // Must come after ZeroInitWorkgroupMemory
- manager.Add<CanonicalizeEntryPointIO>();
- manager.Add<AddEmptyEntryPoint>();
-
- internal_inputs.Add<CanonicalizeEntryPointIO::Config>(
- CanonicalizeEntryPointIO::Config(
- CanonicalizeEntryPointIO::ShaderStyle::kSpirv, 0xFFFFFFFF,
- (cfg && cfg->emit_vertex_point_size)));
-
- auto transformedInput = manager.Run(in, internal_inputs);
-
- if (transformedInput.program.Diagnostics().contains_errors()) {
- return transformedInput;
- }
-
- ProgramBuilder builder;
- CloneContext ctx(&builder, &transformedInput.program);
- // TODO(jrprice): Move the sanitizer into the backend.
- ctx.Clone();
-
- builder.SetTransformApplied(this);
- return Output{Program(std::move(builder))};
-}
-
-Spirv::Config::Config(bool emit_vps, bool disable_wi)
- : emit_vertex_point_size(emit_vps), disable_workgroup_init(disable_wi) {}
-
-Spirv::Config::Config(const Config&) = default;
-Spirv::Config::~Config() = default;
-Spirv::Config& Spirv::Config::operator=(const Config&) = default;
-
-} // namespace transform
-} // namespace tint
diff --git a/src/transform/spirv.h b/src/transform/spirv.h
deleted file mode 100644
index 7e633ab..0000000
--- a/src/transform/spirv.h
+++ /dev/null
@@ -1,75 +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_SPIRV_H_
-#define SRC_TRANSFORM_SPIRV_H_
-
-#include <vector>
-
-#include "src/transform/transform.h"
-
-namespace tint {
-
-// Forward declarations
-class CloneContext;
-
-namespace transform {
-
-/// Spirv is a transform used to sanitize a Program for use with the Spirv
-/// writer. Passing a non-sanitized Program to the Spirv writer will result in
-/// undefined behavior.
-class Spirv : public Castable<Spirv, Transform> {
- public:
- /// Configuration options for the transform.
- struct Config : public Castable<Config, Data> {
- /// Constructor
- /// @param emit_vertex_point_size `true` to generate a PointSize builtin
- /// @param disable_workgroup_init `true` to disable workgroup memory zero
- /// initialization
- Config(bool emit_vertex_point_size = false,
- bool disable_workgroup_init = false);
-
- /// Copy constructor.
- Config(const Config&);
-
- /// Destructor.
- ~Config() override;
-
- /// Assignment operator.
- /// @returns this Config
- Config& operator=(const Config&);
-
- /// Set to `true` to generate a PointSize builtin and have it set to 1.0
- /// from all vertex shaders in the module.
- bool emit_vertex_point_size = false;
-
- /// Set to `true` to disable workgroup memory zero initialization
- bool disable_workgroup_init = false;
- };
-
- /// Constructor
- Spirv();
- ~Spirv() override;
-
- /// Runs the transform on `program`, returning the transformation result.
- /// @param program the source program to transform
- /// @param data optional extra transform-specific input data
- /// @returns the transformation result
- Output Run(const Program* program, const DataMap& data = {}) override;
-};
-
-} // namespace transform
-} // namespace tint
-
-#endif // SRC_TRANSFORM_SPIRV_H_
diff --git a/src/transform/spirv_test.cc b/src/transform/spirv_test.cc
deleted file mode 100644
index e2d4c3c..0000000
--- a/src/transform/spirv_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/spirv.h"
-
-#include "src/transform/test_helper.h"
-
-namespace tint {
-namespace transform {
-namespace {
-
-using SpirvTest = TransformTest;
-
-// TODO(jrprice): Remove this file when we remove the sanitizers.
-
-} // namespace
-} // namespace transform
-} // namespace tint
diff --git a/src/writer/spirv/builder.cc b/src/writer/spirv/builder.cc
index 11942ef..1610463 100644
--- a/src/writer/spirv/builder.cc
+++ b/src/writer/spirv/builder.cc
@@ -36,7 +36,15 @@
#include "src/sem/struct.h"
#include "src/sem/variable.h"
#include "src/sem/vector_type.h"
-#include "src/transform/spirv.h"
+#include "src/transform/add_empty_entry_point.h"
+#include "src/transform/canonicalize_entry_point_io.h"
+#include "src/transform/external_texture_transform.h"
+#include "src/transform/fold_constants.h"
+#include "src/transform/for_loop_to_loop.h"
+#include "src/transform/inline_pointer_lets.h"
+#include "src/transform/manager.h"
+#include "src/transform/simplify.h"
+#include "src/transform/zero_init_workgroup_memory.h"
#include "src/utils/get_or_create.h"
#include "src/writer/append_vector.h"
@@ -260,6 +268,34 @@
} // namespace
+SanitizedResult Sanitize(const Program* in,
+ bool emit_vertex_point_size,
+ bool disable_workgroup_init) {
+ transform::Manager manager;
+ transform::DataMap data;
+
+ if (!disable_workgroup_init) {
+ manager.Add<transform::ZeroInitWorkgroupMemory>();
+ }
+ manager.Add<transform::InlinePointerLets>(); // Required for arrayLength()
+ manager.Add<transform::Simplify>(); // Required for arrayLength()
+ manager.Add<transform::FoldConstants>();
+ manager.Add<transform::ExternalTextureTransform>();
+ manager.Add<transform::ForLoopToLoop>(); // Must come after
+ // ZeroInitWorkgroupMemory
+ manager.Add<transform::CanonicalizeEntryPointIO>();
+ manager.Add<transform::AddEmptyEntryPoint>();
+
+ data.Add<transform::CanonicalizeEntryPointIO::Config>(
+ transform::CanonicalizeEntryPointIO::Config(
+ transform::CanonicalizeEntryPointIO::ShaderStyle::kSpirv, 0xFFFFFFFF,
+ emit_vertex_point_size));
+
+ SanitizedResult result;
+ result.program = std::move(manager.Run(in, data).program);
+ return result;
+}
+
Builder::AccessorInfo::AccessorInfo() : source_id(0), source_type(nullptr) {}
Builder::AccessorInfo::~AccessorInfo() {}
@@ -270,13 +306,6 @@
Builder::~Builder() = default;
bool Builder::Build() {
- if (!builder_.HasTransformApplied<transform::Spirv>()) {
- error_ =
- "SPIR-V writer requires the transform::Spirv sanitizer to have been "
- "applied to the input program";
- return false;
- }
-
push_capability(SpvCapabilityShader);
push_memory_model(spv::Op::OpMemoryModel,
diff --git a/src/writer/spirv/builder.h b/src/writer/spirv/builder.h
index 59ae87b..8af4cf2 100644
--- a/src/writer/spirv/builder.h
+++ b/src/writer/spirv/builder.h
@@ -51,6 +51,20 @@
namespace writer {
namespace spirv {
+/// The result of sanitizing a program for generation.
+struct SanitizedResult {
+ /// The sanitized program.
+ Program program;
+};
+
+/// Sanitize a program in preparation for generating SPIR-V.
+/// @param emit_vertex_point_size `true` to emit a vertex point size builtin
+/// @param disable_workgroup_init `true` to disable workgroup memory zero
+/// @returns the sanitized program and any supplementary information
+SanitizedResult Sanitize(const Program* program,
+ bool emit_vertex_point_size = false,
+ bool disable_workgroup_init = false);
+
/// Builder class to create SPIR-V instructions from a module.
class Builder {
public:
diff --git a/src/writer/spirv/builder_test.cc b/src/writer/spirv/builder_test.cc
index acf48d6..977cdf3 100644
--- a/src/writer/spirv/builder_test.cc
+++ b/src/writer/spirv/builder_test.cc
@@ -22,25 +22,6 @@
using BuilderTest = TestHelper;
-TEST_F(BuilderTest, ErrorIfSanitizerNotRun) {
- auto program = std::make_unique<Program>(std::move(*this));
- spirv::Builder b(program.get());
- EXPECT_FALSE(b.Build());
- EXPECT_EQ(
- b.error(),
- "SPIR-V writer requires the transform::Spirv sanitizer to have been "
- "applied to the input program");
-}
-
-TEST_F(BuilderTest, InsertsPreamble) {
- spirv::Builder& b = Build();
-
- ASSERT_TRUE(b.Build());
- EXPECT_EQ(DumpBuilder(b), R"(OpCapability Shader
-OpMemoryModel Logical GLSL450
-)");
-}
-
TEST_F(BuilderTest, TracksIdBounds) {
spirv::Builder& b = Build();
diff --git a/src/writer/spirv/generator.cc b/src/writer/spirv/generator.cc
index ec249ac..ee477f0 100644
--- a/src/writer/spirv/generator.cc
+++ b/src/writer/spirv/generator.cc
@@ -14,7 +14,6 @@
#include "src/writer/spirv/generator.h"
-#include "src/transform/spirv.h"
#include "src/writer/spirv/binary_writer.h"
namespace tint {
@@ -28,20 +27,17 @@
Result Generate(const Program* program, const Options& options) {
Result result;
- // Run the SPIR-V sanitizer.
- transform::Spirv sanitizer;
- transform::DataMap transform_input;
- transform_input.Add<transform::Spirv::Config>(options.emit_vertex_point_size,
- options.disable_workgroup_init);
- auto output = sanitizer.Run(program, transform_input);
- if (!output.program.IsValid()) {
+ // Sanitize the program.
+ auto sanitized_result = Sanitize(program, options.emit_vertex_point_size,
+ 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 SPIR-V code.
- auto builder = std::make_unique<Builder>(&output.program);
+ auto builder = std::make_unique<Builder>(&sanitized_result.program);
auto writer = std::make_unique<BinaryWriter>();
if (!builder->Build()) {
result.success = false;
diff --git a/src/writer/spirv/test_helper.h b/src/writer/spirv/test_helper.h
index dd8f5c2..90fa2cb 100644
--- a/src/writer/spirv/test_helper.h
+++ b/src/writer/spirv/test_helper.h
@@ -21,7 +21,6 @@
#include "gtest/gtest.h"
#include "spirv-tools/libspirv.hpp"
-#include "src/transform/spirv.h"
#include "src/writer/spirv/binary_writer.h"
namespace tint {
@@ -43,9 +42,6 @@
if (spirv_builder) {
return *spirv_builder;
}
- // Fake that the SPIR-V sanitizer has been applied, so that we can unit test
- // the writer without it erroring.
- SetTransformApplied<transform::Spirv>();
[&]() {
ASSERT_TRUE(IsValid()) << "Builder program is not valid\n"
<< diag::Formatter().format(Diagnostics());
@@ -77,7 +73,7 @@
ASSERT_TRUE(program->IsValid())
<< diag::Formatter().format(program->Diagnostics());
}();
- auto result = transform::Spirv().Run(program.get());
+ auto result = Sanitize(program.get());
[&]() {
ASSERT_TRUE(result.program.IsValid())
<< diag::Formatter().format(result.program.Diagnostics());
diff --git a/test/BUILD.gn b/test/BUILD.gn
index 203b853..13c97b5 100644
--- a/test/BUILD.gn
+++ b/test/BUILD.gn
@@ -380,7 +380,6 @@
tint_unittests_source_set("tint_unittests_spv_writer_src") {
sources = [
- "../src/transform/spirv_test.cc",
"../src/writer/spirv/binary_writer_test.cc",
"../src/writer/spirv/builder_accessor_expression_test.cc",
"../src/writer/spirv/builder_assign_test.cc",
