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>
This commit is contained in:
James Price 2021-09-09 14:40:07 +00:00
parent 25517e9ce8
commit 5b3be6a64a
11 changed files with 80 additions and 234 deletions

View File

@ -681,8 +681,6 @@ libtint_source_set("libtint_msl_writer_src") {
libtint_source_set("libtint_hlsl_writer_src") { libtint_source_set("libtint_hlsl_writer_src") {
sources = [ sources = [
"transform/hlsl.cc",
"transform/hlsl.h",
"writer/hlsl/generator.cc", "writer/hlsl/generator.cc",
"writer/hlsl/generator.h", "writer/hlsl/generator.h",
"writer/hlsl/generator_impl.cc", "writer/hlsl/generator_impl.cc",

View File

@ -486,8 +486,6 @@ endif()
if(${TINT_BUILD_HLSL_WRITER}) if(${TINT_BUILD_HLSL_WRITER})
list(APPEND TINT_LIB_SRCS list(APPEND TINT_LIB_SRCS
transform/hlsl.cc
transform/hlsl.h
writer/hlsl/generator.cc writer/hlsl/generator.cc
writer/hlsl/generator.h writer/hlsl/generator.h
writer/hlsl/generator_impl.cc writer/hlsl/generator_impl.cc
@ -986,9 +984,6 @@ if(${TINT_BUILD_TESTS})
endif() endif()
if (${TINT_BUILD_HLSL_WRITER}) 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 list(APPEND TINT_TEST_SRCS
writer/hlsl/generator_impl_array_accessor_test.cc writer/hlsl/generator_impl_array_accessor_test.cc
writer/hlsl/generator_impl_assign_test.cc writer/hlsl/generator_impl_assign_test.cc

View File

@ -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

View File

@ -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_

View File

@ -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

View File

@ -14,7 +14,6 @@
#include "src/writer/hlsl/generator.h" #include "src/writer/hlsl/generator.h"
#include "src/transform/hlsl.h"
#include "src/writer/hlsl/generator_impl.h" #include "src/writer/hlsl/generator_impl.h"
namespace tint { namespace tint {
@ -28,27 +27,24 @@ Result::Result(const Result&) = default;
Result Generate(const Program* program, const Options& options) { Result Generate(const Program* program, const Options& options) {
Result result; Result result;
// Run the HLSL sanitizer. // Sanitize the program.
transform::Hlsl sanitizer; auto sanitized_result = Sanitize(program, options.disable_workgroup_init);
transform::DataMap transform_input; if (!sanitized_result.program.IsValid()) {
transform_input.Add<transform::Hlsl::Config>(options.disable_workgroup_init);
auto output = sanitizer.Run(program, transform_input);
if (!output.program.IsValid()) {
result.success = false; result.success = false;
result.error = output.program.Diagnostics().str(); result.error = sanitized_result.program.Diagnostics().str();
return result; return result;
} }
// Generate the HLSL code. // 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.success = impl->Generate();
result.error = impl->error(); result.error = impl->error();
result.hlsl = impl->result(); result.hlsl = impl->result();
// Collect the list of entry points in the sanitized program. // 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()) { 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()}); result.entry_points.push_back({name, func->pipeline_stage()});
} }
} }

View File

@ -42,8 +42,19 @@
#include "src/sem/storage_texture_type.h" #include "src/sem/storage_texture_type.h"
#include "src/sem/struct.h" #include "src/sem/struct.h"
#include "src/sem/variable.h" #include "src/sem/variable.h"
#include "src/transform/add_empty_entry_point.h"
#include "src/transform/calculate_array_length.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/defer.h"
#include "src/utils/get_or_create.h" #include "src/utils/get_or_create.h"
#include "src/utils/scoped_assignment.h" #include "src/utils/scoped_assignment.h"
@ -102,19 +113,51 @@ std::ostream& operator<<(std::ostream& s, const RegisterAndSpace& rs) {
} // namespace } // 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(const Program* program) : TextGenerator(program) {}
GeneratorImpl::~GeneratorImpl() = default; GeneratorImpl::~GeneratorImpl() = default;
bool GeneratorImpl::Generate() { 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; const TypeInfo* last_kind = nullptr;
size_t last_padding_line = 0; size_t last_padding_line = 0;

View File

@ -48,6 +48,18 @@ class Intrinsic;
namespace writer { namespace writer {
namespace hlsl { 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 /// Implementation class for HLSL generator
class GeneratorImpl : public TextGenerator { class GeneratorImpl : public TextGenerator {
public: public:

View File

@ -21,16 +21,6 @@ namespace {
using HlslGeneratorImplTest = TestHelper; 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) { TEST_F(HlslGeneratorImplTest, Generate) {
Func("my_func", ast::VariableList{}, ty.void_(), ast::StatementList{}, Func("my_func", ast::VariableList{}, ty.void_(), ast::StatementList{},
ast::DecorationList{}); ast::DecorationList{});

View File

@ -20,7 +20,6 @@
#include <utility> #include <utility>
#include "gtest/gtest.h" #include "gtest/gtest.h"
#include "src/transform/hlsl.h"
#include "src/transform/manager.h" #include "src/transform/manager.h"
#include "src/transform/renamer.h" #include "src/transform/renamer.h"
#include "src/writer/hlsl/generator_impl.h" #include "src/writer/hlsl/generator_impl.h"
@ -44,9 +43,6 @@ class TestHelperBase : public BODY, public ProgramBuilder {
if (gen_) { if (gen_) {
return *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" ASSERT_TRUE(IsValid()) << "Builder program is not valid\n"
<< diag::Formatter().format(Diagnostics()); << diag::Formatter().format(Diagnostics());
@ -60,7 +56,7 @@ class TestHelperBase : public BODY, public ProgramBuilder {
return *gen_; 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. /// and returns a GeneratorImpl from the sanitized program.
/// @note The generator is only built once. Multiple calls to Build() will /// @note The generator is only built once. Multiple calls to Build() will
/// return the same GeneratorImpl without rebuilding. /// return the same GeneratorImpl without rebuilding.
@ -80,13 +76,19 @@ class TestHelperBase : public BODY, public ProgramBuilder {
<< formatter.format(program->Diagnostics()); << 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::Manager transform_manager;
transform::DataMap transform_data; transform::DataMap transform_data;
transform_data.Add<transform::Renamer::Config>( transform_data.Add<transform::Renamer::Config>(
transform::Renamer::Target::kHlslKeywords); transform::Renamer::Target::kHlslKeywords);
transform_manager.Add<tint::transform::Renamer>(); transform_manager.Add<tint::transform::Renamer>();
transform_manager.Add<tint::transform::Hlsl>(); auto result =
auto result = transform_manager.Run(program.get(), transform_data); transform_manager.Run(&sanitized_result.program, transform_data);
[&]() { [&]() {
ASSERT_TRUE(result.program.IsValid()) ASSERT_TRUE(result.program.IsValid())
<< formatter.format(result.program.Diagnostics()); << formatter.format(result.program.Diagnostics());

View File

@ -586,7 +586,6 @@ tint_unittests_source_set("tint_unittests_msl_writer_src") {
tint_unittests_source_set("tint_unittests_hlsl_writer_src") { tint_unittests_source_set("tint_unittests_hlsl_writer_src") {
sources = [ sources = [
"../src/transform/hlsl_test.cc",
"../src/writer/hlsl/generator_impl_array_accessor_test.cc", "../src/writer/hlsl/generator_impl_array_accessor_test.cc",
"../src/writer/hlsl/generator_impl_assign_test.cc", "../src/writer/hlsl/generator_impl_assign_test.cc",
"../src/writer/hlsl/generator_impl_binary_test.cc", "../src/writer/hlsl/generator_impl_binary_test.cc",