Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Update conservative value numbers during CSE #9451

Merged
merged 1 commit into from
Feb 10, 2017

Conversation

JosephTremoulet
Copy link

When a CSE candidate's defs all share the same conservative value number,
its uses can be updated to share that conservative value number as well
when CSE is performed, because we are removing any reloads that may have
been the cause of the divergence. Performing this update can improve
subsequent range check elimination when the CSE use is array length or
index in a bounds check.

When a CSE candidate's defs all share the same conservative value number,
its uses can be updated to share that conservative value number as well
when CSE is performed, because we are removing any reloads that may have
been the cause of the divergence.  Performing this update can improve
subsequent range check elimination when the CSE use is array length or
index in a bounds check.
@JosephTremoulet
Copy link
Author

@dotnet/jit-contrib PTAL; I implemented @mikedn's suggestion from #5371. It gets the related (straight-line) case in #9097; it doesn't get the original (loop) case from #5371, but is an incremental step in the right direction nonetheless.

Diffs are a bunch of removed bounds checks and null checks.

SPMI asm diffs report 0.83% codesize improvement across 6474 affected methods, jit-analyze summary as follows:

Summary:
(Note: Lower is better)

Total bytes of diff: -4882 (0.00 % of base)
    diff is an improvement.

Total byte diff includes 0 bytes from reconciling methods
        Base had    0 unique methods,        0 unique bytes
        Diff had    0 unique methods,        0 unique bytes

Top file improvements by size (bytes):
       -1196 : dasmset_1\diff\Methodical\fp\exgen\10w5d_cs_do\10w5d_cs_do.dasm (-0.13 % of base)
       -1195 : dasmset_1\diff\Methodical\fp\exgen\10w5d_cs_ro\10w5d_cs_ro.dasm (-0.14 % of base)
        -925 : dasmset_1\diff\System.Private.CoreLib.dasm (-0.03 % of base)
        -167 : dasmset_1\diff\System.Linq.Parallel.dasm (-0.03 % of base)
        -162 : dasmset_1\diff\Microsoft.CSharp.dasm (-0.04 % of base)

65 total files with size differences (65 improved, 0 regressed).

Top method improvements by size (bytes):
         -63 : dasmset_1\diff\System.Private.CoreLib.dasm - System.Globalization.CultureData:get_SENGDISPLAYNAME():ref:this
         -61 : dasmset_1\diff\System.Reflection.Metadata.dasm - System.Reflection.PortableExecutable.PEReader:GetPESectionBlock(int):ref:this
         -56 : dasmset_1\diff\System.Linq.Parallel.dasm - System.Linq.Parallel.FixedMaxHeap`1[Int32][System.Int32]:HeapifyRoot():this
         -52 : dasmset_1\diff\System.Private.CoreLib.dasm - System.Globalization.DateTimeFormatInfo:get_DateTimeOffsetPattern():ref:this
         -49 : dasmset_1\diff\Methodical\fp\exgen\10w5d_cs_do\10w5d_cs_do.dasm - testout1:Func_0_1_1_2_1():double

466 total methods with size differences (466 improved, 0 regressed).

Copy link

@pgavlin pgavlin left a comment

Choose a reason for hiding this comment

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

LGTM

@briansull
Copy link

LGTM,
I think these are some of the same range checks that we get if we re-run value numbering a second time after we do the CSE phase...

@JosephTremoulet JosephTremoulet merged commit b0bb85d into dotnet:master Feb 10, 2017
@JosephTremoulet JosephTremoulet deleted the UpdateValnum branch February 10, 2017 00:11
@karelz karelz modified the milestone: 2.0.0 Aug 28, 2017
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
Update conservative value numbers during CSE

Commit migrated from dotnet/coreclr@b0bb85d
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants