Skip to content

[UR] Force rel CRT for Win static UR libs - #23508

Open
bratpiorka wants to merge 1 commit into
intel:syclfrom
bratpiorka:rrudnick_force_mt_on_static_adapters
Open

bratpiorka wants to merge 1 commit into
intel:syclfrom
bratpiorka:rrudnick_force_mt_on_static_adapters

Conversation

@bratpiorka

Copy link
Copy Markdown
Contributor

Force the Release CRT version (/MD) for statically built UR adapters and libraries on Windows.
Additionally, select the appropriate version of XPTI (xpti/xptid) that matches the Debug/Release version of the UR libraries.

@bratpiorka
bratpiorka requested review from a team as code owners October 15, 2026 18:23
@bratpiorka
bratpiorka requested a review from againull October 15, 2026 18:23
set(UMF_LINK_HWLOC_STATICALLY ON CACHE INTERNAL "static HWLOC")
endif()

# Choose the valid XPTI version (xpti or xptid)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Comment seems slightly misleading as here we just override with xpti.

Suggested change
# Choose the valid XPTI version (xpti or xptid)
# The in-tree static UR libraries use /MD even in Debug configurations,
# so force the release CRT XPTI library.

endif()

# Choose the valid XPTI version (xpti or xptid)
if(WIN32 AND UR_STATIC_LOADER)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

UR_STATIC_LOADER defaults to ON inside the following add_subdirectory(), so this check skips the override on a fresh configure unless the option is explicitly supplied. A Windows Debug build then selects xptid despite forcing UR to /MD. Could we initialize the option before this check?

@ldorau ldorau Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1
Nothing in sycl/CMakeLists.txt or buildbot/configure.py sets it earlier. On the first configure, line 95 is false but line 104 (after add_subdirectory) is true. The first-time Windows Debug configure forces UR to /MD but links the debug xptid, which is a CRT mismatch. The second configure fixes it, because by then the option is in the cache.

list(TRANSFORM SYCL_ENABLE_BACKENDS PREPEND "ur_adapter_"
OUTPUT_VARIABLE _ur_crt_targets)
# Add the remaining targets that don't match the "ur_adapter_*" pattern.
list(APPEND _ur_crt_targets

@againull againull Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I wonder if UMF and hwloc should be included here too?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The fix is harder than adding names. UMF doesn't use the property: cmake/helpers.cmake:289 adds /MD$<$<CONFIG:Debug>:d> as an explicit compile option. Setting MSVC_RUNTIME_LIBRARY on UMF targets would leave /MDd in effect, because the later option wins. UMF and hwloc are linked statically into ur_common and the adapters on Windows (UMF_BUILD_SHARED_LIBRARY OFF), so a Debug build likely still mixes CRTs. This mismatch existed before the PR, but the PR is meant to fix exactly this.

Could you also describe how the Windows Debug build was tested, since CI only covers Release?

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants