Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPIR-V] Add SPIRV-Tools for testing #73044

Merged
merged 4 commits into from
Dec 4, 2023

Conversation

sudonatalie
Copy link
Member

@sudonatalie sudonatalie commented Nov 21, 2023

Add spirv-dis (disassembler) and spirv-val (validator) from SPIRV-Tools as external dependencies for testing the SPIR-V backend. These tools are test dependencies only.

SPIR-V backend tests now have a dependency on the spirv-dis and spirv-val targets when the LLVM_INCLUDE_SPIRV_TOOLS_TESTS cmake variable is set, which allows additional test files with the REQUIRES: spirv-tools constraint to run, along with additional RUN: %if spirv-tools ... lines in existing tests. All other SPIR-V backend tests will run normally when LLVM_INCLUDE_SPIRV_TOOLS_TESTS is not set.

Several tests are included to show these tools' use, however more tests will be migrated and added later.

  • OpVariable_order.ll shows how spirv-val can catch bugs in the backend.
  • basic_int_types_spirvdis.ll shows how tests can be much shorter and more readable by FileChecking the spirv-dis output.
  • basic_int_types.ll shows how an additional RUN line can add validation to existing tests.

RFC: https://discourse.llvm.org/t/rfc-add-a-test-dependency-on-spirv-tools/75135

@sudonatalie sudonatalie marked this pull request as ready for review November 29, 2023 20:50
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 29, 2023

@llvm/pr-subscribers-backend-spir-v

Author: Natalie Chouinard (sudonatalie)

Changes

Add spirv-dis (disassembler) and spirv-val (validator) from SPIRV-Tools as external dependencies for testing the SPIR-V backend. These tools are test dependencies only.

SPIR-V tests now have a dependency on the spirv-dis and spirv-val targets, and are only built when the LLVM_INCLUDE_SPIRV_TOOLS_TESTS cmake variable is set.

Two tests are included to show these tools' use, however more tests will be migrated and added later.

  • OpVariable_order.ll shows how spirv-val can catch bugs in the backend.
  • basic_int_types.ll shows how tests can be much shorter and more readable by FileChecking the spirv-dis output.

Full diff: https://github.com/llvm/llvm-project/pull/73044.diff

6 Files Affected:

  • (modified) llvm/test/CMakeLists.txt (+5)
  • (added) llvm/test/CodeGen/SPIRV/OpVariable_order.ll (+17)
  • (modified) llvm/test/CodeGen/SPIRV/basic_int_types.ll (+16-44)
  • (modified) llvm/test/CodeGen/SPIRV/lit.local.cfg (+5)
  • (modified) llvm/test/lit.site.cfg.py.in (+1)
  • (added) llvm/tools/spirv-tools/CMakeLists.txt (+66)
diff --git a/llvm/test/CMakeLists.txt b/llvm/test/CMakeLists.txt
index 8aa652240081ce5..2ae07ec2df752ba 100644
--- a/llvm/test/CMakeLists.txt
+++ b/llvm/test/CMakeLists.txt
@@ -222,6 +222,11 @@ if(LLVM_ENABLE_HTTPLIB)
   list(APPEND LLVM_TEST_DEPENDS llvm-debuginfod)
 endif()
 
+if (LLVM_INCLUDE_SPIRV_TOOLS_TESTS)
+  list(APPEND LLVM_TEST_DEPENDS spirv-dis)
+  list(APPEND LLVM_TEST_DEPENDS spirv-val)
+endif()
+
 add_custom_target(llvm-test-depends DEPENDS ${LLVM_TEST_DEPENDS})
 set_target_properties(llvm-test-depends PROPERTIES FOLDER "Tests")
 
diff --git a/llvm/test/CodeGen/SPIRV/OpVariable_order.ll b/llvm/test/CodeGen/SPIRV/OpVariable_order.ll
new file mode 100644
index 000000000000000..a4ca3aa709f0fab
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/OpVariable_order.ll
@@ -0,0 +1,17 @@
+; REQUIRES: spirv-tools
+; RUN: llc -O0 -mtriple=spirv-unknown-linux %s -o - -filetype=obj | not spirv-val 2>&1 | FileCheck %s
+
+; TODO(#66261): The SPIR-V backend should reorder OpVariable instructions so this doesn't fail,
+;     but in the meantime it's a good example of the spirv-val tool working as intended.
+
+; CHECK: All OpVariable instructions in a function must be the first instructions in the first block.
+
+define void @main() #1 {
+entry:
+  %0 = alloca <2 x i32>, align 4
+  %1 = getelementptr <2 x i32>, ptr %0, i32 0, i32 0
+  %2 = alloca float, align 4
+  ret void
+}
+
+attributes #1 = { "hlsl.numthreads"="4,8,16" "hlsl.shader"="compute" }
diff --git a/llvm/test/CodeGen/SPIRV/basic_int_types.ll b/llvm/test/CodeGen/SPIRV/basic_int_types.ll
index b479cee0bed71cc..3778d897929188b 100644
--- a/llvm/test/CodeGen/SPIRV/basic_int_types.ll
+++ b/llvm/test/CodeGen/SPIRV/basic_int_types.ll
@@ -1,72 +1,44 @@
-; RUN: llc -O0 -mtriple=spirv-unknown-unknown %s -o - | FileCheck %s
-; RUN: llc -O0 -mtriple=spirv32-unknown-unknown %s -o - | FileCheck %s
-; RUN: llc -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s
+; REQUIRES: spirv-tools
+; RUN: llc -O0 -mtriple=spirv-unknown-unknown %s -o - --filetype=obj | spirv-dis | FileCheck %s
+; RUN: llc -O0 -mtriple=spirv32-unknown-unknown %s -o - --filetype=obj | spirv-dis | FileCheck %s
+; RUN: llc -O0 -mtriple=spirv64-unknown-unknown %s -o - --filetype=obj | spirv-dis | FileCheck %s
 
 define void @main() {
 entry:
-; CHECK-DAG:   %[[#short:]] = OpTypeInt 16 0
-; CHECK-DAG:     %[[#int:]] = OpTypeInt 32 0
-; CHECK-DAG:    %[[#long:]] = OpTypeInt 64 0
-
-; CHECK-DAG: %[[#v2short:]] = OpTypeVector %[[#short]] 2
-; CHECK-DAG: %[[#v3short:]] = OpTypeVector %[[#short]] 3
-; CHECK-DAG: %[[#v4short:]] = OpTypeVector %[[#short]] 4
-
-; CHECK-DAG:   %[[#v2int:]] = OpTypeVector %[[#int]] 2
-; CHECK-DAG:   %[[#v3int:]] = OpTypeVector %[[#int]] 3
-; CHECK-DAG:   %[[#v4int:]] = OpTypeVector %[[#int]] 4
-
-; CHECK-DAG:  %[[#v2long:]] = OpTypeVector %[[#long]] 2
-; CHECK-DAG:  %[[#v3long:]] = OpTypeVector %[[#long]] 3
-; CHECK-DAG:  %[[#v4long:]] = OpTypeVector %[[#long]] 4
-
-; CHECK-DAG:   %[[#ptr_Function_short:]] = OpTypePointer Function %[[#short]]
-; CHECK-DAG:     %[[#ptr_Function_int:]] = OpTypePointer Function %[[#int]]
-; CHECK-DAG:    %[[#ptr_Function_long:]] = OpTypePointer Function %[[#long]]
-; CHECK-DAG: %[[#ptr_Function_v2short:]] = OpTypePointer Function %[[#v2short]]
-; CHECK-DAG: %[[#ptr_Function_v3short:]] = OpTypePointer Function %[[#v3short]]
-; CHECK-DAG: %[[#ptr_Function_v4short:]] = OpTypePointer Function %[[#v4short]]
-; CHECK-DAG:   %[[#ptr_Function_v2int:]] = OpTypePointer Function %[[#v2int]]
-; CHECK-DAG:   %[[#ptr_Function_v3int:]] = OpTypePointer Function %[[#v3int]]
-; CHECK-DAG:   %[[#ptr_Function_v4int:]] = OpTypePointer Function %[[#v4int]]
-; CHECK-DAG:  %[[#ptr_Function_v2long:]] = OpTypePointer Function %[[#v2long]]
-; CHECK-DAG:  %[[#ptr_Function_v3long:]] = OpTypePointer Function %[[#v3long]]
-; CHECK-DAG:  %[[#ptr_Function_v4long:]] = OpTypePointer Function %[[#v4long]]
-
-; CHECK: %[[#]] = OpVariable %[[#ptr_Function_short]] Function
+; CHECK: %int16_t_Val = OpVariable %_ptr_Function_ushort Function
   %int16_t_Val = alloca i16, align 2
 
-; CHECK: %[[#]] = OpVariable %[[#ptr_Function_int]] Function
+; CHECK: %int_Val = OpVariable %_ptr_Function_uint Function
   %int_Val = alloca i32, align 4
 
-; CHECK: %[[#]] = OpVariable %[[#ptr_Function_long]] Function
+; CHECK: %int64_t_Val = OpVariable %_ptr_Function_ulong Function
   %int64_t_Val = alloca i64, align 8
 
-; CHECK: %[[#]] = OpVariable %[[#ptr_Function_v2short]] Function
+; CHECK: %int16_t2_Val = OpVariable %_ptr_Function_v2ushort Function
   %int16_t2_Val = alloca <2 x i16>, align 4
 
-; CHECK: %[[#]] = OpVariable %[[#ptr_Function_v3short]] Function
+; CHECK: %int16_t3_Val = OpVariable %_ptr_Function_v3ushort Function
   %int16_t3_Val = alloca <3 x i16>, align 8
 
-; CHECK: %[[#]] = OpVariable %[[#ptr_Function_v4short]] Function
+; CHECK: %int16_t4_Val = OpVariable %_ptr_Function_v4ushort Function
   %int16_t4_Val = alloca <4 x i16>, align 8
 
-; CHECK: %[[#]] = OpVariable %[[#ptr_Function_v2int]] Function
+; CHECK: %int2_Val = OpVariable %_ptr_Function_v2uint Function
   %int2_Val = alloca <2 x i32>, align 8
 
-; CHECK: %[[#]] = OpVariable %[[#ptr_Function_v3int]] Function
+; CHECK: %int3_Val = OpVariable %_ptr_Function_v3uint Function
   %int3_Val = alloca <3 x i32>, align 16
 
-; CHECK: %[[#]] = OpVariable %[[#ptr_Function_v4int]] Function
+; CHECK: %int4_Val = OpVariable %_ptr_Function_v4uint Function
   %int4_Val = alloca <4 x i32>, align 16
 
-; CHECK: %[[#]] = OpVariable %[[#ptr_Function_v2long]] Function
+; CHECK: %int64_t2_Val = OpVariable %_ptr_Function_v2ulong Function
   %int64_t2_Val = alloca <2 x i64>, align 16
 
-; CHECK: %[[#]] = OpVariable %[[#ptr_Function_v3long]] Function
+; CHECK: %int64_t3_Val = OpVariable %_ptr_Function_v3ulong Function
   %int64_t3_Val = alloca <3 x i64>, align 32
 
-; CHECK: %[[#]] = OpVariable %[[#ptr_Function_v4long]] Function
+; CHECK: %int64_t4_Val = OpVariable %_ptr_Function_v4ulong Function
   %int64_t4_Val = alloca <4 x i64>, align 32
 
   ret void
diff --git a/llvm/test/CodeGen/SPIRV/lit.local.cfg b/llvm/test/CodeGen/SPIRV/lit.local.cfg
index 78dd74cd6dc6349..00f50fb6f1cff7c 100644
--- a/llvm/test/CodeGen/SPIRV/lit.local.cfg
+++ b/llvm/test/CodeGen/SPIRV/lit.local.cfg
@@ -1,2 +1,7 @@
 if not "SPIRV" in config.root.targets:
     config.unsupported = True
+
+if config.spirv_tools_tests:
+    config.available_features.add("spirv-tools")
+    config.substitutions.append(("spirv-dis", os.path.join(config.llvm_tools_dir, "spirv-dis")))
+    config.substitutions.append(("spirv-val", os.path.join(config.llvm_tools_dir, "spirv-val")))
diff --git a/llvm/test/lit.site.cfg.py.in b/llvm/test/lit.site.cfg.py.in
index 6f3ab6561c9ec9c..1138b2ccf7bce7e 100644
--- a/llvm/test/lit.site.cfg.py.in
+++ b/llvm/test/lit.site.cfg.py.in
@@ -60,6 +60,7 @@ config.expensive_checks = @LLVM_ENABLE_EXPENSIVE_CHECKS@
 config.reverse_iteration = @LLVM_ENABLE_REVERSE_ITERATION@
 config.dxil_tests = @LLVM_INCLUDE_DXIL_TESTS@
 config.have_llvm_driver = @LLVM_TOOL_LLVM_DRIVER_BUILD@
+config.spirv_tools_tests = @LLVM_INCLUDE_SPIRV_TOOLS_TESTS@
 
 import lit.llvm
 lit.llvm.initialize(lit_config, config)
diff --git a/llvm/tools/spirv-tools/CMakeLists.txt b/llvm/tools/spirv-tools/CMakeLists.txt
new file mode 100644
index 000000000000000..f139e2a46e57c3e
--- /dev/null
+++ b/llvm/tools/spirv-tools/CMakeLists.txt
@@ -0,0 +1,66 @@
+option(LLVM_INCLUDE_SPIRV_TOOLS_TESTS "Include tests that use SPIRV-Tools" Off)
+mark_as_advanced(LLVM_INCLUDE_SPIRV_TOOLS_TESTS)
+
+if (NOT LLVM_INCLUDE_SPIRV_TOOLS_TESTS)
+  return()
+endif ()
+
+if (NOT "SPIRV" IN_LIST LLVM_TARGETS_TO_BUILD)
+  message(FATAL_ERROR "Building SPIRV-Tools tests is unsupported without the SPIR-V target")
+endif ()
+
+# SPIRV_DIS and SPIRV_VAL variables can be used to provide paths to existing
+# spirv-dis and spirv-val binaries, respectively. Otherwise, build them from
+# SPIRV-Tools source.
+if (NOT SPIRV_DIS OR NOT SPIRV_VAL)
+  include(ExternalProject)
+
+  set(BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/SPIRVTools-bin)
+
+  ExternalProject_Add(SPIRVTools
+    GIT_REPOSITORY https://github.com/KhronosGroup/SPIRV-Tools.git
+    GIT_TAG main
+    BINARY_DIR ${BINARY_DIR}
+    BUILD_COMMAND ${CMAKE_COMMAND} --build ${BINARY_DIR} --target spirv-dis spirv-val
+    BUILD_BYPRODUCTS ${BINARY_DIR}/tools/spirv-dis ${BINARY_DIR}/tools/spirv-val
+    # Don't auto-update on every build.
+    UPDATE_DISCONNECTED 1
+    # Allow manual updating with an explicit SPIRVTools-update target.
+    STEP_TARGETS update
+    INSTALL_COMMAND ""
+    )
+
+  ExternalProject_Add_Step(SPIRVTools get-deps
+    COMMENT "Getting SPIRV-Tools dependencies"
+    COMMAND ${Python3_EXECUTABLE} utils/git-sync-deps
+    DEPENDEES update
+    WORKING_DIRECTORY <SOURCE_DIR>
+    )
+endif ()
+
+if (CMAKE_HOST_UNIX)
+  set(LLVM_LINK_OR_COPY create_symlink)
+else ()
+  set(LLVM_LINK_OR_COPY copy)
+endif ()
+
+# Link the provided or just built spirv-dis and spirv-val binaries.
+if (SPIRV_DIS)
+  add_custom_target(spirv-dis
+    COMMAND ${CMAKE_COMMAND} -E ${LLVM_LINK_OR_COPY} "${SPIRV_DIS}" "${LLVM_RUNTIME_OUTPUT_INTDIR}/spirv-dis")
+else ()
+  add_custom_target(spirv-dis
+    COMMAND ${CMAKE_COMMAND} -E ${LLVM_LINK_OR_COPY} "${BINARY_DIR}/tools/spirv-dis" "${LLVM_RUNTIME_OUTPUT_INTDIR}/spirv-dis"
+    DEPENDS SPIRVTools
+    )
+endif ()
+
+if (SPIRV_VAL)
+  add_custom_target(spirv-val
+    COMMAND ${CMAKE_COMMAND} -E ${LLVM_LINK_OR_COPY} "${SPIRV_VAL}" "${LLVM_RUNTIME_OUTPUT_INTDIR}/spirv-val")
+else ()
+  add_custom_target(spirv-val
+    COMMAND ${CMAKE_COMMAND} -E ${LLVM_LINK_OR_COPY} "${BINARY_DIR}/tools/spirv-val" "${LLVM_RUNTIME_OUTPUT_INTDIR}/spirv-val"
+    DEPENDS SPIRVTools
+    )
+endif ()

@sudonatalie
Copy link
Member Author

@Keenuts @michalpaszkowski I'll continue to monitor the RFC for any more incoming feedback but for now I think it's reasonable to go ahead with review on this, so please take a look whenever you can.

Copy link
Contributor

@Keenuts Keenuts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for putting this together. I'd like to have this PR in 😊

llvm/tools/spirv-tools/CMakeLists.txt Outdated Show resolved Hide resolved
Add spirv-dis (disassembler) and spirv-val (validator) from SPIRV-Tools
as external dependencies for testing the SPIR-V backend. These tools are
test dependencies only.

SPIR-V tests now have a dependency on the spirv-dis and spirv-val
targets, and are only built when the LLVM_INCLUDE_SPIRV_TOOLS_TESTS cmake
variable is set.

Two tests are included to show these tools' use, however more tests will
be migrated and added later.
* OpVariable_order.ll shows how spirv-val can catch bugs in the backend.
* basic_int_types.ll shows how tests can be much shorter and more
  readable by FileChecking the spirv-dis output.
@sudonatalie sudonatalie force-pushed the add-spirv-tools branch 3 times, most recently from f8af8c5 to cb04f7d Compare December 1, 2023 17:20
@sudonatalie
Copy link
Member Author

I see the bot failure, but I'm not yet sure why @LLVM_INCLUDE_SPIRV_TOOLS_TESTS@ is being substituted with OFF while other cmake vars in the file are being substituted with 0. Looking into it.

@sudonatalie sudonatalie merged commit f368e64 into llvm:main Dec 4, 2023
3 checks passed
@sudonatalie sudonatalie deleted the add-spirv-tools branch December 4, 2023 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants