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: work around issue with GDV and Boxing #56126

Merged
merged 1 commit into from
Jul 22, 2021

Conversation

AndyAyersMS
Copy link
Member

If a call is a GDV candidate and returns a struct via hidden buffer, and that
return value is immediately boxed, the GDV expansion will produce IR in
incorrect order, leading to bad codegen.

This seems to be a rare enough sequence that disabling GDV is a reasonable
workaround for now.

Actually the box expansion is producing IR in the wrong order and GDV fails
to fix the order (unlike inlining, which does fix the order).

Longer term we should avoid producing out of order IR. But that seems a bit
more complicated and may have other CQ impact.

Added a test case.

Closes #53549.

If a call is a GDV candidate and returns a struct via hidden buffer, and that
return value is immediately boxed, the GDV expansion will produce IR in
incorrect order, leading to bad codegen.

This seems to be a rare enough sequence that disabling GDV is a reasonable
workaround for now.

Actually the box expansion is producing IR in the wrong order and GDV fails
to fix the order (unlike inlining, which does fix the order).

Longer term we should avoid producing out of order IR. But that seems a bit
more complicated and may have other CQ impact.

Added a test case.

Closes dotnet#53549.
@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 Jul 22, 2021
@AndyAyersMS
Copy link
Member Author

@EgorBo PTAL
cc @dotnet/jit-contrib

Not thrilled with dropping GDV for some call sites, but doing this avoids the problem, and the number of cases where we see this pattern is likely pretty small.

No diffs in SPMI (other fix attempts had ~100's of diffs in asp.net). Also I'm able to run libraries xunit test harness under tiered PGO without odd errors.

@AndyAyersMS AndyAyersMS merged commit 47e82c1 into dotnet:main Jul 22, 2021
@AndyAyersMS AndyAyersMS deleted the FixGDVStructBoxIssue branch July 22, 2021 21:56
@EgorBo EgorBo mentioned this pull request Aug 15, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Aug 21, 2021
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.

Arm32 Libraries tests failures with full PGO
2 participants