Tests: Add optional dependency on Effcee stateful matcher

Add Effcee as an optional dependency for use in tests. In future it will be a required dependency. Effcee is a stateful pattern matcher that has much of the functionality of LLVM's FileCheck, except in library form. Effcee makes it much easier to write tests for optimization passes. Demonstrate its use in a test for the strength-reduction pass. Update README.md with example commands of how to get sources. Update Appveyor and Travis-CI build rules. Also: Include test libraries if not SPIRV_SKIP_TESTS - SPIRV_SKIP_TESTS is implied by SPIRV_SKIP_EXECUTABLES
2024-10-18 11:10:05 +00:00 · 2017-11-08 00:23:42 -05:00 · 2017-11-08 00:23:42 -05:00 · 76555bd4ba
commit 76555bd4ba
parent efe12ff5a1
9 changed files with 150 additions and 27 deletions
--- a/.appveyor.yml
+++ b/.appveyor.yml
@ -29,6 +29,8 @@ matrix:
 install:
  - git clone https://github.com/KhronosGroup/SPIRV-Headers.git external/spirv-headers
  - git clone https://github.com/google/googletest.git external/googletest
+  - git clone https://github.com/google/effcee.git external/effcee
+  - git clone https://github.com/google/re2.git external/re2

 build:
  parallel: true  # enable MSBuild parallel builds
--- a/.travis.yml
+++ b/.travis.yml
@ -56,6 +56,8 @@ before_install:
 before_script:
  - git clone --depth=1 https://github.com/KhronosGroup/SPIRV-Headers external/spirv-headers
  - git clone --depth=1 https://github.com/google/googletest          external/googletest
+  - git clone --depth=1 https://github.com/google/effcee              external/effcee
+  - git clone --depth=1 https://github.com/google/re2                 external/re2

 script:
  # Due to the limitation of Travis platform, we cannot start too many concurrent jobs.
--- a/3
+++ b/3
@ -3,6 +3,9 @@ Revision history for SPIRV-Tools
 v2017.1-dev 2017-09-01
 - Update README with details on the public_spirv_tools_dev@khronos.org mailing list.
 - General:
+   - Add the 'effcee' library as an optional dependency for use in tests.
+     Eventually it will be a required dependency, once downstream projects have
+     a chance to adjust.
   - Avoid static-duration variables of class type (with constructors).
 - Validator:
   - Type check basic arithmetic operations
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@ -182,6 +182,10 @@ endif()

 add_subdirectory(external)

+if (TARGET effcee)
+  add_definitions(-DSPIRV_EFFCEE)
+endif()
+
 add_subdirectory(source)
 add_subdirectory(tools)

--- a/README.md
+++ b/README.md
@ -171,6 +171,11 @@ We intend to maintain a linear history on the GitHub `master` branch.
 * `example`: demo code of using SPIRV-Tools APIs
 * `external/googletest`: Intended location for the
  [googletest][googletest] sources, not provided
+* `external/effcee`: Location of [Effcee][effcee] sources, if the `effcee` library
+  is not already configured by an enclosing project.
+* `external/re2`: Location of [RE2][re2] sources, if the `effcee` library is not already
+  configured by an enclosing project.
+  (The Effcee project already requires RE2.)
 * `include/`: API clients should add this directory to the include search path
 * `external/spirv-headers`: Intended location for
  [SPIR-V headers][spirv-headers], not provided
@ -179,6 +184,14 @@ We intend to maintain a linear history on the GitHub `master` branch.
 * `test/`: Tests, using the [googletest][googletest] framework
 * `tools/`: Command line executables

+Example of getting sources, assuming SPIRV-Tools is configured as a standalone project:
+
+    git clone https://github.com/KhronosGroup/SPIRV-Tools.git   spirv-tools
+    git clone https://github.com/KhronosGroup/SPIRV-Headers.git spirv-tools/external/spirv-headers
+    git clone https://github.com/google/googletest.git          spirv-tools/external/googletest
+    git clone https://github.com/google/effcee.git              spirv-tools/external/effcee
+    git clone https://github.com/google/re2.git                 spirv-tools/external/re2
+
 ### Tests

 The project contains a number of tests, used to drive development
@ -197,6 +210,18 @@ tests:
 The fix is included on the googletest master branch any time after 2015-11-10.
 In particular, googletest must be newer than version 1.7.0.

+### Optional dependency on Effcee
+
+Some tests depend on the [Effcee][effcee] library for stateful matching.
+Effcee itself depends on [RE2][re2].
+
+* If SPIRV-Tools is configured as part of a larger project that already uses
+  Effcee, then that project should include Effcee before SPIRV-Tools.
+* Otherwise, SPIRV-Tools expects Effcee sources to appear in `external/effcee`
+  and RE2 sources to appear in `external/re2`.
+
+Currently Effcee is an optional dependency, but soon it will be required.
+
 ## Build

 The project uses [CMake][cmake] to generate platform-specific build
@ -470,6 +495,8 @@ limitations under the License.
 [googletest]: https://github.com/google/googletest
 [googletest-pull-612]: https://github.com/google/googletest/pull/612
 [googletest-issue-610]: https://github.com/google/googletest/issues/610
+[effcee]: https://github.com/google/effcee
+[re2]: https://github.com/google/re2
 [CMake]: https://cmake.org/
 [cpp-style-guide]: https://google.github.io/styleguide/cppguide.html
 [clang-sanitizers]: http://clang.llvm.org/docs/UsersManual.html#controlling-code-generation
--- a/external/CMakeLists.txt
+++ b/external/CMakeLists.txt
@ -27,7 +27,7 @@ else()
    "SPIRV-Headers was not found - please checkout a copy under external/.")
 endif()

-if (NOT ${SPIRV_SKIP_EXECUTABLES})
+if (NOT ${SPIRV_SKIP_TESTS})
  # Find gmock if we can. If it's not already configured, then try finding
  # it in external/googletest.
  if (TARGET gmock)
@ -54,4 +54,48 @@ if (NOT ${SPIRV_SKIP_EXECUTABLES})
      set_property(TARGET ${target} PROPERTY FOLDER GoogleTest)
    endforeach()
  endif()
+
+  # Find Effcee and RE2, for testing.
+  # Optional for now, but eventually we'll make this required.
+
+  # First find RE2, since Effcee depends on it.
+  # If already configured, then use that.  Otherwise, prefer to find it under 're2'
+  # in this directory.
+  if (NOT TARGET re2)
+    # If we are configuring RE2, then turn off its testing.  It takes a long time and
+    # does not add much value for us.  If an enclosing project configured RE2, then it
+    # has already chosen whether to enable RE2 tesitng.
+    set(RE2_BUILD_TESTING OFF CACHE STRING "Run RE2 Tests")
+    if (NOT RE2_SOURCE_DIR)
+      if (EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/re2)
+        set(RE2_SOURCE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/re2" CACHE STRING "RE2 source dir" )
+      endif()
+    endif()
+  endif()
+
+  if (NOT TARGET effcee)
+    # Expect to find effcee in this directory.
+    if (EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/effcee)
+      # If we're configuring RE2 (via Effcee), then turn off RE2 testing.
+      if (NOT TARGET re2)
+        set(RE2_BUILD_TESTING OFF)
+      endif()
+      if (MSVC)
+        # SPIRV-Tools uses the shared CRT with MSVC.  Tell Effcee to do the same.
+        set(EFFCEE_ENABLE_SHARED_CRT ON)
+      endif()
+      add_subdirectory(effcee)
+      set_property(TARGET effcee PROPERTY FOLDER Effcee)
+      # Turn off warnings for effcee and re2
+      set_property(TARGET effcee APPEND PROPERTY COMPILE_OPTIONS -w)
+      set_property(TARGET re2 APPEND PROPERTY COMPILE_OPTIONS -w)
+    endif()
+  endif()
+  # TODO(dneto): Eventually, require this.
+  if (TARGET effcee)
+    message(STATUS "SPIRV-Tools: Effcee is configured")
+  else()
+    message(STATUS "SPIRV-Tools: Effcee is not configured.  Skipping Effcee-based tests.")
+  endif()
+
 endif()
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@ -39,6 +39,8 @@ function(add_spvtools_unittest)
    spvtools_default_compile_options(${target})
    if(${COMPILER_IS_LIKE_GNU})
      target_compile_options(${target} PRIVATE -Wno-undef)
+      # Effcee and RE2 headers exhibit shadowing.
+      target_compile_options(${target} PRIVATE -Wno-shadow)
    endif()
    if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "MSVC")
      # Disable C4503 "decorated name length exceeded" warning,
@ -56,7 +58,14 @@ function(add_spvtools_unittest)
      ${gtest_SOURCE_DIR}/include
      ${gmock_SOURCE_DIR}/include
    )
+    if (TARGET effcee)
+      # If using Effcee for testing, then add its include directory.
+      target_include_directories(${target} PRIVATE ${effcee_SOURCE_DIR})
+    endif()
    target_link_libraries(${target} PRIVATE ${ARG_LIBS})
+    if (TARGET effcee)
+      target_link_libraries(${target} PRIVATE effcee)
+    endif()
    target_link_libraries(${target} PRIVATE gmock_main)
    add_test(NAME spirv-tools-${target} COMMAND ${target})
    set_property(TARGET ${target} PROPERTY FOLDER "SPIRV-Tools tests")
--- a/test/opt/pass_fixture.h
+++ b/test/opt/pass_fixture.h
@ -22,6 +22,10 @@

 #include <gtest/gtest.h>

+#ifdef SPIRV_EFFCEE
+#include "effcee/effcee.h"
+#endif
+
 #include "opt/build_module.h"
 #include "opt/make_unique.h"
 #include "opt/pass_manager.h"
@ -149,6 +153,25 @@ class PassTest : public TestT {
      std::forward<Args>(args)...);
  }

+#ifdef SPIRV_EFFCEE
+  // Runs a single pass of class |PassT| on the binary assembled from the
+  // |original| assembly, then runs an Effcee matcher over the disassembled
+  // result, using checks parsed from |original|.  Always skips OpNop.
+  // This does *not* involve pass manager.  Callers are suggested to use
+  // SCOPED_TRACE() for better messages.
+  template <typename PassT, typename... Args>
+  void SinglePassRunAndMatch(const std::string& original, Args&&... args) {
+    const bool skip_nop = true;
+    auto pass_result = SinglePassRunAndDisassemble<PassT>(
+        original, skip_nop, std::forward<Args>(args)...);
+    auto disassembly = std::get<0>(pass_result);
+    auto match_result = effcee::Match(disassembly, original);
+    EXPECT_EQ(effcee::Result::Status::Ok, match_result.status())
+        << match_result.message() << "\nChecking result:\n"
+        << disassembly;
+  }
+#endif
+
  // Adds a pass to be run.
  template <typename PassT, typename... Args>
  void AddPass(Args&&... args) {
--- a/test/opt/strength_reduction_test.cpp
+++ b/test/opt/strength_reduction_test.cpp
@ -63,36 +63,45 @@ TEST_F(StrengthReductionBasicTest, BasicReplaceMulBy8) {
  EXPECT_THAT(output, HasSubstr("OpShiftLeftLogical %uint %uint_5 %uint_3"));
 }

-// Test to make sure we replace 16*5.
+// TODO(dneto): Add Effcee as required dependency, and make this unconditional.
+#ifdef SPIRV_EFFCEE
+// Test to make sure we replace 16*5
+// Also demonstrate use of Effcee matching.
 TEST_F(StrengthReductionBasicTest, BasicReplaceMulBy16) {
-  const std::vector<const char*> text = {
-      // clang-format off
-               "OpCapability Shader",
-          "%1 = OpExtInstImport \"GLSL.std.450\"",
-               "OpMemoryModel Logical GLSL450",
-               "OpEntryPoint Vertex %main \"main\"",
-               "OpName %main \"main\"",
-       "%void = OpTypeVoid",
-          "%4 = OpTypeFunction %void",
-        "%int = OpTypeInt 32 1",
-      "%int_16 = OpConstant %int 16",
-      "%int_5 = OpConstant %int 5",
-       "%main = OpFunction %void None %4",
-          "%8 = OpLabel",
-          "%9 = OpIMul %int %int_16 %int_5",
-               "OpReturn",
-               "OpFunctionEnd"
-      // clang-format on
-  };
+  const std::string text = R"(
+               OpCapability Shader
+          %1 = OpExtInstImport "GLSL.std.450"
+               OpMemoryModel Logical GLSL450
+               OpEntryPoint Vertex %main "main"
+               OpName %main "main"
+       %void = OpTypeVoid
+          %4 = OpTypeFunction %void
+; We know disassembly will produce %uint here, but
+;  CHECK: %uint = OpTypeInt 32 0
+;  CHECK-DAG: [[five:%[a-zA-Z_\d]+]] = OpConstant %uint 5

-  auto result = SinglePassRunAndDisassemble<opt::StrengthReductionPass>(
-      JoinAllInsts(text), /* skip_nop = */ true);
+; We have RE2 regular expressions, so \w matches [_a-zA-Z0-9].
+; This shows the preferred pattern for matching SPIR-V identifiers.
+; (We could have cheated in this case since we know the disassembler will
+; generate the 'nice' name of "%uint_4".
+;  CHECK-DAG: [[four:%\w+]] = OpConstant %uint 4
+       %uint = OpTypeInt 32 0
+     %uint_5 = OpConstant %uint 5
+    %uint_16 = OpConstant %uint 16
+       %main = OpFunction %void None %4
+; CHECK: OpLabel
+          %8 = OpLabel
+; CHECK-NEXT: OpShiftLeftLogical %uint [[five]] [[four]]
+; The multiplication disappears.
+; CHECK-NOT: OpIMul
+          %9 = OpIMul %uint %uint_16 %uint_5
+               OpReturn
+; CHECK: OpFunctionEnd
+               OpFunctionEnd)";

-  EXPECT_EQ(opt::Pass::Status::SuccessWithChange, std::get<1>(result));
-  const std::string& output = std::get<0>(result);
-  EXPECT_THAT(output, Not(HasSubstr("OpIMul")));
-  EXPECT_THAT(output, HasSubstr("OpShiftLeftLogical %int %int_5 %uint_4"));
+  SinglePassRunAndMatch<opt::StrengthReductionPass>(text);
 }
+#endif

 // Test to make sure we replace a multiple of 32 and 4.
 TEST_F(StrengthReductionBasicTest, BasicTwoPowersOf2) {