JIT: Clone ADDR(LCL) for DUP

[EgorBo]

Low-hanging fruit, closes #57055 and codegen issues in this tweet https://twitter.com/badamczewski01/status/1484990815910957058

Diffs: https://dev.azure.com/dnceng/public/_build/results?buildId=1572041&view=ms.vss-build-web.run-extensions-tab

[ghost]

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

Issue Details

---

Low-hanging fruit, closes #57055 and codegen issues in this tweet https://twitter.com/badamczewski01/status/1484990815910957058

Author:	EgorBo
Assignees:	-
Labels:	area-CodeGen-coreclr
Milestone:	-

[tannergooding]

Should this also check for TYP_I_IMPL or GT_IND? What about the GT_OBJ scenario?

Asking since these are all special cases we handle in impSIMDPopStack to cover ref T, T*, and a few other scenarios: https://github.com/dotnet/runtime/blob/main/src/coreclr/jit/simd.cpp#L1225-L1305

So I'd imagine they may also qualify here in some circumstances.

[SingleAccretion]

This is checking for addresses only, so, no need to check for indirections.

This should probably be using impIsAddressInLocal to catch both LCL_VAR_ADDR and LCL_FLD_ADDR - equivalent trees.

[kunalspathak]

coreclr_tests, libraries_tests and libraries.crossgen2 seems to have regression core_clr and libraries_test the most:

Summary of Code Size diffs:
(Lower is better)

Total bytes of base: 168170904 (overridden on cmd)
Total bytes of diff: 168332188 (overridden on cmd)
Total bytes of delta: 161284 (0.10 % of base)
    diff is a regression.
    relative diff is a regression.

coreclr_tests.pmi.windows.x64.checked.mch:

Summary of Code Size diffs:
(Lower is better)

Total bytes of base: 127473217 (overridden on cmd)
Total bytes of diff: 127708677 (overridden on cmd)
Total bytes of delta: 235460 (0.18 % of base)
    diff is a regression.
    relative diff is a regression.

I guess there is some more tuning needed to make sure we don't do unnecessary copies? I will have to see if some of them can be eliminated by #64130.

[EgorBo]

coreclr_tests, libraries_tests and libraries.crossgen2 seems to have regression core_clr and libraries_test the most:

Thanks, I didn't realize that for compDbg I had to always "clone" rather than making a temp, fixed.

This should probably be using impIsAddressInLocal to catch both LCL_VAR_ADDR and LCL_FLD_ADDR - equivalent trees.

I am seeing a lot of size regression with impIsAddressInLocal so I decided to leave it as is. Also, added a workaround to fix more regressions where clonned byref is used to zero a local struct, e.g. sharplab

There are still some regressions since struct enreg is not always a good thing e.g. when we need to copy struct. Overall, diff is negative and the example from the tweet is improved.

[kunalspathak]

There are still some regressions since struct enreg is not always a good thing e.g. when we need to copy struct. Overall, diff is negative and the example from the tweet is improved.

Can you paste the diff of regression with a sample example? It will help me in my future work on structs.

[EgorBo]

@kunalspathak here is a minimal repro:

public virtual DateTimeOffset? EndTime { get; set; }

public TimeSpan Duration() => 
    (EndTime?.Subtract(DateTimeOffset.MinValue)).Value;

Diff: https://www.diffchecker.com/QZjlpePR

All of them look like this (quite often connected with Nullable<>). I feel like I should not be doing this opt for something that won't be enregistrated but it's too early to check I guess (I tried structPromotionHelper->CanPromoteStructType but it made diffs way smaller)

Diffs: https://dev.azure.com/dnceng/public/_build/results?buildId=1572041&view=ms.vss-build-web.run-extensions-tab

[kunalspathak]

LGTM