Skip to content

Use correct assignedInterval for SPILL_COST calculation - #55247

Merged
kunalspathak merged 1 commit into
dotnet:mainfrom
kunalspathak:spill-cost
Jul 8, 2021
Merged

Use correct assignedInterval for SPILL_COST calculation#55247
kunalspathak merged 1 commit into
dotnet:mainfrom
kunalspathak:spill-cost

Conversation

@kunalspathak

@kunalspathakkunalspathak commented Jul 7, 2021

Copy link
Copy Markdown
Contributor

In #53853, I changed the SPILL_COST heuristic to also consider reload weight if the variable will be reloaded in future location rather than getting spilled at past location. However, while doing that, I incorrectly checking the wrong interval for isLocalVar. I wanted to use similar condition as used in

if (fromRefPosition->RegOptional() && !(interval->isLocalVar && fromRefPosition->IsActualRef()))

However, the interval I was using was that of the current refposition for which we are allocating the register rather than the interval to which the candidate register is already assigned.

Here is the sample diff of Algorithms.VectorDoubleRenderer:RenderSingleThreadedWithADT(float,float,float,float,float) that we can see: The high weight local variable gets assigned to the register.

image

With better selection of register, we can now eliminate unnecessary spilling/resolution of expensive variables:

image

Some of the other regressions seen will be fixed by #54345.

Fixes: #54352

@ghostghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jul 7, 2021
@kunalspathak

Copy link
Copy Markdown
ContributorAuthor

Code Size

ScenarioOS/archmainPRDiffDiff %
BenchmarksLinux.x641068301069961660.16%
Benchmarkswindows.x647943379432-10.00%
Benchmarkswindows.x862136902141104200.20%
Libraries.crossgen2Linux.x645699857020220.04%
Libraries.crossgen2windows.x6469726865-107-1.53%
Libraries.crossgen2windows.x86240319240312-70.00%
Libraries.pmiLinux.x648221982134-85-0.10%
Libraries.pmiwindows.x6473971743814100.55%
Libraries.pmiwindows.x864106984112635650.14%

Perf Score

OS/archmainPRDiffDiff %
Linux.x64102397100669.68-1727.32-1.69%
windows.x64168025.98164684.59-3341.39-1.99%
windows.x8610261610.2710258036.27-3574-0.03%
Linux.x64107910.52106060.97-1849.55-1.71%
windows.x6426584.6626107-477.66-1.80%
windows.x86535206.26535575.34369.080.07%
Linux.x64708552.68706913.69-1638.99-0.23%
windows.x64128077.33126861.96-1215.37-0.95%
windows.x864150066498040594662922-906002058.5-2.18%

Detail diffs are: https://gist.github.com/kunalspathak/49940d4690ef99f2e24b2ee8bfbac9e2

@kunalspathak
kunalspathak marked this pull request as ready for review July 7, 2021 21:25
@kunalspathakkunalspathak changed the title fix spill cost calculationUse correct assignedInterval for SPILL_COST calculationJul 7, 2021
@kunalspathak

Copy link
Copy Markdown
ContributorAuthor

@dotnet/jit-contrib

@kunalspathak

Copy link
Copy Markdown
ContributorAuthor

Failures seems to be infra issues.

@kunalspathak
kunalspathak merged commit 7f88911 into dotnet:mainJul 8, 2021
@sandreenkosandreenko mentioned this pull request Jul 8, 2021
@kunalspathakkunalspathak mentioned this pull request Jul 8, 2021
@ghostghost locked as resolved and limited conversation to collaborators Aug 7, 2021
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-CodeGen-coreclrCLR 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.

Regressions from using reloadWeight when evaluating spill cost

3 participants

@kunalspathak@BruceForstall@sandreenko