[xpupti] Fix Windows build support for xpupti tests and multi-config generators.#1394
[xpupti] Fix Windows build support for xpupti tests and multi-config generators.#1394tsocha wants to merge 14 commits into
Conversation
* Refactor test/xpupti/CMakeLists.txt to handle Windows and
multi-config generators (VS, Ninja Multi-Config):
- Force Ninja as the inner ExternalProject generator on Windows
and propagate $<CONFIG> via CONFIGURE_COMMAND for multi-config
- Use WINDOWS_EXPORT_ALL_SYMBOLS and /FORCE:UNRESOLVED so the
shared test library can build against ComputeOnXpu defined in
the compute executable
- Pass LINK_LIBRARY_NAME/LINK_LIBRARY_DIR instead of a hardcoded
lib<name>.so path; inner project resolves via find_library with
config-aware PATH_SUFFIXES
- Forward SYCL lib path to the inner linker on Windows
- Skip -Wl,--export-dynamic on Windows in compute/CMakeLists.txt
* Add XpuptiActivityHandlersTest executable (unit test that does
not require the SYCL compute ExternalProject); rename make_test
to make_sycl_test to reflect the remaining SYCL-based tests.
* Fix XPUPTI_BUILD_FLAG so -DHAS_XPUPTI is visible in both the
current and parent scopes (was only set in PARENT_SCOPE).
* Reorder id/linked/metadata assignments in
XpuptiActivityProfilerSession::handleRuntimeKernelMemcpyMemsetActivities
and inline the linkedActivity lookup — no behavior change.
* Include src/ActivityBuffers.h in XpuptiTestUtilities.cpp and
hoist iterator declarations out of the for-init to satisfy MSVC.
|
@gujinghui please review it. |
xpupti files including into compilation is handled in cmakefiles
|
DONE |
|
@divyanshk @scotts This PR is ready for your review. Thanks. |
|
@ryanzhang22 @scotts |
|
@ryanzhang22 has imported this pull request. If you are a Meta employee, you can view this in D111713633. |
| POSITION_INDEPENDENT_CODE ON | ||
| WINDOWS_EXPORT_ALL_SYMBOLS ON | ||
| ) | ||
| # On Windows, DLLs must resolve all symbols at link time (unlike Linux .so). |
There was a problem hiding this comment.
for my understanding: I see that the build is forced to progress despite the error with this flag enabled, but how does this end up producing a valid binary?
There was a problem hiding this comment.
It is caused by circular dependency.
The library is compiled with a missing symbol and during program loading this symbol is provided by the test .exe file, as it exports all of its symbols.
|
The following ciflow label(s) have been added but CI has not been triggered yet because the workflows are awaiting approval:
Once a maintainer approves the workflows (scroll to the bottom of the PR page), the corresponding CI jobs will be triggered automatically. Please ping one of the reviewers if you do not have access to approve and run workflows. |
|
@ryanzhang22 I have included a change from @aostrowski-hbn Circular dependency is resolved and CMake is cleaner now. |
It's a part of #1335 1/3
Refactor test/xpupti/CMakeLists.txt to handle Windows and multi-config generators (VS, Ninja Multi-Config):
Rename make_test to make_sycl_test to reflect SYCL-based tests.
Fix XPUPTI_BUILD_FLAG so -DHAS_XPUPTI is visible in both the current and parent scopes (was only set in PARENT_SCOPE).
Reorder id/linked/metadata assignments in XpuptiActivityProfilerSession::handleRuntimeKernelMemcpyMemsetActivities and inline the linkedActivity lookup — no behavior change.
Include src/ActivityBuffers.h in XpuptiTestUtilities.cpp and hoist iterator declarations out of the for-init to satisfy MSVC.