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

JIT: Use unsigned native return types for small structs #86631

Merged
merged 1 commit into from
May 23, 2023

Conversation

jakobbotsch
Copy link
Member

@jakobbotsch jakobbotsch commented May 23, 2023

Prefer the return type to be unsigned when small structs are returned in registers. This causes the backend to use zero extension instead of sign extension in a few cases, which has a smaller encoding on xarch.

Fixes a few small regressions I saw in #86388.

E.g.

[MethodImpl(MethodImplOptions.NoInlining)]
public static (byte, byte) Foo(in (byte, byte) arg) => arg;

Before:

G_M50947_IG02:  ;; offset=0000H
       movsx    rax, word  ptr [rcx]
						;; size=4 bbWeight=1 PerfScore 4.00

G_M50947_IG03:  ;; offset=0004H
       ret      

After:

G_M50947_IG02:  ;; offset=0000H
       movzx    rax, word  ptr [rcx]
						;; size=3 bbWeight=1 PerfScore 2.00

G_M50947_IG03:  ;; offset=0003H
       ret      

Prefer the return type to be unsigned when small structs are returned in
registers. This causes the backend to use zero extension instead of sign
extension in a few cases, which has a smaller encoding on xarch.
@ghost ghost assigned jakobbotsch May 23, 2023
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label May 23, 2023
@ghost
Copy link

ghost commented May 23, 2023

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

Issue Details

Prefer the return type to be unsigned when small structs are returned in registers. This causes the backend to use zero extension instead of sign extension in a few cases, which has a smaller encoding on xarch.

Fixes a few small regressions I saw in #86388.

Author: jakobbotsch
Assignees: jakobbotsch
Labels:

area-CodeGen-coreclr

Milestone: -

@jakobbotsch
Copy link
Member Author

cc @dotnet/jit-contrib PTAL @EgorBo.

Small number of (non-test) improvement. Few regressions related to CSE/register allocation.

@jakobbotsch jakobbotsch requested a review from EgorBo May 23, 2023 14:38
Copy link
Member

@EgorBo EgorBo left a comment

Choose a reason for hiding this comment

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

This is a correctness change, right? Since, I guess, sign extension is not expected when you have C# byte

@jakobbotsch
Copy link
Member Author

This is a correctness change, right? Since, I guess, sign extension is not expected when you have C# byte

No, we don't make any guarantees about normalization when structs are returned in registers, so there's no correctness issue with the existing codegen.

@jakobbotsch jakobbotsch merged commit 62609dc into dotnet:main May 23, 2023
@jakobbotsch jakobbotsch deleted the small-structs-zero-extend branch May 23, 2023 16:54
@ghost ghost locked as resolved and limited conversation to collaborators Jun 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants