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

Improve TeddyHelper.RightShift helpers for ARM #96928

Merged
merged 1 commit into from
Jan 14, 2024

Conversation

MihaZupan
Copy link
Member

@tannergooding mentioned on Discord that AdvSimd.ExtractVector128 is the comparable ARM instruction to Ssse3.AlignRight.
This lets us delete the logic we were using to emulate the behavior via shuffles.

@MihaZupan MihaZupan added this to the 9.0.0 milestone Jan 13, 2024
@MihaZupan MihaZupan self-assigned this Jan 13, 2024
@ghost
Copy link

ghost commented Jan 13, 2024

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

Issue Details

@tannergooding mentioned on Discord that AdvSimd.ExtractVector128 is the comparable ARM instruction to Ssse3.AlignRight.
This lets us delete the logic we were using to emulate the behavior via shuffles.

Author: MihaZupan
Assignees: MihaZupan
Labels:

area-System.Buffers

Milestone: 9.0.0

}
return Ssse3.IsSupported
? Ssse3.AlignRight(right, left, 15)
: AdvSimd.ExtractVector128(left, right, 15);
Copy link
Member

Choose a reason for hiding this comment

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

It's worth noting that...

AlignRight is exposed as (left, right, mask) where left is the upper 128-bits and right is the lower 128-bits of the concatenated vector, therefore mask 1 picks right[1]-right[15] and left[0] as the last element.

AdvSimd.ExtractVector128 is exposed as (upper, lower, mask), but it should've been (lower, upper, mask), so its inverse from the AlignRight order. This can be potentially confusing when reviewing the code.

The PR here is doing it correctly, just wanted to call out that its a weird thing if you go look at stuff

@MihaZupan
Copy link
Member Author

@MihuBot -arm

@MihaZupan MihaZupan merged commit 53a2021 into dotnet:main Jan 14, 2024
175 of 178 checks passed
tmds pushed a commit to tmds/runtime that referenced this pull request Jan 23, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Feb 14, 2024
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.

2 participants