JIT: Optimize simple arithmetic with GT_NEG

[EgorBo]

I was surprised it's not handled in morph, e.g. -a + b => b - a.

int M1(int a, int b) => -a +  b; // optimize to  "b - a"
int M2(int a, int b) =>  a + -b; // optimize to  "a - b"
int M3(int a, int b) =>  a - -b; // optimize to  "a + b"
int M4(int a, int b) => -a - -b; // optimize to  "b - a"

Current codegen:

; Method CC:M1(int,int):int:this
G_M48868_IG02:
       mov      eax, edx
       neg      eax
       add      eax, r8d

; Method CC:M2(int,int):int:this
G_M15271_IG02:
       mov      eax, r8d
       neg      eax
       add      eax, edx

; Method CC:M3(int,int):int:this
G_M13857_IG02:
       mov      eax, r8d
       neg      eax
       sub      edx, eax
       mov      eax, edx

; Method CC:M4(int,int):int:this
G_M28384_IG02:
       mov      eax, edx
       neg      eax
       mov      edx, r8d
       neg      edx
       sub      eax, edx

Expected codegen: https://godbolt.org/z/nmZxv8

Also:

int M5(int a) =>  -a / 10;  // optimize to  "a / -10"
int M6(int a) => -(a / 10); // optimize to  "a / -10"
int M7(int a) =>  -a * 10;  // optimize to  "a * -10"
int M8(int a) => -(a * 10); // optimize to  "a * -10"

category:cq
theme:basic-cq
skill-level:beginner
cost:small

[JulieLeeMSFT]

With the help with Eugene, I was able to get optimized code for M1 to M4 as shown in https://godbolt.org/z/nmZxv8.
Found some issues during jit-diff that does not handle cases where exception is thrown from both operands, and investigation WIP.

[erozenfeld]

Note that currently morph does the opposite transformation when one of the operands is a const:

runtime/src/coreclr/src/jit/morph.cpp

Lines 13217 to 13263 in 7742b57

	case GT_SUB:
	if (tree->gtOverflow())
	{
	goto CM_OVF_OP;
	}
	// TODO #4104: there are a lot of other places where
	// this condition is not checked before transformations.
	if (fgGlobalMorph)
	{
	/* Check for "op1 - cns2" , we change it to "op1 + (-cns2)" */
	noway_assert(op2);
	if (op2->IsCnsIntOrI())
	{
	/* Negate the constant and change the node to be "+" */
	op2->AsIntConCommon()->SetIconValue(-op2->AsIntConCommon()->IconValue());
	op2->AsIntConRef().gtFieldSeq = FieldSeqStore::NotAField();
	oper = GT_ADD;
	tree->ChangeOper(oper);
	goto CM_ADD_OP;
	}
	/* Check for "cns1 - op2" , we change it to "(cns1 + (-op2))" */
	noway_assert(op1);
	if (op1->IsCnsIntOrI())
	{
	noway_assert(varTypeIsIntOrI(tree));
	// The type of the new GT_NEG node cannot just be op2->TypeGet().
	// Otherwise we may sign-extend incorrectly in cases where the GT_NEG
	// node ends up feeding directly into a cast, for example in
	// GT_CAST<ubyte>(GT_SUB(0, s_1.ubyte))
	tree->AsOp()->gtOp2 = op2 = gtNewOperNode(GT_NEG, genActualType(op2->TypeGet()), op2);
	fgMorphTreeDone(op2);
	oper = GT_ADD;
	tree->ChangeOper(oper);
	goto CM_ADD_OP;
	}
	/* No match - exit */
	}
	break;

op1 - cns2 => op1 + (-cns2)
cns1 - op2 => cns1 + (-op2)

The first transformation is fine, since we are negating a const.

The second transformation is then continued

cns1 - op2 => cns1 + (-op2) => (-op2) + cns1

to canonicalize the location of the const.
Since constants are canonicalized as second operands, transformations like

((x+icon1)+(y+icon2)) => ((x+y)+(icon1+icon2))

and

((x+icon1)+icon2) => (x+(icon1+icon2))

are simplified.

@JulieLeeMSFT I think you should just disable your optimization when the operand that's not negated is a const.

[JulieLeeMSFT]

That seems the best solution. We will skip transforming NEG when non negated operand is constant. Will check if bitwise shift pattern needs to be revisited where NEG was replaced with SUB pattern.

[JulieLeeMSFT]

Simple ADD and SUB arithmetic optimization has been completed with no regression.

• We skipped optimizing patterns when non-NEG operand is constant as discussed above.
• Many patterns in the jit-diff --frameworks test have constant operands, so code size reduction is not big.
• NEG operator is more expensive (e.g., 5-6 CPU cycles) than SUB operator (e.g., 1 cycle) in terms of latency, so it shows better perf scores when optimized.

PMI CodeSize Diffs for System.Private.CoreLib.dll, framework assemblies for  default jit
Summary of Code Size diffs:
(Lower is better)
Total bytes of diff: -50 (-0.00% of base)
    diff is an improvement.
Top file improvements (bytes):
         -13 : System.Private.CoreLib.dasm (-0.00% of base)
         -12 : Microsoft.VisualBasic.Core.dasm (-0.00% of base)
         -12 : System.Runtime.Numerics.dasm (-0.02% of base)
          -8 : System.Private.Xml.dasm (-0.00% 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), 259 unchanged.
Top method improvements (bytes):
         -12 (-2.11% of base) : Microsoft.VisualBasic.Core.dasm - Financial:NPer(double,double,double,double,int):double
         -12 (-7.50% 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):
         -12 (-7.50% of base) : System.Runtime.Numerics.dasm - Complex:op_Division(Complex,Complex):Complex
         -12 (-2.11% of base) : Microsoft.VisualBasic.Core.dasm - Financial:NPer(double,double,double,double,int):double
          -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
          -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), 244443 unchanged.

Next, will look into MUL and DIV operator optimization.

[JulieLeeMSFT]

Completed optimizing + and - operators. Will open a new issue to track * and /.