Skip to content

[release/9.0-staging] Use minipal_getcpufeatures to detect for AVX (#113032) #113489

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

Conversation

cshung
Copy link
Contributor

@cshung cshung commented Mar 13, 2025

Backport of # to release/9.0-staging

/cc @cshung

Customer Impact

@Typhon226 reported on #112897 that, on some machines, buggy behavior in __builtin_cpu_supports for Unix platforms could lie to us that it supports AVX512 when it doesn't. This could lead to runtime crashes.

Regression

  • Yes
  • No

The regression was introduced in PR #98712, commit 68cf2dd when I enabled VxSort for unix platforms, but this is not the root cause.

Testing

Local testing is done on various machines, but none of them hit the __builtin_cpu_supports buggy behavior, so it worked both before and after the changes.
#113032 (comment)

Customer testing is done on the machines that hit the issue, and the fixed version works as expected, for both #112897 (comment) (9.0 binaries) and #112897 (comment) (10.0 binaries)

Risk

The risk is low, the same function is used by the JIT to determine if the CPU supports AVX512.

@Copilot Copilot AI review requested due to automatic review settings March 13, 2025 19:23
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

Copy link
Contributor

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

@mangod9
Copy link
Member

mangod9 commented Mar 14, 2025

the build is still failing to find an include file.

@cshung cshung force-pushed the backport/pr-113032-to-release/9.0-staging branch from 0399a20 to ef9bf29 Compare March 14, 2025 21:07
Copy link
Member

@jeffschwMSFT jeffschwMSFT left a comment

Choose a reason for hiding this comment

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

lgtm. we will take for consideration in 9.0.x

@jeffschwMSFT jeffschwMSFT added the Servicing-consider Issue for next servicing release review label Mar 14, 2025
@jeffschwMSFT jeffschwMSFT added this to the 9.0.x milestone Mar 14, 2025
@leecow leecow added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Mar 18, 2025
@leecow leecow modified the milestones: 9.0.x, 9.0.5 Mar 18, 2025
@cshung cshung merged commit 9226298 into dotnet:release/9.0-staging Mar 18, 2025
96 of 97 checks passed
@cshung cshung deleted the backport/pr-113032-to-release/9.0-staging branch March 18, 2025 18:55
@github-actions github-actions bot locked and limited conversation to collaborators Apr 18, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-GC-coreclr Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants