Skip to content

NativeAOT: Conversion for opcode conv_u4 is unsigned #76386

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Sep 30, 2022
Merged

NativeAOT: Conversion for opcode conv_u4 is unsigned #76386

merged 2 commits into from
Sep 30, 2022

Conversation

@yowl
Copy link
Contributor

@yowl yowl commented Sep 29, 2022

This PR makes the conversion for conv_u4 unsigned, matching conv_u8 in the following switch case. Originates from dotnet/runtimelab#1994 (comment)

@ghost ghost added community-contribution Indicates that the PR has been added by a community member area-crossgen2-coreclr labels Sep 29, 2022
@MichalStrehovsky
Copy link
Member

Do we have the same bug here?

runtime/src/coreclr/tools/Common/TypeSystem/IL/ILImporter.cs

Lines 822 to 824 in 43fa22f

case ILOpcode.conv_u:
ImportConvert(WellKnownType.UIntPtr, false, false);
break;

MichalStrehovsky
MichalStrehovsky approved these changes Sep 29, 2022
View reviewed changes
Copy link
Member

@MichalStrehovsky MichalStrehovsky left a comment

Choose a reason for hiding this comment

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

Thanks!

@jkotas jkotas mentioned this pull request Sep 29, 2022
Closed
2 tasks
@jkotas
Copy link
Member

jkotas commented Sep 30, 2022

Do we have the same bug here?

I think so - pushed commit with the fix.

@yowl
Copy link
Contributor Author

yowl commented Sep 30, 2022

I think so - pushed commit with the fix.

Thanks!

@jkotas jkotas merged commit 5481c4b into dotnet:main Sep 30, 2022
@yowl yowl deleted the conv-u4-unsigned branch September 30, 2022 19:17
@ghost ghost locked as resolved and limited conversation to collaborators Oct 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@MichalStrehovsky MichalStrehovsky MichalStrehovsky approved these changes

Assignees

No one assigned

Labels

area-crossgen2-coreclr community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@yowl @MichalStrehovsky @jkotas