Skip to content

Conversation

@sandreenko
Copy link
Contributor

The value of lvDoNotEnregister is not finalized when we call optCopyProp_LclVarScore during global morph, so we should not rely on it.

It causes many asm diffs when we change what we mark as lvDoNotEnregister so I want to avoid this noise by merging this first.

Note: there is another call to optCopyProp_LclVarScore during optCopyProp where such check could make sense but it is guarded with

if (lvaTable[lclNum].lvDoNotEnregister != lvaTable[newLclNum].lvDoNotEnregister)
{
continue;
}

so also not needed.

The diffs are small and are mostly noise.

SuperPMI diffs
benchmarks.run.windows.x64.checked.mch
Total bytes of delta: 32
1 total files with Code Size differences (0 improved, 1 regressed)

libraries.pmi.windows.x64.checked.mch
Total bytes of delta: 3
27 total files with Code Size differences (16 improved, 11 regressed)

libraries_tests.pmi.windows.x64.checked.mch
Total bytes of delta: -26
2 total files with Code Size differences (2 improved, 0 regressed)

coreclr_tests.pmi.Linux.arm64.checked.mch
Total bytes of delta: -8
1 total files with Code Size differences (1 improved, 0 regressed)

libraries.pmi.Linux.arm64.checked.mch
Total bytes of delta: -20
27 total files with Code Size differences (16 improved, 11 regressed)

libraries_tests.pmi.Linux.arm64.checked.mch
Total bytes of delta: -56
9 total files with Code Size differences (7 improved, 2 regressed)

coreclr_tests.pmi.Linux.x64.checked.mch
Total bytes of delta: -3
2 total files with Code Size differences (1 improved, 1 regressed)

libraries.pmi.Linux.x64.checked.mch
Total bytes of delta: -21
29 total files with Code Size differences (18 improved, 11 regressed)

libraries_tests.pmi.Linux.x64.checked.mch
Total bytes of delta: -107
11 total files with Code Size differences (9 improved, 2 regressed)

coreclr_tests.pmi.windows.arm64.checked.mch
Total bytes of delta: -8
1 total files with Code Size differences (1 improved, 0 regressed)

libraries.pmi.windows.arm64.checked.mch
Total bytes of delta: -20
27 total files with Code Size differences (16 improved, 11 regressed)

libraries_tests.pmi.windows.arm64.checked.mch
Total bytes of delta: -44
9 total files with Code Size differences (7 improved, 2 regressed)

benchmarks.run.windows.x86.checked.mch
Total bytes of delta: -734
17 total files with Code Size differences (16 improved, 1 regressed)

coreclr_tests.pmi.windows.x86.checked.mch
Total bytes of delta: -111
35 total files with Code Size differences (11 improved, 24 regressed)

libraries.pmi.windows.x86.checked.mch
Total bytes of delta: 86
5 total files with Code Size differences (0 improved, 5 regressed)

libraries_tests.pmi.windows.x86.checked.mch
Total bytes of delta: -1748
80 total files with Code Size differences (78 improved, 2 regressed)

libraries.pmi.Linux.arm.checked.mch
Total bytes of delta: -10
32 total files with Code Size differences (17 improved, 15 regressed)

libraries_tests.pmi.Linux.arm.checked.mch
Total bytes of delta: -56
7 total files with Code Size differences (6 improved, 1 regressed)

@sandreenko sandreenko added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jun 15, 2021
@sandreenko
Copy link
Contributor Author

PTAL @BruceForstall @dotnet/jit-contrib

Copy link
Contributor

@BruceForstall BruceForstall left a comment

Choose a reason for hiding this comment

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

LGTM.

Should lvDoNotEnregister be a PhasedVar<bool> (at least in DEBUG)?

@sandreenko
Copy link
Contributor Author

Should lvDoNotEnregister be a PhasedVar (at least in DEBUG)?

I don't think it is possible now, we are sure that it is set only after lowering, the check was added in #52802

but we read it early and often to guess about perf transformations or avoid checks that are not necessary. I don't think we could finalize its value earlier without pulling target dependencies from lowering to morph so I don't see a good way to make it a phased variable, but I will think about it for 7.0.

@sandreenko sandreenko merged commit 71e13c2 into dotnet:main Jun 15, 2021
@sandreenko sandreenko deleted the dontRelyOnUnsetFlag branch June 15, 2021 21:11
@ghost ghost locked as resolved and limited conversation to collaborators Jul 15, 2021
@kunalspathak
Copy link
Contributor

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.

3 participants