From d13982ff2e4d252b0f9935eccbe8c809010904fd Mon Sep 17 00:00:00 2001 From: Arman Uguray Date: Wed, 14 Apr 2021 15:36:58 +0000 Subject: [PATCH] [msl-writer] Support MSL compiler validation in unit tests The MSL writer GTest harness (TestHelper) now provides a function to invoke the XCode SDK Metal compiler for the MSL output of a given tint::Program. The tint_unittests binary now provides the `--validate-msl` and `--xcrun-path` command-line flags to optionally enable MSL validation and to configure its path. The MSL validation logic itself is conditionally compiled based on the TINT_BUILD_MSL_WRITER define. The TINT_BUILD_* flags were previously not propagated to the GTest binary which this CL addresses by linking the common/public tint configs when building the tint_unittests_main target. Bug: tint:535 Fixed: tint:696 Change-Id: I08b1c36ba59c606ef6cffa5fa5454fd8cf8b035d Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/45800 Kokoro: Kokoro Reviewed-by: Ben Clayton Commit-Queue: Arman Uguray --- src/CMakeLists.txt | 1 + src/test_main.cc | 31 +++++++++++++ src/writer/msl/test_helper.cc | 82 +++++++++++++++++++++++++++++++++++ src/writer/msl/test_helper.h | 37 ++++++++++++++++ test/BUILD.gn | 2 + 5 files changed, 153 insertions(+) create mode 100644 src/writer/msl/test_helper.cc diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index a691c6260f..a3c6828c9b 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -774,6 +774,7 @@ if(${TINT_BUILD_TESTS}) writer/msl/generator_impl_type_test.cc writer/msl/generator_impl_unary_op_test.cc writer/msl/generator_impl_variable_decl_statement_test.cc + writer/msl/test_helper.cc writer/msl/test_helper.h ) endif() diff --git a/src/test_main.cc b/src/test_main.cc index 8e56b05305..fa81891fbb 100644 --- a/src/test_main.cc +++ b/src/test_main.cc @@ -15,6 +15,7 @@ #include "gmock/gmock.h" #include "src/utils/command.h" #include "src/writer/hlsl/test_helper.h" +#include "src/writer/msl/test_helper.h" namespace { @@ -25,6 +26,8 @@ void TintInternalCompilerErrorReporter(const tint::diag::List& diagnostics) { struct Flags { bool validate_hlsl = false; std::string dxc_path; + bool validate_msl = false; + std::string xcrun_path; bool parse(int argc, char** argv) { bool errored = false; @@ -47,6 +50,9 @@ struct Flags { if (match("--validate-hlsl") || parse_value("--dxc-path", dxc_path)) { validate_hlsl = true; + } else if (match("--validate-msl") || + parse_value("--xcrun-path", xcrun_path)) { + validate_msl = true; } else { std::cout << "Unknown flag '" << argv[i] << "'" << std::endl; return false; @@ -91,6 +97,31 @@ int main(int argc, char** argv) { } #endif // TINT_BUILD_HLSL_WRITER +#if TINT_BUILD_MSL_WRITER + // This must be kept alive for the duration of RUN_ALL_TESTS() as the c_str() + // is passed into tint::writer::msl::EnableMSLValidation(), which does not + // make a copy. This is to work around Chromium's strict rules on globals + // having no constructors / destructors. + std::string xcrun_path; + if (flags.validate_msl) { + auto xcrun = flags.xcrun_path.empty() + ? tint::utils::Command::LookPath("xcrun") + : tint::utils::Command(flags.xcrun_path); + + if (!xcrun.Found()) { + std::cout << "xcrun executable not found" << std::endl; + return -1; + } + + std::cout << "MSL validation with XCode SDK enabled" << std::endl; + + xcrun_path = xcrun.Path(); + tint::writer::msl::EnableMSLValidation(xcrun_path.c_str()); + } else { + std::cout << "MSL validation with XCode SDK is not enabled" << std::endl; + } +#endif // TINT_BUILD_MSL_WRITER + tint::SetInternalCompilerErrorReporter(&TintInternalCompilerErrorReporter); auto res = RUN_ALL_TESTS(); diff --git a/src/writer/msl/test_helper.cc b/src/writer/msl/test_helper.cc new file mode 100644 index 0000000000..87c10a28e9 --- /dev/null +++ b/src/writer/msl/test_helper.cc @@ -0,0 +1,82 @@ +// 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/writer/msl/test_helper.h" + +#include "src/utils/command.h" +#include "src/utils/tmpfile.h" + +namespace tint { +namespace writer { +namespace msl { + +namespace { + +const char* xcrun_path = nullptr; + +} // namespace + +void EnableMSLValidation(const char* xcrun) { + xcrun_path = xcrun; +} + +CompileResult Compile(Program* program) { + CompileResult result; + + if (!xcrun_path) { + result.status = CompileResult::Status::kVerificationNotEnabled; + return result; + } + + auto xcrun = utils::Command(xcrun_path); + if (!xcrun.Found()) { + result.output = "xcrun not found at '" + std::string(xcrun_path) + "'"; + result.status = CompileResult::Status::kFailed; + return result; + } + + auto gen = std::make_unique(program); + if (!gen->Generate()) { + result.output = gen->error(); + result.status = CompileResult::Status::kFailed; + return result; + } + result.msl = gen->result(); + + utils::TmpFile file(".metal"); + file << result.msl; + + auto xcrun_res = + xcrun("-sdk", "macosx", "metal", "-o", "/dev/null", "-c", file.Path()); + if (!xcrun_res.out.empty()) { + if (!result.output.empty()) { + result.output += "\n"; + } + result.output += xcrun_res.out; + } + if (!xcrun_res.err.empty()) { + if (!result.output.empty()) { + result.output += "\n"; + } + result.output += xcrun_res.err; + } + result.status = (xcrun_res.error_code == 0) ? CompileResult::Status::kSuccess + : CompileResult::Status::kFailed; + + return result; +} + +} // namespace msl +} // namespace writer +} // namespace tint diff --git a/src/writer/msl/test_helper.h b/src/writer/msl/test_helper.h index 37d5200e87..0e85ae0514 100644 --- a/src/writer/msl/test_helper.h +++ b/src/writer/msl/test_helper.h @@ -16,6 +16,7 @@ #define SRC_WRITER_MSL_TEST_HELPER_H_ #include +#include #include #include "gtest/gtest.h" @@ -27,6 +28,28 @@ namespace tint { namespace writer { namespace msl { +/// Enables verification of MSL shaders by running the Metal compiler and +/// checking no errors are reported. +/// @param xcrun_path the path to the `xcrun` executable +void EnableMSLValidation(const char* xcrun_path); + +/// The return structure of Compile() +struct CompileResult { + /// Status is an enumerator of status codes from Compile() + enum class Status { kSuccess, kFailed, kVerificationNotEnabled }; + /// The resulting status of the compilation + Status status; + /// Output of the Metal compiler + std::string output; + /// The MSL source that was compiled + std::string msl; +}; + +/// Compile attempts to compile the shader with xcrun if found on PATH. +/// @param program the MSL program +/// @return the result of the compile +CompileResult Compile(Program* program); + /// Helper class for testing template class TestHelperBase : public BASE, public ProgramBuilder { @@ -84,6 +107,20 @@ class TestHelperBase : public BASE, public ProgramBuilder { return *gen_; } + /// Validate generates MSL code for the current contents of `program` and + /// passes the output of the generator to the XCode SDK Metal compiler. + /// + /// If the Metal compiler finds problems, then any GTest test case that + /// invokes this function test will fail. + /// This function does nothing, if the Metal compiler path has not been + /// configured by calling `EnableMSLValidation()`. + void Validate() { + auto res = Compile(program.get()); + if (res.status == CompileResult::Status::kFailed) { + FAIL() << "MSL Validation failed.\n\n" << res.msl << "\n\n" << res.output; + } + } + /// The program built with a call to Build() std::unique_ptr program; diff --git a/test/BUILD.gn b/test/BUILD.gn index 43e014bdfe..ccf6d9f221 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -88,6 +88,7 @@ source_set("tint_unittests_main") { sources = [ "//gpu/tint_unittests_main.cc" ] } else { sources = [ "../src/test_main.cc" ] + deps += [ "${tint_root_dir}/src:libtint" ] } } @@ -538,6 +539,7 @@ source_set("tint_unittests_msl_writer_src") { "../src/writer/msl/generator_impl_type_test.cc", "../src/writer/msl/generator_impl_unary_op_test.cc", "../src/writer/msl/generator_impl_variable_decl_statement_test.cc", + "../src/writer/msl/test_helper.cc", "../src/writer/msl/test_helper.h", ]