Skip to content

Commit

Permalink
[spirv][ir] Simplify test helper.
Browse files Browse the repository at this point in the history 
This CL simplifies the test helper code to directly call the
`writer::Generate` method. The `Module` is removed from the helper and
tests are updated to just use the SPIR-V disassembly.

Change-Id: I03b4e3c4b793b6c7bb030d67896f1028ba8bc61e
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/203534
Reviewed-by: James Price <[email protected]>
Commit-Queue: dan sinclair <[email protected]>
  • Loading branch information
@dj2
dj2 authored and Dawn LUCI CQ committed Aug 22, 2024
1 parent 9cef842 commit 5196d11
Show file tree
Hide file tree
Showing 9 changed files with 34 additions and 57 deletions.
3 changes: 1 addition & 2 deletions src/tint/lang/spirv/writer/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,10 +136,9 @@ cc_library(
"//conditions:default": [],
}) + select({
":tint_build_spv_writer": [
"//src/tint/lang/spirv/writer",
"//src/tint/lang/spirv/writer/common",
"//src/tint/lang/spirv/writer/common:test",
"//src/tint/lang/spirv/writer/printer",
"//src/tint/lang/spirv/writer/raise",
],
"//conditions:default": [],
}),
Expand Down
3 changes: 1 addition & 2 deletions src/tint/lang/spirv/writer/BUILD.cmake
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,9 @@ endif(TINT_BUILD_SPV_READER OR TINT_BUILD_SPV_WRITER)

if(TINT_BUILD_SPV_WRITER)
tint_target_add_dependencies(tint_lang_spirv_writer_test test
tint_lang_spirv_writer
tint_lang_spirv_writer_common
tint_lang_spirv_writer_common_test
tint_lang_spirv_writer_printer
tint_lang_spirv_writer_raise
)
endif(TINT_BUILD_SPV_WRITER)

Expand Down
3 changes: 1 addition & 2 deletions src/tint/lang/spirv/writer/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,10 +142,9 @@ if (tint_build_unittests) {

if (tint_build_spv_writer) {
deps += [
"${tint_src_dir}/lang/spirv/writer",
"${tint_src_dir}/lang/spirv/writer/common",
"${tint_src_dir}/lang/spirv/writer/common:unittests",
"${tint_src_dir}/lang/spirv/writer/printer",
"${tint_src_dir}/lang/spirv/writer/raise",
]
}
}
Expand Down
3 changes: 1 addition & 2 deletions src/tint/lang/spirv/writer/common/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,8 @@ cc_library(
"//conditions:default": [],
}) + select({
":tint_build_spv_writer": [
"//src/tint/lang/spirv/writer",
"//src/tint/lang/spirv/writer/common",
"//src/tint/lang/spirv/writer/printer",
"//src/tint/lang/spirv/writer/raise",
],
"//conditions:default": [],
}),
Expand Down
3 changes: 1 addition & 2 deletions src/tint/lang/spirv/writer/common/BUILD.cmake
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,8 @@ endif(TINT_BUILD_SPV_READER OR TINT_BUILD_SPV_WRITER)

if(TINT_BUILD_SPV_WRITER)
tint_target_add_dependencies(tint_lang_spirv_writer_common_test test
tint_lang_spirv_writer
tint_lang_spirv_writer_common
tint_lang_spirv_writer_printer
tint_lang_spirv_writer_raise
)
endif(TINT_BUILD_SPV_WRITER)

Expand Down
3 changes: 1 addition & 2 deletions src/tint/lang/spirv/writer/common/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,8 @@ if (tint_build_unittests) {

if (tint_build_spv_writer) {
deps += [
"${tint_src_dir}/lang/spirv/writer",
"${tint_src_dir}/lang/spirv/writer/common",
"${tint_src_dir}/lang/spirv/writer/printer",
"${tint_src_dir}/lang/spirv/writer/raise",
]
}
}
Expand Down
35 changes: 7 additions & 28 deletions src/tint/lang/spirv/writer/common/helper_test.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@
#include "src/tint/lang/core/type/sampled_texture.h"
#include "src/tint/lang/core/type/storage_texture.h"
#include "src/tint/lang/spirv/writer/common/spv_dump_test.h"
#include "src/tint/lang/spirv/writer/printer/printer.h"
#include "src/tint/lang/spirv/writer/raise/raise.h"
#include "src/tint/lang/spirv/writer/writer.h"

namespace tint::spirv::writer {

Expand Down Expand Up @@ -104,44 +103,27 @@ class SpirvWriterTestHelperBase : public BASE {
/// SPIR-V output.
std::string output_;

/// The generated SPIR-V
writer::Module spirv_;

/// @returns the error string from the validation
std::string Error() const { return err_; }

/// Run the printer on the IR module and validate the result.
/// @param options the optional writer options to use when raising the IR
/// @param zero_init_workgroup_memory `true` to initialize all the variables in the Workgroup
/// storage class with OpConstantNull
/// @returns true if generation and validation succeeded
bool Generate(Options options = {}, bool zero_init_workgroup_memory = false) {
auto raised = Raise(mod, options);
if (raised != Success) {
err_ = raised.Failure().reason.Str();
return false;
}

if (zero_init_workgroup_memory) {
options.disable_workgroup_init = false;
options.use_zero_initialize_workgroup_memory_extension = true;
}

auto spirv = PrintModule(mod, options);
if (spirv != Success) {
err_ = spirv.Failure().reason.Str();
bool Generate(Options options = {}) {
auto result = writer::Generate(mod, options);
if (result != Success) {
err_ = result.Failure().reason.Str();
return false;
}

output_ = Disassemble(spirv->Code(), SPV_BINARY_TO_TEXT_OPTION_FRIENDLY_NAMES |
output_ = Disassemble(result->spirv, SPV_BINARY_TO_TEXT_OPTION_FRIENDLY_NAMES |
SPV_BINARY_TO_TEXT_OPTION_INDENT |
SPV_BINARY_TO_TEXT_OPTION_COMMENT);

if (!Validate(spirv->Code())) {
if (!Validate(result->spirv)) {
return false;
}

spirv_ = std::move(spirv.Get());
return true;
}

Expand Down Expand Up @@ -180,9 +162,6 @@ class SpirvWriterTestHelperBase : public BASE {
return result;
}

/// @returns the disassembled types from the generated module.
std::string DumpTypes() { return DumpInstructions(spirv_.Types()); }

/// Helper to make a scalar type corresponding to the element type `type`.
/// @param type the element type
/// @returns the scalar type
Expand Down
32 changes: 16 additions & 16 deletions src/tint/lang/spirv/writer/type_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -414,14 +414,14 @@ TEST_F(SpirvWriterTest, Type_DepthTexture_DedupWithSampledTexture) {
});

ASSERT_TRUE(Generate()) << Error() << output_;
EXPECT_EQ(DumpTypes(), R"(%4 = OpTypeFloat 32
%3 = OpTypeImage %4 2D 0 0 0 1 Unknown
%2 = OpTypePointer UniformConstant %3
%1 = OpVariable %2 UniformConstant
%6 = OpTypePointer UniformConstant %3
%5 = OpVariable %6 UniformConstant
%8 = OpTypeVoid
%9 = OpTypeFunction %8
EXPECT_INST(R"( %float = OpTypeFloat 32
%3 = OpTypeImage %float 2D 0 0 0 1 Unknown
%_ptr_UniformConstant_3 = OpTypePointer UniformConstant %3
%v1 = OpVariable %_ptr_UniformConstant_3 UniformConstant ; DescriptorSet 0, Binding 1, Coherent
%_ptr_UniformConstant_3_0 = OpTypePointer UniformConstant %3
%v2 = OpVariable %_ptr_UniformConstant_3_0 UniformConstant ; DescriptorSet 0, Binding 2, Coherent
%void = OpTypeVoid
%9 = OpTypeFunction %void
)");
}

Expand All @@ -447,14 +447,14 @@ TEST_F(SpirvWriterTest, Type_DepthMultisampledTexture_DedupWithMultisampledTextu
});

ASSERT_TRUE(Generate()) << Error() << output_;
EXPECT_EQ(DumpTypes(), R"(%4 = OpTypeFloat 32
%3 = OpTypeImage %4 2D 0 0 1 1 Unknown
%2 = OpTypePointer UniformConstant %3
%1 = OpVariable %2 UniformConstant
%6 = OpTypePointer UniformConstant %3
%5 = OpVariable %6 UniformConstant
%8 = OpTypeVoid
%9 = OpTypeFunction %8
EXPECT_INST(R"( %float = OpTypeFloat 32
%3 = OpTypeImage %float 2D 0 0 1 1 Unknown
%_ptr_UniformConstant_3 = OpTypePointer UniformConstant %3
%v1 = OpVariable %_ptr_UniformConstant_3 UniformConstant ; DescriptorSet 0, Binding 1, Coherent
%_ptr_UniformConstant_3_0 = OpTypePointer UniformConstant %3
%v2 = OpVariable %_ptr_UniformConstant_3_0 UniformConstant ; DescriptorSet 0, Binding 2, Coherent
%void = OpTypeVoid
%9 = OpTypeFunction %void
)");
}

Expand Down
6 changes: 5 additions & 1 deletion src/tint/lang/spirv/writer/var_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,12 @@ TEST_F(SpirvWriterTest, WorkgroupVar_LoadAndStore) {
TEST_F(SpirvWriterTest, WorkgroupVar_ZeroInitializeWithExtension) {
mod.root_block->Append(b.Var("v", ty.ptr<workgroup, i32>()));

Options opts{};
opts.disable_workgroup_init = false;
opts.use_zero_initialize_workgroup_memory_extension = true;

// Create a writer with the zero_init_workgroup_memory flag set to `true`.
ASSERT_TRUE(Generate({}, /* zero_init_workgroup_memory */ true)) << Error() << output_;
ASSERT_TRUE(Generate(opts)) << Error() << output_;
EXPECT_INST("%4 = OpConstantNull %int");
EXPECT_INST("%v = OpVariable %_ptr_Workgroup_int Workgroup %4");
}
Expand Down

0 comments on commit 5196d11

Please sign in to comment.