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

diff --git a/.appveyor.yml b/.appveyor.yml
index 309fd12..50fe13f 100644 (file)
--- 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

diff --git a/.travis.yml b/.travis.yml
index ed1f4e8..ba3d252 100644 (file)
--- 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.

diff --git a/CHANGES b/CHANGES
index dd2c5be..690a440 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -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

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 4ce9615..48eb5a0 100644 (file)
--- 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)
 

diff --git a/README.md b/README.md
index 2e61851..1daefb5 100644 (file)
--- 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

diff --git a/external/CMakeLists.txt b/external/CMakeLists.txt
index e710ddd..412d8c3 100644 (file)
--- 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()

diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index 1b8cad2..176aa91 100644 (file)
--- 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")

diff --git a/test/opt/pass_fixture.h b/test/opt/pass_fixture.h
index 93ef916..7ad7181 100644 (file)
--- 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) {

diff --git a/test/opt/strength_reduction_test.cpp b/test/opt/strength_reduction_test.cpp
index 541be60..32eb4c3 100644 (file)
--- 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
-  };
-
-  auto result = SinglePassRunAndDisassemble<opt::StrengthReductionPass>(
-      JoinAllInsts(text), /* skip_nop = */ true);
-
-  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"));
+  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
+
+; 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)";
+
+  SinglePassRunAndMatch<opt::StrengthReductionPass>(text);
 }
+#endif
 
 // Test to make sure we replace a multiple of 32 and 4.
 TEST_F(StrengthReductionBasicTest, BasicTwoPowersOf2) {