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

[interp] Repair interp simd's ability to find certain operators; support vectors of char #86475

Merged
merged 1 commit into from
May 19, 2023

Conversation

kg
Copy link
Member

@kg kg commented May 19, 2023

While inspecting the performance of some vectorized BCL code, I noticed that I accidentally mangled one of the binary search lists.
There also appear to be some cases where vectors of char are used, so I touched up the code to make sure that we will select the right intrinsic in that case if we already supported U2.

Make sure vectors of char work in more cases
@ghost
Copy link

ghost commented May 19, 2023

Tagging subscribers to this area: @BrzVlad, @kotlarmilos
See info in area-owners.md if you want to be subscribed.

Issue Details

While inspecting the performance of some vectorized BCL code, I noticed that I accidentally mangled one of the binary search lists.
There also appear to be some cases where vectors of char are used, so I touched up the code to make sure that we will select the right intrinsic in that case if we already supported U2.

Author: kg
Assignees: -
Labels:

area-Codegen-Interpreter-mono

Milestone: -

@kg
Copy link
Member Author

kg commented May 19, 2023

The two browser-wasm windows lane failures appear to be some sort of AZDO git flake (fetch failed) that persisted even after I re-ran the lanes. Debugger failure is an unrelated chrome crash. One of the others is NuGet Migrations.

@kg kg merged commit 51d467a into dotnet:main May 19, 2023
@tannergooding
Copy link
Member

There also appear to be some cases where vectors of char are used

Can you point me to this code? Char is strictly unsupported and any of those paths should throw. We have one managed helper, that I’m aware of, which deals with ref char and returning a Vector###<ushort> but it’s not intended to be an intrinsic

@kg
Copy link
Member Author

kg commented May 19, 2023

There also appear to be some cases where vectors of char are used

Can you point me to this code? Char is strictly unsupported and any of those paths should throw. We have one managed helper, that I’m aware of, which deals with ref char and returning a Vector###<ushort> but it’s not intended to be an intrinsic

I was digging into cases where intrinsics weren't being properly handled, and I saw a lot of stuff over <T> that appeared to support char. If char is officially not supported then I can remove that part from this in a future PR.

@tannergooding
Copy link
Member

If char is officially not supported then I can remove that part from this in a future PR.

It's not. We only support the types that return IsSupported == true: https://github.com/dotnet/runtime/blob/main/src/libraries/System.Private.CoreLib/src/System/Runtime/Intrinsics/Vector128_1.cs#L65-L84

Anything else should be throwing a NotSupportedException if a public API is called.

I saw a lot of stuff over that appeared to support char.

We have a lot of code that operates over a backing buffer of char, but the constructed Vector###<T> should always be a supported type (typically ushort).

If you find any code that isn't setup like this, then we probably have a bug that snuck in somewhere and that we should get patched.

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