Update cloning of the DirectXShaderCompiler repo to not include DXC tests #122178
Conversation
…. This prevents any unneccessary dependency on TAEF when building as it's not used for dxil-dis testing
|
@llvm/pr-subscribers-backend-directx Author: Brian Favela (bfavela) ChangesThis prevents any unnecessary dependency on TAEF when building as it's not used for dxil-dis testing Full diff: https://github.com/llvm/llvm-project/pull/122178.diff 1 Files Affected:
diff --git a/llvm/tools/dxil-dis/CMakeLists.txt b/llvm/tools/dxil-dis/CMakeLists.txt
index d0541fcf802e98..ad540bbd53df62 100644
--- a/llvm/tools/dxil-dis/CMakeLists.txt
+++ b/llvm/tools/dxil-dis/CMakeLists.txt
@@ -38,7 +38,7 @@ ExternalProject_Add(DXC
${GIT_SETTINGS}
SOURCE_DIR ${SOURCE_DIR}
BINARY_DIR ${BINARY_DIR}
- CMAKE_ARGS -C ${SOURCE_DIR}/cmake/caches/PredefinedParams.cmake -DLLVM_INCLUDE_TESTS=On
+ CMAKE_ARGS -C ${SOURCE_DIR}/cmake/caches/PredefinedParams.cmake -DLLVM_INCLUDE_TESTS=On -DLLVM_INCLUDE_TESTS=Off -DCLANG_INCLUDE_TESTS=Off -DHLSL_INCLUDE_TESTS=Off
BUILD_COMMAND ${CMAKE_COMMAND} --build ${BINARY_DIR} --target llvm-dis
BUILD_BYPRODUCTS ${BINARY_DIR}/bin/llvm-dis
INSTALL_COMMAND ""
|
llvm/tools/dxil-dis/CMakeLists.txt
Outdated
| SOURCE_DIR ${SOURCE_DIR} | ||
| BINARY_DIR ${BINARY_DIR} | ||
| CMAKE_ARGS -C ${SOURCE_DIR}/cmake/caches/PredefinedParams.cmake -DLLVM_INCLUDE_TESTS=On | ||
| CMAKE_ARGS -C ${SOURCE_DIR}/cmake/caches/PredefinedParams.cmake -DLLVM_INCLUDE_TESTS=On -DLLVM_INCLUDE_TESTS=Off -DCLANG_INCLUDE_TESTS=Off -DHLSL_INCLUDE_TESTS=Off |
There was a problem hiding this comment.
| CMAKE_ARGS -C ${SOURCE_DIR}/cmake/caches/PredefinedParams.cmake -DLLVM_INCLUDE_TESTS=On -DLLVM_INCLUDE_TESTS=Off -DCLANG_INCLUDE_TESTS=Off -DHLSL_INCLUDE_TESTS=Off | |
| CMAKE_ARGS -C ${SOURCE_DIR}/cmake/caches/PredefinedParams.cmake -DLLVM_INCLUDE_TESTS=Off -DCLANG_INCLUDE_TESTS=Off -DHLSL_INCLUDE_TESTS=Off |
There was a problem hiding this comment.
Presumably we don't need two -DLLVM_INCLUDE_TESTS params?
There was a problem hiding this comment.
Ugh. I should have caught that. Thanks!
bogner
left a comment
bogner
left a comment
There was a problem hiding this comment.
This seems reasonable enough.
It does look like @llvm-beanz explicitly included the -DLLVM_INCLUDE_TESTS flag when he first implemented this, so it would be best to get an explicit sign off from him.
The flag should be "off". The intention here is to remove the test infrastructure completely from being built so as to not rely on external programs existing like TAEF. |
|
This isn't working right. I need to do some more testing. Please don't merge yet. |
Yea, I'm not sure why I wrote it that way... I did give @bfavela the initial patch for this. Once Brian is comfortable I'm happy to merge this. |
The issue I mentioned was just with building using msbuild (yes, yes). I think this can get safely merged. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/11844 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/146/builds/2094 Here is the relevant piece of the build log for the reference |
6 participants
This prevents any unnecessary dependency on TAEF when building as it's not used for dxil-dis testing