From 58a36249354ead24151b0affd8be9eedb2810d23 Mon Sep 17 00:00:00 2001 From: David Neto Date: Wed, 5 May 2021 09:46:31 +0000 Subject: [PATCH] Add --dump-spirv option to tint_unittests The --dump-spirv option tells tint_unittests to output the SPIR-V assembly text for a module which did not make the SPIR-V reader fail. This lets us get extract a corpus of SPIR-V modules, and lets us more easily verify that the test shaders are valid in the first place. Also: - Add test/extract-spvasm.py to split that output to separate SPIR-V assembly files - Add optional second argument test/test-all.sh to specify a directory look for input files. - BUILD.gn: Add dependency from //test:tint_unittests_main to //test:tint_unittests_config to pick up source dependency on the internal header of the SPIRV-Tools optimizer, needed by the indirection through src/reader/spirv/parser_impl_test_helper.h This is useful for bulk testing Fixed: tint:756 Change-Id: I4fe232ac736003f7d9be35544328302d652381ea Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49605 Auto-Submit: David Neto Commit-Queue: Corentin Wallez Kokoro: Kokoro Reviewed-by: Corentin Wallez Reviewed-by: Ben Clayton --- src/CMakeLists.txt | 2 + src/reader/spirv/parser_impl_test_helper.cc | 41 +++++++++++++ src/reader/spirv/parser_impl_test_helper.h | 30 ++++++++-- src/reader/spirv/spirv_tools_helpers_test.cc | 2 +- src/test_main.cc | 10 ++++ test/BUILD.gn | 4 ++ test/extract-spvasm.py | 62 ++++++++++++++++++++ test/test-all.sh | 28 ++++++--- 8 files changed, 166 insertions(+), 13 deletions(-) create mode 100644 src/reader/spirv/parser_impl_test_helper.cc create mode 100755 test/extract-spvasm.py diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 064c2f944b..85a6f3f9ca 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -625,6 +625,8 @@ if(${TINT_BUILD_TESTS}) reader/spirv/parser_impl_import_test.cc reader/spirv/parser_impl_module_var_test.cc reader/spirv/parser_impl_named_types_test.cc + reader/spirv/parser_impl_test_helper.cc + reader/spirv/parser_impl_test_helper.h reader/spirv/parser_impl_test.cc reader/spirv/parser_impl_user_name_test.cc reader/spirv/parser_test.cc diff --git a/src/reader/spirv/parser_impl_test_helper.cc b/src/reader/spirv/parser_impl_test_helper.cc new file mode 100644 index 0000000000..f6bab45ec5 --- /dev/null +++ b/src/reader/spirv/parser_impl_test_helper.cc @@ -0,0 +1,41 @@ +// 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/reader/spirv/parser_impl_test_helper.h" + +namespace tint { +namespace reader { +namespace spirv { +namespace test { + +// Default to not dumping the SPIR-V assembly. +bool ParserImplWrapperForTest::dump_successfully_converted_spirv_ = false; + +ParserImplWrapperForTest::ParserImplWrapperForTest( + const std::vector& input) + : impl_(input) {} + +ParserImplWrapperForTest::~ParserImplWrapperForTest() { + if (dump_successfully_converted_spirv_ && !impl_.spv_binary().empty() && + impl_.success()) { + std::string disassembly = Disassemble(impl_.spv_binary()); + std::cout << "BEGIN ConvertedOk:\n" + << disassembly << "\nEND ConvertedOk" << std::endl; + } +} + +} // namespace test +} // namespace spirv +} // namespace reader +} // namespace tint diff --git a/src/reader/spirv/parser_impl_test_helper.h b/src/reader/spirv/parser_impl_test_helper.h index 0d9dcc6a03..35cb63583d 100644 --- a/src/reader/spirv/parser_impl_test_helper.h +++ b/src/reader/spirv/parser_impl_test_helper.h @@ -28,6 +28,7 @@ #include "src/reader/spirv/function.h" #include "src/reader/spirv/namer.h" #include "src/reader/spirv/parser_impl.h" +#include "src/reader/spirv/spirv_tools_helpers_test.h" #include "src/reader/spirv/usage.h" namespace tint { @@ -38,8 +39,16 @@ namespace test { // A test class that wraps ParseImpl class ParserImplWrapperForTest { public: - explicit ParserImplWrapperForTest(const std::vector& input) - : impl_(input) {} + // Constructor + explicit ParserImplWrapperForTest(const std::vector& input); + // Dumps SPIR-V if the conversion succeeded, then destroys the wrapper. + ~ParserImplWrapperForTest(); + + // Sets global state to force dumping of the assembly text of succesfully + // SPIR-V. + static void DumpSuccessfullyConvertedSpirv() { + dump_successfully_converted_spirv_ = true; + } // Returns a new function emitter for the given function ID. // Assumes ParserImpl::BuildInternalRepresentation has been run and @@ -57,6 +66,7 @@ class ParserImplWrapperForTest { const std::string error() { return impl_.error(); } FailStream& Fail() { return impl_.Fail(); } spvtools::opt::IRContext* ir_context() { return impl_.ir_context(); } + bool BuildInternalModule() { return impl_.BuildInternalModule(); } bool BuildAndParseInternalModuleExceptFunctions() { return impl_.BuildAndParseInternalModuleExceptFunctions(); @@ -67,9 +77,14 @@ class ParserImplWrapperForTest { bool RegisterUserAndStructMemberNames() { return impl_.RegisterUserAndStructMemberNames(); } + bool RegisterTypes() { return impl_.RegisterTypes(); } + bool RegisterHandleUsage() { return impl_.RegisterHandleUsage(); } + bool EmitModuleScopeVariables() { return impl_.EmitModuleScopeVariables(); } + const std::unordered_set& glsl_std_450_imports() const { return impl_.glsl_std_450_imports(); } + sem::Type* ConvertType(uint32_t id) { return impl_.ConvertType(id); } DecorationList GetDecorationsFor(uint32_t id) const { return impl_.GetDecorationsFor(id); @@ -93,12 +108,9 @@ class ParserImplWrapperForTest { return impl_.GetEntryPointInfo(entry_point); } Usage GetHandleUsage(uint32_t id) const { return impl_.GetHandleUsage(id); } - bool RegisterHandleUsage() { return impl_.RegisterHandleUsage(); } - bool EmitModuleScopeVariables() { return impl_.EmitModuleScopeVariables(); } const spvtools::opt::Instruction* GetInstructionForTest(uint32_t id) const { return impl_.GetInstructionForTest(id); } - bool RegisterTypes() { return impl_.RegisterTypes(); } const ParserImpl::BuiltInPositionInfo& GetBuiltInPositionInfo() { return impl_.GetBuiltInPositionInfo(); } @@ -110,8 +122,15 @@ class ParserImplWrapperForTest { private: ParserImpl impl_; + static bool dump_successfully_converted_spirv_; }; +// Sets global state to force dumping of the assembly text of succesfully +// SPIR-V. +inline void DumpSuccessfullyConvertedSpirv() { + ParserImplWrapperForTest::DumpSuccessfullyConvertedSpirv(); +} + } // namespace test /// SPIR-V Parser test class @@ -127,6 +146,7 @@ class SpvParserTestBase : public T { std::unique_ptr parser( const std::vector& input) { auto parser = std::make_unique(input); + // Don't run the Resolver when building the program. // We're not interested in type information with these tests. parser->builder().SetResolveOnBuild(false); diff --git a/src/reader/spirv/spirv_tools_helpers_test.cc b/src/reader/spirv/spirv_tools_helpers_test.cc index 0c97d0f904..a6b2268be5 100644 --- a/src/reader/spirv/spirv_tools_helpers_test.cc +++ b/src/reader/spirv/spirv_tools_helpers_test.cc @@ -75,7 +75,7 @@ std::string Disassemble(const std::vector& spirv_module) { std::string result; const auto success = tools.Disassemble( - spirv_module, &result, 0 /* no friendly names, so we get raw IDs */); + spirv_module, &result, SPV_BINARY_TO_TEXT_OPTION_FRIENDLY_NAMES); EXPECT_TRUE(success) << errors.str(); return result; diff --git a/src/test_main.cc b/src/test_main.cc index fa81891fbb..d4425c7f07 100644 --- a/src/test_main.cc +++ b/src/test_main.cc @@ -13,6 +13,7 @@ // limitations under the License. #include "gmock/gmock.h" +#include "src/reader/spirv/parser_impl_test_helper.h" #include "src/utils/command.h" #include "src/writer/hlsl/test_helper.h" #include "src/writer/msl/test_helper.h" @@ -28,6 +29,7 @@ struct Flags { std::string dxc_path; bool validate_msl = false; std::string xcrun_path; + bool spirv_reader_dump_converted = false; bool parse(int argc, char** argv) { bool errored = false; @@ -53,6 +55,8 @@ struct Flags { } else if (match("--validate-msl") || parse_value("--xcrun-path", xcrun_path)) { validate_msl = true; + } else if (match("--dump-spirv")) { + spirv_reader_dump_converted = true; } else { std::cout << "Unknown flag '" << argv[i] << "'" << std::endl; return false; @@ -122,6 +126,12 @@ int main(int argc, char** argv) { } #endif // TINT_BUILD_MSL_WRITER +#if TINT_BUILD_SPV_READER + if (flags.spirv_reader_dump_converted) { + tint::reader::spirv::test::DumpSuccessfullyConvertedSpirv(); + } +#endif // TINT_BUILD_SPV_READER + tint::SetInternalCompilerErrorReporter(&TintInternalCompilerErrorReporter); auto res = RUN_ALL_TESTS(); diff --git a/test/BUILD.gn b/test/BUILD.gn index c7bed0386a..96dabc3ff5 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -88,10 +88,12 @@ source_set("tint_unittests_main") { sources = [ "//gpu/tint_unittests_main.cc" ] } else { sources = [ "../src/test_main.cc" ] + configs += [ ":tint_unittests_config" ] deps += [ ":tint_test_helpers", ":tint_unittests_hlsl_writer_src", ":tint_unittests_msl_writer_src", + ":tint_unittests_spv_reader_src", "${tint_root_dir}/src:libtint", ] } @@ -343,6 +345,8 @@ tint_unittests_source_set("tint_unittests_spv_reader_src") { "../src/reader/spirv/parser_impl_module_var_test.cc", "../src/reader/spirv/parser_impl_named_types_test.cc", "../src/reader/spirv/parser_impl_test.cc", + "../src/reader/spirv/parser_impl_test_helper.cc", + "../src/reader/spirv/parser_impl_test_helper.h", "../src/reader/spirv/parser_impl_user_name_test.cc", "../src/reader/spirv/parser_test.cc", "../src/reader/spirv/spirv_tools_helpers_test.cc", diff --git a/test/extract-spvasm.py b/test/extract-spvasm.py new file mode 100755 index 0000000000..acc563507a --- /dev/null +++ b/test/extract-spvasm.py @@ -0,0 +1,62 @@ +#!/usr/bin/env python3 + +# 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. + +# Extract SPIR-V assembly dumps from the output of +# tint_unittests --dump-spirv +# Writes each module to a distinct filename, which is a sanitized +# form of the test name, and with a ".spvasm" suffix. +# +# Usage: +# tint_unittests --dump-spirv | python3 extract-spvasm.py + + +import sys +import re + +def extract(): + test_name = '' + in_spirv = False + parts = [] + for line in sys.stdin: + run_match = re.match('\[ RUN\s+\]\s+(\S+)', line) + if run_match: + test_name = run_match.group(1) + test_name = re.sub('[^0-9a-zA-Z]', '_', test_name) + '.spvasm' + elif re.match('BEGIN ConvertedOk', line): + parts = [] + in_spirv = True + elif re.match('END ConvertedOk', line): + with open(test_name, 'w') as f: + f.write('; Test: ' + test_name + '\n') + for l in parts: + f.write(l) + f.close() + elif in_spirv: + parts.append(line) + +def main(argv): + if '--help' in argv or '-h' in argv: + print('Extract SPIR-V from the output of tint_unittests --dump-spirv\n') + print('Usage:\n tint_unittests --dump-spirv | python3 extract-spvasm.py\n') + print('Writes each module to a distinct filename, which is a sanitized') + print('form of the test name, and with a ".spvasm" suffix.') + return 1 + else: + extract() + return 0 + +if __name__ == '__main__': + exit(main(sys.argv[1:])) diff --git a/test/test-all.sh b/test/test-all.sh index db98d5bfa6..35ba00746d 100755 --- a/test/test-all.sh +++ b/test/test-all.sh @@ -21,14 +21,15 @@ TEXT_GREEN="\033[0;32m" TEXT_RED="\033[0;31m" TEXT_DEFAULT="\033[0m" -TINT=$1 +TINT="$1" +SUBDIR="$2" if [ ! -x "$TINT" ]; then echo "test-all.sh compiles with tint all the .wgsl files in the tint/test" echo "directory, for each of the SPIR-V, MSL, HLSL and WGSL backends." echo "Any errors are reported as test failures." echo "" - echo "Usage: test-all.sh " + echo "Usage: test-all.sh []" exit 1 fi @@ -54,7 +55,7 @@ function should_skip() { # check(TEST_FILE, FORMAT) function check() { - local TEST_FILE=$1 + local TEST_FILE="$1" local FORMAT=$2 SKIP= @@ -69,7 +70,7 @@ function check() { return fi set +e - ${TINT} ${SCRIPT_DIR}/${TEST_FILE} --format ${FORMAT} -o /dev/null + "${TINT}" ${SCRIPT_DIR}/${TEST_FILE} --format ${FORMAT} -o /dev/null if [ $? -eq 0 ]; then echo -e "${TEXT_GREEN}PASS${TEXT_DEFAULT}" NUM_PASS=$((${NUM_PASS}+1)) @@ -80,21 +81,34 @@ function check() { set -e } -for TEST_FILE in ${SCRIPT_DIR}/*.spvasm ${SCRIPT_DIR}/*.wgsl -do +# check_formats(TEST_FILE) +function check_formats() { + local TEST_FILE="$1" if [ -x realpath ]; then TEST_FILE=$(realpath --relative-to="$SCRIPT_DIR" "$TEST_FILE") else TEST_FILE=$(echo -n "$TEST_FILE"| sed -e "s'${SCRIPT_DIR}/*''") fi echo - echo "Testing $TEST_FILE..." + echo "Testing ${TEST_FILE}..." check "${TEST_FILE}" wgsl check "${TEST_FILE}" spirv check "${TEST_FILE}" msl check "${TEST_FILE}" hlsl +} + +for F in ${SCRIPT_DIR}/*.spvasm ${SCRIPT_DIR}/*.wgsl +do + check_formats "$F" done +if [ -d "${SUBDIR}" ]; then + for F in "${SUBDIR}"/*; + do + check_formats "$F" + done +fi + if [ ${NUM_FAIL} -ne 0 ]; then echo echo -e "${TEXT_RED}${NUM_FAIL} tests failed. ${TEXT_DEFAULT}${NUM_SKIP} skipped. ${NUM_PASS} passed.${TEXT_DEFAULT}"