Skip to content

Simplify impSIMDPopStack and delete a quirk from gtNewLoadValueNode#85070

Merged
EgorBo merged 2 commits intodotnet:mainfrom
SingleAccretion:Simpler-impSIMDPopStack-Upstream
Apr 20, 2023
Merged

Simplify impSIMDPopStack and delete a quirk from gtNewLoadValueNode#85070
EgorBo merged 2 commits intodotnet:mainfrom
SingleAccretion:Simpler-impSIMDPopStack-Upstream

Conversation

@SingleAccretion
Copy link
Contributor

@SingleAccretion SingleAccretion commented Apr 19, 2023

Takes advantage of recent IR improvements.

Diffs.

@ghost ghost added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI community-contribution Indicates that the PR has been added by a community member labels Apr 19, 2023
@ghost
Copy link

ghost commented Apr 19, 2023

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

Issue Details

Takes advantage of recent IR improvements.

Author: SingleAccretion
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

Copy link
Member

@tannergooding tannergooding left a comment

Choose a reason for hiding this comment

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

Nice improvement! This may conflict with one more of the AVX-512 PRs. Most are just attaching to existing switch cases though, so hopefully not too bad

@tannergooding
Copy link
Member

Couple regressions, nothing major but the Matrix4x4 one would be nice to fix as it will show up in perf runs.

Did quite a bit of work improving that in .NET 8 and it was almost perfect (minus a couple register to register moves) and now has a bit of spilling again.

-- Happy to tweak the managed code if its something we can fix there instead. I haven't looked at switching to BitCast yet and its doing "weird" things with Unsafe.As to get the good codegen atm.

@SingleAccretion SingleAccretion changed the title Simplify impSIMDPopStack Simplify impSIMDPopStack and delete a quirk from gtNewLoadValueNode Apr 20, 2023
@SingleAccretion SingleAccretion marked this pull request as ready for review April 20, 2023 18:00
@SingleAccretion
Copy link
Contributor Author

Diffs.

The couple regressions in tests are due to the new logic using gtNewLoadValueNode instead of gtNewOperNode(GT_IND, type, tree);, which sets appropriate side effect flags.

@dotnet/jit-contrib

Copy link
Member

@tannergooding tannergooding left a comment

Choose a reason for hiding this comment

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

Thanks for fixing the Matrix scenario!

@EgorBo EgorBo merged commit c27a160 into dotnet:main Apr 20, 2023
@SingleAccretion SingleAccretion deleted the Simpler-impSIMDPopStack-Upstream branch April 20, 2023 23:10
@ghost ghost locked as resolved and limited conversation to collaborators May 21, 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 community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants