Simple GT_NEG optimization for #13837

[JulieLeeMSFT]

Fixes #13837

replaces #43148 (use rebase to fix merge related issues).

Total bytes of diff: -42 (-0.00% of base)
    diff is an improvement.
Top file improvements (bytes):
         -13 : System.Private.CoreLib.dasm (-0.00% of base)
          -8 : Microsoft.VisualBasic.Core.dasm (-0.00% of base)
          -8 : System.Private.Xml.dasm (-0.00% of base)
          -8 : System.Runtime.Numerics.dasm (-0.01% of base)
          -4 : Microsoft.CodeAnalysis.CSharp.dasm (-0.00% of base)
          -1 : Microsoft.CodeAnalysis.dasm (-0.00% of base)
6 total files with Code Size differences (6 improved, 0 regressed), 262 unchanged.
Top method improvements (bytes):
          -8 (-1.44% of base) : Microsoft.VisualBasic.Core.dasm - Financial:NPer(double,double,double,double,int):double
          -8 (-5.41% of base) : System.Runtime.Numerics.dasm - Complex:op_Division(Complex,Complex):Complex
          -6 (-1.46% of base) : System.Private.Xml.dasm - XmlSqlBinaryReader:FillAllowEOF():bool:this
          -5 (-0.70% of base) : System.Private.CoreLib.dasm - Number:NumberToFloatingPointBitsSlow(byref,byref,int,int,int):long
          -4 (-1.02% of base) : Microsoft.CodeAnalysis.CSharp.dasm - SourceConstructorSymbol:CalculateLocalSyntaxOffset(int,SyntaxTree):int:this
          -4 (-1.94% of base) : System.Private.CoreLib.dasm - ScopeTree:AddScopeInfo(byte,int):this
          -2 (-1.55% of base) : System.Private.CoreLib.dasm - Grisu3:TryRunCounted(byref,int,Span`1,byref,byref):bool
          -2 (-1.05% of base) : System.Private.CoreLib.dasm - Grisu3:TryRunShortest(byref,byref,byref,Span`1,byref,byref):bool
          -2 (-0.19% of base) : System.Private.Xml.dasm - FloatingDecimal:AdjustDbl(double):double:this
          -1 (-0.20% of base) : Microsoft.CodeAnalysis.dasm - CustomDebugInfoWriter:SerializeReferenceToIteratorClass(String,ArrayBuilder`1)
Top method improvements (percentages):
          -8 (-5.41% of base) : System.Runtime.Numerics.dasm - Complex:op_Division(Complex,Complex):Complex
          -4 (-1.94% of base) : System.Private.CoreLib.dasm - ScopeTree:AddScopeInfo(byte,int):this
          -2 (-1.55% of base) : System.Private.CoreLib.dasm - Grisu3:TryRunCounted(byref,int,Span`1,byref,byref):bool
          -6 (-1.46% of base) : System.Private.Xml.dasm - XmlSqlBinaryReader:FillAllowEOF():bool:this
          -8 (-1.44% of base) : Microsoft.VisualBasic.Core.dasm - Financial:NPer(double,double,double,double,int):double
          -2 (-1.05% of base) : System.Private.CoreLib.dasm - Grisu3:TryRunShortest(byref,byref,byref,Span`1,byref,byref):bool
          -4 (-1.02% of base) : Microsoft.CodeAnalysis.CSharp.dasm - SourceConstructorSymbol:CalculateLocalSyntaxOffset(int,SyntaxTree):int:this
          -5 (-0.70% of base) : System.Private.CoreLib.dasm - Number:NumberToFloatingPointBitsSlow(byref,byref,int,int,int):long
          -1 (-0.20% of base) : Microsoft.CodeAnalysis.dasm - CustomDebugInfoWriter:SerializeReferenceToIteratorClass(String,ArrayBuilder`1)
          -2 (-0.19% of base) : System.Private.Xml.dasm - FloatingDecimal:AdjustDbl(double):double:this
10 total methods with Code Size differences (10 improved, 0 regressed), 258863 unchanged.

[danmoseley]

@JulieLeeMSFT I added triple-back-ticks (```) above and below your text dump above so it's easier to read.

[JulieLeeMSFT]

@CarolEidt please see the below diff. The net effects of before and after are the same. However, we could save the 2nd mov if we moved dword ptr [rsp+E4H] to edx initially. PTAL.

Before:

       mov      edx, ebp
       neg      edx
       add      edx, dword ptr [rsp+E4H]

After:

       mov      eax, dword ptr [rsp+E4H]
       mov      edx, eax
       sub      edx, ebp

PMI dump:

Scope info: begin block BB19, IL range [0B4..0BE)
Scope info: open scopes =
   1 (V01 arg1) [000..202)
   2 (V02 arg2) [000..202)
   13 (V13 loc10) [000..202)
   8 (V08 loc5) [000..202)
   3 (V03 loc0) [000..202)
   9 (V09 loc6) [000..202)
   10 (V10 loc7) [000..202)
Added IP mapping: 0x00B4 STACK_EMPTY (G_M6661_IG16,ins#0,ofs#0) label
Generating: N421 (???,???) [000932] ------------                 IL_OFFSET void   IL offset: 0xb4 REG NA
Generating: N423 (  1,  1) [000101] -----------z       t101 =    LCL_VAR   int    V10 loc7         u:2 rax (last use) REG rax <l:$350, c:$34f>
Generating: N425 (  1,  1) [000104] ------------       t104 =    LCL_VAR   int    V53 tmp23        u:2 rbp REG rbp <l:$240, c:$280>
                                                              /--*  t101   int    
                                                              +--*  t104   int    
Generating: N427 (  3,  3) [000106] ----G-------       t106 = *  SUB       int    REG rdx <l:$359, c:$358>
IN0058:        mov      eax, dword ptr [V10 rsp+E4H]
							V10 in reg rax is becoming live  [000101]
							Live regs: 0000F0E8 {rbx rbp rsi rdi r12 r13 r14 r15} => 0000F0E9 {rax rbx rbp rsi rdi r12 r13 r14 r15}
							V10 in reg rax is becoming dead  [000101]
							Live regs: 0000F0E9 {rax rbx rbp rsi rdi r12 r13 r14 r15} => 0000F0E8 {rbx rbp rsi rdi r12 r13 r14 r15}
							Live vars: {V01 V02 V03 V08 V09 V10 V13 V52 V53} => {V01 V02 V03 V08 V09 V13 V52 V53}
IN0059:        mov      edx, eax
IN005a:        sub      edx, ebp
                                                              /--*  t106   int    
Generating: N429 (  7,  6) [000108] DA--G-------              *  STORE_LCL_VAR int    V32 tmp2         d:3 rdx REG rdx
							V32 in reg rdx is becoming live  [000108]
							Live regs: 0000F0E8 {rbx rbp rsi rdi r12 r13 r14 r15} => 0000F0EC {rdx rbx rbp rsi rdi r12 r13 r14 r15}
							Live vars: {V01 V02 V03 V08 V09 V13 V52 V53} => {V01 V02 V03 V08 V09 V13 V32 V52 V53}

Scope info: end block BB19, IL range [0B4..0BE)

[CarolEidt]

The net effects of before and after are the same. However, we could save the 2nd mov if we moved dword ptr [rsp+E4H] to edx initially.

This appears to be an issue with preferencing, though I can't tell for certain without analyzing the register allocation dump.

[briansull]

This change should fix DevDiv_545500

[JulieLeeMSFT]

This appears to be an issue with preferencing, though I can't tell for certain without analyzing the register allocation dump.

diff-13837.txt

@CarolEidt Attached the log file.

[JulieLeeMSFT]

@dotnet/jit-contrib PTAL. All tests have passed.

[briansull]

Approved
But, I would suggest that we remove all the unnecessary unchanged comment lines

[CarolEidt]

@JulieLeeMSFT - sorry for the delay in analyzing the dump that you uploaded. Here's my analysis

If you look at the dump where we create the RefPositions (I search for "DefList" and then the instruction number we're interested in, [000106] is the first one in this example):

DefList: {  }
N427 (  3,  3) [000106] ----G-------              *  SUB       int    REG NA <l:$359, c:$358>
<RefPosition #309 @427 RefTypeUse <Ivl:7 V10> LCL_VAR BB19 regmask=[allInt] minReg=1 last>
<RefPosition #310 @427 RefTypeUse <Ivl:36 V53> LCL_VAR BB19 regmask=[allInt] minReg=1 last>
Interval 112: int RefPositions {} physReg:NA Preferences=[allInt]
<RefPosition #311 @428 RefTypeDef <Ivl:112> SUB BB19 regmask=[allInt] minReg=1>
Interval <V10/L7> already has a related interval

This says that V10 already has a related interval, so we can't set Interval 112 as its relatedInterval.
This means that when we pick a register for I112 we won't try to give it the same register as V10.

DefList: { N427.t106. SUB }
N429 (  7,  6) [000108] DA--G-------              *  STORE_LCL_VAR int    V32 tmp2         d:3 NA REG NA
<RefPosition #312 @429 RefTypeUse <Ivl:112>  BB19 regmask=[allInt] minReg=1 last>
Assigning related <V32/L22> to <I112>

Here we make V32 the relatedInterval of I112 (the value produced by the SUB). That means it will
try to give I112 and V32 the same register, as a target preference. However, we've lost the more
critical RMW-related preference.

There's a general issue, #11959, for tuning of LSRA preferencing. It would be good to consider this example in that context. What test or library method was this?

[JulieLeeMSFT]

There's a general issue, #11959, for tuning of LSRA preferencing. It would be good to consider this example in that context. What test or library method was this?

Thanks for the analysis. It was from jit-diff --frameworks test.

jit-diff diff --output c:\diffs --frameworks --pmi ...

And, it showed in

.\microsoft.codeanalysis.dasm
RealParser:ConvertDecimalToFloatingPointBits(DecimalFloatingPointString,FloatingPointType,byref):int