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] LclMorph GT_IND(GT_LCL_VAR_ADDR) => GT_CAST(GT_LCL_VAR) narrow-cast only #81454

Merged
merged 30 commits into from
Feb 17, 2023

Conversation

TIHan
Copy link
Contributor

@TIHan TIHan commented Feb 1, 2023

Description

Will resolve: #55064

There are cases where IND{short}(LCL_VAR_ADDR{int}) can be transformed to CAST{int <- short <- int}(LCL_VAR{int}) to prevent storing and reading from memory, but only if the indirection is narrow for integer types.

This transformation will not occur for the storage of an assignment: ASG(IND{short}(LCL_VAR_ADDR{int}}, ..).

Acceptance Criteria

  • Disasm tests

@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 Feb 1, 2023
@ghost ghost assigned TIHan Feb 1, 2023
@ghost
Copy link

ghost commented Feb 1, 2023

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

Issue Details

null

Author: TIHan
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@TIHan TIHan changed the title [JIT] EXPERIMENT - Folding IND(GT_LCL_VAR_ADDR) [JIT] EXPERIMENT - Folding GT_IND(GT_LCL_VAR_ADDR) Feb 1, 2023
@TIHan TIHan changed the title [JIT] EXPERIMENT - Folding GT_IND(GT_LCL_VAR_ADDR) [JIT] EXPERIMENT - Folding GT_IND(GT_LCL_VAR_ADDR) => GT_LCL_VAR Feb 1, 2023
@TIHan TIHan changed the title [JIT] EXPERIMENT - Folding GT_IND(GT_LCL_VAR_ADDR) => GT_LCL_VAR [JIT] EXPERIMENT - Folding GT_IND(GT_LCL_VAR_ADDR) => GT_LCL_VAR for small types Feb 4, 2023
@TIHan TIHan changed the title [JIT] EXPERIMENT - Folding GT_IND(GT_LCL_VAR_ADDR) => GT_LCL_VAR for small types [JIT] EXPERIMENT - Folding GT_IND(GT_LCL_VAR_ADDR) => GT_CAST(GT_LCL_VAR) for small types Feb 4, 2023
@TIHan TIHan changed the title [JIT] EXPERIMENT - Folding GT_IND(GT_LCL_VAR_ADDR) => GT_CAST(GT_LCL_VAR) for small types [JIT] Folding GT_IND(GT_LCL_VAR_ADDR) => GT_CAST(GT_LCL_VAR) for small types Feb 4, 2023
@TIHan TIHan changed the title [JIT] Folding GT_IND(GT_LCL_VAR_ADDR) => GT_CAST(GT_LCL_VAR) for small types [JIT] LclMorph GT_IND(GT_LCL_VAR_ADDR) => GT_CAST(GT_LCL_VAR) for small types Feb 4, 2023
@TIHan TIHan changed the title [JIT] LclMorph GT_IND(GT_LCL_VAR_ADDR) => GT_CAST(GT_LCL_VAR) for small types [JIT] LclMorph GT_IND(GT_LCL_VAR_ADDR) => GT_CAST(GT_LCL_VAR) narrow-cast only Feb 7, 2023
@TIHan TIHan closed this Feb 9, 2023
@TIHan TIHan reopened this Feb 9, 2023
@TIHan TIHan marked this pull request as ready for review February 14, 2023 23:23
@TIHan
Copy link
Contributor Author

TIHan commented Feb 14, 2023

@dotnet/jit-contrib cc @jakobbotsch

Diffs

There are a lot of ARM32 regressions, but this is due to the CSE rules for that target.
Other regressions are due to more CSEs.

@jakobbotsch
Copy link
Member

There are a lot of ARM32 regressions, but this is due to the CSE rules for that target.

Can you show some examples? What are/aren't we CSE'ing that changed from before?

src/coreclr/jit/lclmorph.cpp Outdated Show resolved Hide resolved
src/coreclr/jit/lclmorph.cpp Outdated Show resolved Hide resolved
src/coreclr/jit/lclmorph.cpp Outdated Show resolved Hide resolved
src/coreclr/jit/lclmorph.cpp Outdated Show resolved Hide resolved
src/coreclr/jit/lclmorph.cpp Outdated Show resolved Hide resolved
src/coreclr/jit/lclmorph.cpp Outdated Show resolved Hide resolved
src/coreclr/jit/lclmorph.cpp Outdated Show resolved Hide resolved
@TIHan
Copy link
Contributor Author

TIHan commented Feb 15, 2023

@jakobbotsch diff dump of one of the ARM32 regressions
arm32_diff_dump.zip

Co-authored-by: SingleAccretion <62474226+SingleAccretion@users.noreply.github.com>
@TIHan
Copy link
Contributor Author

TIHan commented Feb 16, 2023

@SingleAccretion Ok, looks like I was seeing things before - allowed looking at long types for 32bit and there are quite a bit of improvements.

Copy link
Contributor

@SingleAccretion SingleAccretion left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@jakobbotsch jakobbotsch left a comment

Choose a reason for hiding this comment

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

LGTM.

We looked at one of the arm32 regressions offline and it was caused by CSE being less aggressive due to more tracked locals now existing, so not something we should be compensating for here.

@TIHan TIHan merged commit 925028b into dotnet:main Feb 17, 2023
@TIHan TIHan deleted the fold-gt-lcl-var-addr branch February 17, 2023 23:19
@ghost ghost locked as resolved and limited conversation to collaborators Mar 20, 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.

Sub-optimal code generated for Unsafe.As
4 participants