Skip to content

[mono][jit] Fix float to int/uint casting behavior#86941

Merged
jandupej merged 4 commits intodotnet:mainfrom
jandupej:cast-float-int
Jun 22, 2023
Merged

[mono][jit] Fix float to int/uint casting behavior#86941
jandupej merged 4 commits intodotnet:mainfrom
jandupej:cast-float-int

Conversation

@jandupej
Copy link
Copy Markdown
Contributor

@jandupej jandupej commented May 31, 2023

Modifies f32 to i32 and u32 casting behavior on arm64 so that overflow cases saturate. 16-bit and 8-bit integers retain previous casting model. Addresses #85316. Enables SIMD f->i casting tests.

@jandupej jandupej self-assigned this May 31, 2023
@danmoseley
Copy link
Copy Markdown
Member

Does this need breaking change doc label?

@jandupej
Copy link
Copy Markdown
Contributor Author

Does this need breaking change doc label?

I'm not sure. AFAIK the overflow behavior of these casts is not standardized in .NET. The purpose of this change is to move the casting behavior closer to what CoreCLR does and to unify it with the SIMD case.

@jandupej
Copy link
Copy Markdown
Contributor Author

@jkotas Do we need to document this as a breaking change?

@jkotas
Copy link
Copy Markdown
Member

jkotas commented Jun 21, 2023

The overflow behavior of float to int/uint conversion has been documented as unspecified (see e.g. https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/builtin-types/numeric-conversions).

I do not think that this needs breaking change notification. It is changing behavior on one specific runtime flavor to make it more similar to other runtime flavors. Both old and new behavior is valid according to the docs.

@jandupej
Copy link
Copy Markdown
Contributor Author

Thanks for confirming. CI only shows known errors. Merging.

@jandupej jandupej merged commit cd11f88 into dotnet:main Jun 22, 2023
@jandupej jandupej deleted the cast-float-int branch June 22, 2023 08:59
@ghost ghost locked as resolved and limited conversation to collaborators Jul 22, 2023
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.

4 participants