Fix wrong copy prop after ASG(promoted LCL_VAR with 1 field, call).

[sandreenko]

Diffs for the library from #41100 :

PMI CodeSize Diffs for C:\Users\AC-93\.nuget\packages\microsoft.net.compilers.toolset\3.8.0-3.20416.3\tasks\netcoreapp3.1\bincore\Microsoft.CodeAnalysis.CSharp.dll for  default jit
Summary of Code Size diffs:
(Lower is better)
Total bytes of diff: 38 (0.001% of base)
    diff is a regression.
Top file regressions (bytes):
          38 : Microsoft.CodeAnalysis.CSharp.dasm (0.001% of base)
1 total files with Code Size differences (0 improved, 1 regressed), 0 unchanged.
Top method regressions (bytes):
          44 (5.556% of base) : Microsoft.CodeAnalysis.CSharp.dasm - PEMethodSymbol:get_ExplicitInterfaceImplementations():ImmutableArray`1:this
Top method improvements (bytes):
          -3 (-0.057% of base) : Microsoft.CodeAnalysis.CSharp.dasm - MethodBodySynthesizer:MakeSubmissionInitialization(ArrayBuilder`1,SyntaxNode,MethodSymbol,SynthesizedSubmissionFields,CSharpCompilation)
          -3 (-0.372% of base) : Microsoft.CodeAnalysis.CSharp.dasm - SynthesizedParameterSymbol:DeriveParameters(MethodSymbol,MethodSymbol):ImmutableArray`1
Top method regressions (percentages):
          44 (5.556% of base) : Microsoft.CodeAnalysis.CSharp.dasm - PEMethodSymbol:get_ExplicitInterfaceImplementations():ImmutableArray`1:this
Top method improvements (percentages):
          -3 (-0.372% of base) : Microsoft.CodeAnalysis.CSharp.dasm - SynthesizedParameterSymbol:DeriveParameters(MethodSymbol,MethodSymbol):ImmutableArray`1
          -3 (-0.057% of base) : Microsoft.CodeAnalysis.CSharp.dasm - MethodBodySynthesizer:MakeSubmissionInitialization(ArrayBuilder`1,SyntaxNode,MethodSymbol,SynthesizedSubmissionFields,CSharpCompilation)
3 total methods with Code Size differences (2 improved, 1 regressed), 37448 unchanged.
Completed analysis in 6.31s

diffs in PEMethodSymbol:get_ExplicitInterfaceImplementations are the fix, diffs in DeriveParameters and MakeSubmissionInitialization are correct, but the base version asm was correct as well (a replacement was wrong but the result was valid).

0 diffs in master framework libraries (crossgen), pmi diffs:

Total bytes of diff: -5 (-0.000% of base)
    diff is an improvement.
Top file regressions (bytes):
           3 : Microsoft.CodeAnalysis.CSharp.dasm (0.000% of base)
Top file improvements (bytes):
          -8 : Microsoft.CodeAnalysis.VisualBasic.dasm (-0.000% of base)
2 total files with Code Size differences (1 improved, 1 regressed), 263 unchanged.
Top method regressions (bytes):
           3 (0.067% of base) : Microsoft.CodeAnalysis.CSharp.dasm - MethodBodySynthesizer:ConstructFieldLikeEventAccessorBody_Regular(SourceEventSymbol,bool,CSharpCompilation,DiagnosticBag):BoundBlock
Top method improvements (bytes):
          -5 (-0.108% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - Binder:BindLambdaBody(LambdaSymbol,DiagnosticBag,byref):BoundBlock:this
          -3 (-0.146% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - LocalRewriter:RewriteWinRtEvent(BoundAddRemoveHandlerStatement,BoundEventAccess,bool):BoundStatement:this

The regression in MethodBodySynthesizer:ConstructFieldLikeEventAccessorBody_Regular is the result of using an unstable mechanism to choose the copy prop replacement, so additional inserts (because we now add for ASG(LCL_VAR, call)) change one replacement and it was less optimal. Both the base and the diff were producing correct asm.

The same for Microsoft.CodeAnalysis.VisualBasic.dasm - Binder:BindLambdaBody(LambdaSymbol,DiagnosticBag,byref):BoundBlock:this and LocalRewriter:RewriteWinRtEvent(BoundAddRemoveHandlerStatement,BoundEventAccess,bool):BoundStatement:this.

Example of the copy diff.

N003 (  1,  3) [000034] -A------R---              *  ASG       ref   
N002 (  1,  1) [000033] D------N----              +--*  LCL_VAR   ref    V07 loc3         d:2
N001 (  1,  1) [000032] ------------              \--*  LCL_VAR   ref    V05 loc1         u:2

N003 (  1,  3) [003384] -A------R---              *  ASG       ref   
N002 (  1,  1) [003383] D------N----              +--*  LCL_VAR   ref    V66 tmp36        d:2
N001 (  1,  1) [000611] ------?-----              \--*  LCL_VAR   ref    V05 loc1         u:2

N009 ( 15, 20) [003382] -A-X--------              *  JTRUE     void  
N008 ( 13, 18) [000602] JA-X--?N----              \--*  EQ        int   
N002 (  3,  2) [000601] ---X--?-----                 +--*  IND       long  
N001 (  1,  1) [000595] ------?-----                 |  \--*  LCL_VAR   ref    V05 loc1         u:2
N007 (  9, 15) [000599] -A----?-----                 \--*  COMMA     long  
N005 (  6, 13) [000597] -A----?-R---                    +--*  ASG       long  
N004 (  3,  2) [000596] D-----?N----                    |  +--*  LCL_VAR   long   V65 tmp35        d:2
N003 (  2, 10) [000594] ------?-----                    |  \--*  CNS_INT(h) long   0xd1ffab1e class
N006 (  3,  2) [000598] ------?-----                    \--*  LCL_VAR   long   V65 tmp35        u:2

in the base we were replacing 000595 with V66 from 000033, in the diff we replace it with V07 from 33 because V07 and V66 switched the order in the hashtable as a random side-effect from correct handling of

N007 ( 21, 21) [000496] -ACXG---R---              *  ASG       struct (copy) $VN.Void
N006 (  3,  2) [000494] D------N----              +--*  LCL_VAR   struct<System.Collections.Immutable.ImmutableArray`1[__Canon], 8>(P) V58 tmp28        d:2
                                                  +--*    ref    V58.array (offs=0x00) -> V255 tmp225      
N005 ( 17, 18) [000481] --CXG-------              \--*  CALL      struct System.Collections.Immutable.ImmutableArray.Create $5ca
N003 (  1,  1) [000480] ------------ arg1 in rdx     +--*  LCL_VAR   ref    V19 loc15        u:2 $393
N004 (  2, 10) [000482] ------------ arg0 in rcx     \--*  CNS_INT(h) long   0xd1ffab1e method $223

there.

The BING spmi was showing me 3 diffs but it was actually the same method 3 times and it was generating a bad code before, now it is fixed, not sure why removeDuplicate did not delete them (-c 105012, 456212, 449211).

The pri1 spmi collection has not found any diffs not caught by PMI.

Fixes #41100, will be ported to RC1.

Thanks Andy for the analysis, the repro test, and the proposed fix.

[sandreenko]

PTAL @AndyAyersMS @dotnet/jit-contrib

[AndyAyersMS]

Changes LGTM.

You looked into the test failures? I took a quick look and the all seem unrelated.

Can you open a follow-up about potential missing VNs? Or maybe more broadly review all the places where we use lvaInSsa?

[sandreenko]

You looked into the test failures? I took a quick look and the all seem unrelated.

All tests were passing before the last commit (comment only change), so I think ThreadStartUInt_2.sh is an unrelated unstable failures but have not found open issues for them, ran a retry. System.Xml.Xsl.XslCompiledTransformApi.Tests failure is covered by #36903.

[AndyAyersMS]

/backport to release/5.0

[github-actions]

Started backporting to release/5.0: https://github.com/dotnet/runtime/actions/runs/221470846