-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
[release/8.0] [mono][interp] Mask all shift amounts #90991
Conversation
Tagging subscribers to this area: @BrzVlad, @kotlarmilos Issue DetailsBackport of #90666 to release/8.0 /cc @BrzVlad Customer ImpactTestingRiskIMPORTANT: If this backport is for a servicing release, please verify that:
|
@marek-safar @jeffschwMSFT - release\8.0 approval needed for this PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approved. once ready you can merge
@BrzVlad @fanyang-mono @SamMonoRT The CI has mono faiures, are they related? Note: Please ping me when ready. I can merge it for you to bypass the merge restriction, so no need to click on "Update branch", or it will restart the CI (only click it if you really really need to rebase on top of the base branch). |
Backport of #90666 to release/8.0
/cc @BrzVlad
Customer Impact
In C#, the shift operator implies a masking of the shift amount according to the operand width. The vectorized intrinsics computing shifts added on interpreter for this release regressed this behavior. Prior to this fix, using Vector128 Shift intrinsics with shift amount over the operand bit width was producing incorrect values.
Testing
Tested locally. Regression discovered with tests from our suite, which are now passing.
Risk
Very low.
IMPORTANT: If this backport is for a servicing release, please verify that:
The PR target branch is
release/X.0-staging
, notrelease/X.0
.If the change touches code that ships in a NuGet package, you have added the necessary package authoring and gotten it explicitly reviewed.