-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Remove redundant sign/zero extension for SIMD broadcasts #108824
base: main
Are you sure you want to change the base?
Conversation
@tannergooding @kunalspathak @dotnet/jit-contrib PTAL |
Diffs show additional improvement with memory source: - mov eax, dword ptr [rbp-0x4C]
- movzx rax, ax
- vpbroadcastw ymm0, eax
+ vpbroadcastw ymm0, word ptr [rbp-0x4C] |
Missed redundant sign-extension with memory source:
mov eax, dword ptr [rbp-0x34]
cwde
vpbroadcastb xmm0, eax |
I'm confident that changes like this are better:
I'm not so confident on changes like: - movsx rdx, dl
vpbroadcastb zmm0, edx
It might be that the typical JIT patterns are already avoiding such 8/16-bit register writes in most cases, but it would be good to double check this. |
if (!varTypeIsFloating(cast->CastToType()) && | ||
!varTypeIsFloating(cast->CastFromType()) && |
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.
Should this rather check for varTypeIsInteger
on both?
We have several types (integer, floating, simd, and mask) so a negative test may accidentally include the wrong things
I can probably bench it (my bot is currently down), but looks like native compilers don't emit any movs e.g. https://godbolt.org/z/hKrGnz7nj |
Notably this is coming from memory, where its safe to elide. The consideration would be when it's coming from register and the value was because of a partial register write (i.e. it wrote just |
Closes #108820
Diffs