Skip to content

[Driver][SYCL] Do not set workaround predefines with /MDd #18252

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

Merged
merged 2 commits into from
May 2, 2025

Conversation

mdtoguchi
Copy link
Contributor

We are currently setting 2 pre-defines which workaound a compilation issue: -D_CONTAINER_DEBUG_LEVEL=0 and -D_ITERATOR_DEBUG_LEVEL=0. We can no longer rely on these macros as _CONTAINER_DEBUG_LEVEL will be going away with a future release of Visual Studio.

The issues that this worked around are detailed in our release notes as a known issue:

- Compilation may fail on Windows in debug mode if a kernel uses

Remove these workarounds in preparation for the upcoming MSVS release.

We are currently setting 2 pre-defines which workaound a compilation
issue:  -D_CONTAINER_DEBUG_LEVEL=0 and -D_ITERATOR_DEBUG_LEVEL=0.  We
can no longer rely on these macros as _CONTAINER_DEBUG_LEVEL will be
going away with a future release of Visual Studio.

The issues that this worked around are detailed in our release notes as
a known issue: https://github.com/intel/llvm/blob/aa2c876f529f942943afaf9395694fe07a7f6121/sycl/ReleaseNotes.md?plain=1#L2700

Remove these workarounds in preparation for the upcoming MSVS release.
@mdtoguchi mdtoguchi requested a review from a team as a code owner April 29, 2025 23:49
@hchilama
Copy link
Contributor

Hi Mike, Is there a JIRA for this to get more details?
And you said "_CONTAINER_DEBUG_LEVEL will be going away with a future release of Visual Studio"
Removing this 2 macros right now from the driver wouldn't effect the current release?

@hchilama
Copy link
Contributor

hchilama commented Apr 30, 2025

I found the JIRA
I think it make sense to remove the _CONTAINER_DEBUG_LEVEL predefine, but it is with the MS VS 17.14.0 Preview 3 & 4 only not with the release right?

@mdtoguchi
Copy link
Contributor Author

mdtoguchi commented Apr 30, 2025

I found the JIRA I think it make sense to remove the _CONTAINER_DEBUG_LEVEL predefine, but it is with the MS VS 17.14.0 Preview 3 & 4 only not with the release right?

Right - the behavior is changing with the current preview release 3 and 4 (i.e when those previews become regular releases). Given the timing of the releases, they will be available during the 2025.2 release timeframe, basically breaking users who update their underlying MSVC version.

@srividya-sundaram
Copy link
Contributor

We are currently setting 2 pre-defines which workaound a compilation issue: -D_CONTAINER_DEBUG_LEVEL=0 and -D_ITERATOR_DEBUG_LEVEL=0. We can no longer rely on these macros as _CONTAINER_DEBUG_LEVEL will be going away with a future release of Visual Studio.

The issues that this worked around are detailed in our release notes as a known issue:

- Compilation may fail on Windows in debug mode if a kernel uses

Remove these workarounds in preparation for the upcoming MSVS release.

Should these references be updated as well?
L2709
L2986
L3360
L3777

@mdtoguchi
Copy link
Contributor Author

We are currently setting 2 pre-defines which workaound a compilation issue: -D_CONTAINER_DEBUG_LEVEL=0 and -D_ITERATOR_DEBUG_LEVEL=0. We can no longer rely on these macros as _CONTAINER_DEBUG_LEVEL will be going away with a future release of Visual Studio.
The issues that this worked around are detailed in our release notes as a known issue:

- Compilation may fail on Windows in debug mode if a kernel uses

Remove these workarounds in preparation for the upcoming MSVS release.

Should these references be updated as well? L2709 L2986 L3360 L3777

The release notes look to be a running update. With the next iteration, the workaround should be updated to not use the _CONTAINER_DEBUG_LEVEL value when dealing with the upcoming MSVC releases, but I don't know if there is a valid workaround from that point on. @AlexeySachkov, any thoughts of how we can update the release notes in regards to this?

@mdtoguchi
Copy link
Contributor Author

@srividya-sundaram, any other comments here? We can address the doc updates separately, this is on the MFL and needs to get some movement. Thanks!

@srividya-sundaram
Copy link
Contributor

@srividya-sundaram, any other comments here? We can address the doc updates separately, this is on the MFL and needs to get some movement. Thanks!

Ok, I thought you were waiting for @AlexeySachkov's feedback.

We can address the doc updates separately

Can you create a github issue or JIRA to track the doc update?

@mdtoguchi
Copy link
Contributor Author

Can you create a github issue or JIRA to track the doc update?

Will do - thanks for the review!

@mdtoguchi
Copy link
Contributor Author

@intel/llvm-gatekeepers, this is ready for merge, thanks!

@martygrant martygrant merged commit 53a718f into intel:sycl May 2, 2025
41 of 43 checks passed
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.

4 participants