Drop #pragma detect_mismatch for mutex size#4301
Merged
StephanTLavavej merged 1 commit intomicrosoft:mainfrom Jan 9, 2024
Merged
Drop #pragma detect_mismatch for mutex size#4301StephanTLavavej merged 1 commit intomicrosoft:mainfrom
#pragma detect_mismatch for mutex size#4301StephanTLavavej merged 1 commit intomicrosoft:mainfrom
Conversation
Followup to GH 4294.
CaseyCarter
approved these changes
Jan 8, 2024
Contributor
|
Roll a d20 for ODR violation. |
Member
Author
|
I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Followup to #4294.
In theory, this
#pragma detect_mismatchwas a good thing. In practice, it causes ~200 directories to fail with linker errors in the Windows build. That's too many to feasibly deal with on short notice, even if we added an escape hatch, so we should simply drop this attempt to detect mismatch. (The change was originally tested in Windows in December withoutdetect_mismatch.)The net effect of #4294 and this PR is to make
_Critical_section_sizeadditionally sensitive toUNDOCKED_WINDOWS_UCRT. This should be a strict improvement, reducing mismatches between the headers and separately compiled code (we know for a fact that this was causing problems). If these ~200 directories are mixing TUs with varying settings ofUNDOCKED_WINDOWS_UCRT, then depending on linker behavior that might prevent this fix from taking effect, but it shouldn't make things worse compared to the state before #4294.If we continue to have problems in this area, we may need to consider removing the Windows special case for
mutexentirely (according to my understanding, we could do that because Windows is always built fresh with its LKG toolset, so it doesn't actually care about ABI stability here). That would increasemutexsize, of course, but we've ripped out the other inefficiencies (virtual calls etc.), so the potential performance impact would be less than it previously would have been.