Skip to content
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

Disable msvc vectorization in GC get_promoted_bytes #100309

Merged
merged 2 commits into from
Mar 27, 2024
Merged

Conversation

markples
Copy link
Member

@markples markples commented Mar 26, 2024

This fixes the code generation rather than trying to address the symptoms of tests failing. Therefore, test Callback_Svr can be enabled again.

See https://developercommunity.visualstudio.com/t/Bad-codegen-in-coreclr-GC-code/10625094. Issue may have been fixed already as later toolsets don't seem to show the issue, but this hasn't been verified.

Copy link
Contributor

Tagging subscribers to this area: @dotnet/gc
See info in area-owners.md if you want to be subscribed.

@@ -24211,6 +24211,9 @@ size_t gc_heap::get_promoted_bytes()

dprintf (3, ("h%d getting surv", heap_number));
size_t promoted = 0;
#ifdef _MSC_VER
#pragma loop(no_vector)
Copy link
Member

Choose a reason for hiding this comment

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

Add a link to the MSVC bug that this is working around?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I think I will add it to the description and merge as soon as the tests finish so that this stops impacting everyone, and then I can add a comment when the timing doesn't matter (and hopefully we'll know more -- latest toolchains don't repro this anymore, but we'd like to know if it was directly fixed. Andy filed https://developercommunity.visualstudio.com/t/Bad-codegen-in-coreclr-GC-code/10625094)

Copy link
Member Author

@markples markples Apr 2, 2024

Choose a reason for hiding this comment

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

https://devdiv.visualstudio.com/DevDiv/_git/msvc/commit/e72c28af106a0efbc93eb67b7b21c433e8749424

I haven't checked the timing yet. We might be able to remove the workaround already.

Copy link
Member Author

Choose a reason for hiding this comment

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

We've stopped using preview compilers. The codegen bug was fixed. This workaround was removed in #101995.

@markples
Copy link
Member Author

image

@markples markples marked this pull request as ready for review March 27, 2024 06:54
@markples markples merged commit 765ca4e into dotnet:main Mar 27, 2024
98 checks passed
markples added a commit to markples/runtime that referenced this pull request Apr 10, 2024
…dotnet#100309)"

This partially reverts commit 765ca4e.

(reverts the workaround but keeps the test)
@github-actions github-actions bot locked and limited conversation to collaborators May 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants