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 <dneto@google.com>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Ben Clayton <bclayton@google.com>

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

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<uint32_t>& 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

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<uint32_t>& input)
+  // Constructor
-      : impl_(input) {}
+  explicit ParserImplWrapperForTest(const std::vector<uint32_t>& 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<uint32_t>& 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<test::ParserImplWrapperForTest> parser(
       const std::vector<uint32_t>& input) {
     auto parser = std::make_unique<test::ParserImplWrapperForTest>(input);
+
     // Don't run the Resolver when building the program.
     // We're not interested in type information with these tests.
     parser->builder().SetResolveOnBuild(false);

src/reader/spirv/spirv_tools_helpers_test.cc

@@ -75,7 +75,7 @@ std::string Disassemble(const std::vector<uint32_t>& 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;

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();

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",

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:]))

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 <path-to-tint-executable>"
+    echo "Usage: test-all.sh <path-to-tint-executable> [<subdir-with-more-samples>]"
     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
+# check_formats(TEST_FILE)
-do
+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}"