Skip to content

Fix JIT using too wide indirections when returning small structs - #68160

Merged
jakobbotsch merged 8 commits into
dotnet:mainfrom
jakobbotsch:fix-58874
Apr 20, 2022
Merged

Fix JIT using too wide indirections when returning small structs#68160
jakobbotsch merged 8 commits into
dotnet:mainfrom
jakobbotsch:fix-58874

Conversation

@jakobbotsch

@jakobbotschjakobbotsch commented Apr 18, 2022•
edited
Loading

Copy link
Copy Markdown
Member

Fix#58874
Fix#64802
Fix#68157

We should backport this given the two customer reports.

@ghostghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Apr 18, 2022
@ghost

ghost commented Apr 18, 2022

Copy link
Copy Markdown

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

Issue Details

Fix #58874
Fix #64802
Fix #68157

Author:jakobbotsch
Assignees:-
Labels:

area-CodeGen-coreclr

Milestone:-

EgorBo
EgorBo reviewed Apr 18, 2022
View reviewed changes
Comment threadsrc/tests/JIT/Regression/JitBlue/Runtime_58874/Runtime_58874.cs Outdated
Tornhoof
Tornhoof reviewed Apr 18, 2022
View reviewed changes
Comment threadsrc/coreclr/jit/lower.cpp
GenTree* retVal = ret->gtGetOp1();
GenTree* retVal = ret->gtGetOp1();
var_types nativeReturnType = comp->info.compRetNativeType;
// Note: small types are returned as INT.

@TornhoofTornhoofApr 18, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is that comment still correct with your changes?

@EgorBoEgorBoApr 18, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should be still relevant, the following genActualType should upscale all small-types to int

@jakobbotschjakobbotschApr 18, 2022•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Right, there are two nodes here, the return and the operand of the return. The return is still wide but the bug was that we were retyping the operand to a wider type, which is wrong.

@jakobbotsch

jakobbotsch commented Apr 20, 2022

Copy link
Copy Markdown
MemberAuthor

Failure is dotnet/arcade#9088

@jakobbotsch
jakobbotsch merged commit 47f9ecd into dotnet:mainApr 20, 2022
@jakobbotsch
jakobbotsch deleted the fix-58874 branch April 20, 2022 16:07
directhex pushed a commit to directhex/runtime that referenced this pull request Apr 21, 2022
@jakobbotschJo Shields
Fix JIT using too wide indirections when returning small structs (dot…
f083867
…net#68160)
Fixdotnet#58874Fixdotnet#64802Fixdotnet#68157
@jakobbotsch

jakobbotsch commented Apr 22, 2022

Copy link
Copy Markdown
MemberAuthor

/backport to release/6.0

@github-actions

github-actionsBot commented Apr 22, 2022

Copy link
Copy Markdown
Contributor

Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/2206848351

@github-actionsgithub-actionsBot mentioned this pull request Apr 22, 2022
Merged
@SingleAccretionSingleAccretion mentioned this pull request Apr 27, 2022
Merged
@ghostghost locked as resolved and limited conversation to collaborators May 22, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@EgorBoEgorBoEgorBo approved these changes
+1 more reviewer
@TornhoofTornhoofTornhoof left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@jakobbotschjakobbotsch

Labels

area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Milestone

No milestone

3 participants

@jakobbotsch@EgorBo@Tornhoof