Fix CMake gen when HLSL_BUILD_DXILCONV=0 #6911
Conversation
Do not add dep on DxcEtw and DxcRuntimeEtw if HLSL_BUILD_DXILCONV is not enabled, otherwise CMake gen fails because these targets are not defined.
| @@ -58,7 +58,7 @@ target_link_libraries(dxildll PRIVATE | |||
| dxcvalidator | |||
| ) | |||
|
|
|||
| if (WIN32) | |||
| if (WIN32 AND HLSL_BUILD_DXILCONV) | |||
There was a problem hiding this comment.
dxildll does use DxcRuntimeEtw and DxcEtw.
Maybe we should always enable DxcRuntimeEtw and DxcEtw instead of guard it with HLSL_BUILD_DXILCONV on WIN32.
There was a problem hiding this comment.
We currently disable HLSL_BUILD_DXILCONV in our build as we don't need it, and it speeds up our builds at Google. I would rather we keep this guard.
There was a problem hiding this comment.
Only DxcRuntimeEtw is guard by HLSL_BUILD_DXILCONV currently because dxcetw is used by dxcompiler.
But dxildll does use DxcRuntimeEtw, without it dxildll will get error for cannot find DxcRuntimeEtw.h.
Maybe we should add HLSL_BUILD_DXILDLL, and guard DxcRuntimeEtw with (HLSL_BUILD_DXILCONV or HLSL_BUILD_DXILDLL)?
There was a problem hiding this comment.
Oops, I pushed too early. Still fixing this.
There was a problem hiding this comment.
Okay, I pushed what I think you asked for. However, I'm not sure if adding HLSL_BUILD_DXILDLL is what we want. This is perhaps a maintenance burden that isn't warranted for DXC.
There was a problem hiding this comment.
If we always build dxildll, then there's no need to guard DxcRuntimeEtw since it will be required by dxildll.
That will be another way to fix this issue.
Let's see how others feel about it.
There was a problem hiding this comment.
Yes, exactly what I was thinking! I just pushed #6912 which does exactly that.
| @@ -58,6 +58,7 @@ target_link_libraries(dxildll PRIVATE | |||
| dxcvalidator | |||
| ) | |||
|
|
|||
| if (HLSL_BUILD_DXILDLL) | |||
There was a problem hiding this comment.
We could use HLSL_BUILD_DXILDLL guard add_subdirectory(dxildll) in tools/clang/tools/CMakeLists.txt.
| list(APPEND CLANG_TEST_DEPS | ||
| dxildll | ||
| ) | ||
| endif() | ||
| else() |
There was a problem hiding this comment.
| else() | |
| endif() |
|
Closing this one in favor of #6912 |
Do not add dep on DxcEtw and DxcRuntimeEtw if HLSL_BUILD_DXILCONV is not enabled, otherwise CMake gen fails because these targets are not defined.