Expand BBJ_RETURN blocks with bool conditions

[EgorBo]

Expand BBJ_RETURN basic blocks with boolean conditions, let me explain it via a pic:

So it basically de-optimizes the control flow (optimized by Roslyn) but makes it a bit more friendly for optimizations, e.g. now both BB1 and BB2 jump into the same block and can be now handled by the optOptimizeBools optimizations, e.g.:

static bool AreZero(int x, int y)
{
    return x == 0 && y == 0;
}

Was:

G_M22205_IG01:
G_M22205_IG02:
       test     ecx, ecx
       jne      SHORT G_M22205_IG05
G_M22205_IG03:		;; bbWeight=0.50
       test     edx, edx
       sete     al
       movzx    rax, al
G_M22205_IG04:		;; bbWeight=0.50
       ret      
G_M22205_IG05:		;; bbWeight=0.50
       xor      eax, eax
G_M22205_IG06:		;; bbWeight=0.50
       ret      
; Total bytes of code: 16

Now (without optMergeBoolReturns):

G_M22205_IG01:
G_M22205_IG02:
       or       edx, ecx
       jne      SHORT G_M22205_IG05
G_M22205_IG03:
       mov      eax, 1
G_M22205_IG04:
       ret      
G_M22205_IG05:
       xor      eax, eax
G_M22205_IG06:
       ret   

With optMergeBoolReturns:

G_M22205_IG01:
G_M22205_IG02:
       or       edx, ecx
       sete    al
       movzx    rax, al
G_M22205_IG03:
       ret      

So my de-optimization basically helped optOptimizeBools and now it's easier to go further and replace it with just return x | y != 0 without jumps (see https://github.com/dotnet/coreclr/issues/27148).

The fgExpandBoolReturns and optMergeBoolReturns impl are just proof of concept so let me know please if I am moving in a wrong direction 🙂

Jit diff:

Found 68 files with textual diffs.

Summary:
(Lower is better)

Total bytes of diff: -3914 (-0.01% of base)
    diff is an improvement.

Top file improvements by size (bytes):
        -549 : Microsoft.CodeAnalysis.CSharp.dasm (-0.01% of base)
        -420 : System.Private.CoreLib.dasm (-0.01% of base)
        -308 : System.Private.Xml.dasm (-0.01% of base)
        -213 : System.Linq.Parallel.dasm (-0.01% of base)
        -201 : Microsoft.CodeAnalysis.VisualBasic.dasm (-0.00% of base)

68 total files with size differences (68 improved, 0 regressed), 61 unchanged.

Top method regressions by size (bytes):
           7 ( 4.05% of base) : Microsoft.CodeAnalysis.dasm - SwitchBucket:BucketOverflowUInt64Limit(ref,ref):bool
           6 ( 1.72% of base) : NuGet.Packaging.dasm - NuGet.Packaging.PackageHelper:IsPackageFile(ref,int):bool
           4 ( 1.12% of base) : Microsoft.CodeAnalysis.CSharp.dasm - Microsoft.CodeAnalysis.CSharp.Syntax.InternalSyntax.LanguageParser:IsPossibleExpression():bool:this
           4 ( 3.15% of base) : Microsoft.CodeAnalysis.CSharp.dasm - Microsoft.CodeAnalysis.CSharp.Symbols.DynamicTypeSymbol:Equals(ref,bool,bool):bool:this
           4 ( 1.30% of base) : System.Private.Xml.dasm - System.Xml.Xsl.IlGen.XmlILVisitor:CachesResult(ref):bool:this

Top method improvements by size (bytes):
         -41 (-6.47% of base) : NuGet.Versioning.dasm - NuGet.Versioning.FloatRange:Satisfies(ref):bool:this
         -41 (-1.97% of base) : System.Linq.Parallel.dasm - TakeOrSkipWhileQueryOperatorEnumerator`1[Vector`1,Int64][System.Numerics.Vector`1[System.Single],System.Int64]:MoveNext(byref,byref):bool:this
         -35 (-10.74% of base) : System.IO.IsolatedStorage.dasm - System.IO.IsolatedStorage.IsolatedStorageFile:ContainsUnknownFiles(ref):bool:this
         -35 (-1.78% of base) : System.Linq.Parallel.dasm - TakeOrSkipWhileQueryOperatorEnumerator`1[Double,Int64][System.Double,System.Int64]:MoveNext(byref,byref):bool:this
         -35 (-6.92% of base) : System.Private.CoreLib.dasm - System.Nullable:Equals(struct,struct):bool (6 methods)

Top method regressions by size (percentage):
           7 ( 4.05% of base) : Microsoft.CodeAnalysis.dasm - SwitchBucket:BucketOverflowUInt64Limit(ref,ref):bool
           4 ( 3.15% of base) : Microsoft.CodeAnalysis.CSharp.dasm - Microsoft.CodeAnalysis.CSharp.Symbols.DynamicTypeSymbol:Equals(ref,bool,bool):bool:this
           6 ( 1.72% of base) : NuGet.Packaging.dasm - NuGet.Packaging.PackageHelper:IsPackageFile(ref,int):bool
           3 ( 1.69% of base) : Microsoft.Diagnostics.Tracing.TraceEvent.dasm - Microsoft.Diagnostics.Tracing.Stacks.CallTreeNode:get_HasChildren():bool:this
           1 ( 1.32% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - MemberLookup:ShouldLookupExtensionMethods(int,ref):bool

Top method improvements by size (percentage):
          -4 (-26.67% of base) : Microsoft.CSharp.dasm - BinOpFullSig:isLifted():bool:this
          -4 (-26.67% of base) : Microsoft.CSharp.dasm - UnaOpFullSig:isLifted():bool:this
          -4 (-26.67% of base) : System.ComponentModel.TypeConverter.dasm - FilterCacheItem:IsValid(ref):bool:this
          -4 (-26.67% of base) : System.Data.Common.dasm - System.Data.LookupNode:DependsOn(ref):bool:this
          -8 (-26.67% of base) : System.Diagnostics.TraceSource.dasm - System.Diagnostics.BooleanSwitch:get_Enabled():bool:this

Full diff: https://gist.github.com/EgorBo/6f84535bffe833981e550209dc1b8c56

[jkotas]

cc @dotnet/jit-contrib

[AndyAyersMS]

I wonder if the expansion part of this should be moved into the "constant value return merging" logic that was introduced in #13792.

[EgorBo]

@AndyAyersMS will try!

[briansull]

Why do you use
OperIs(GT_EQ, GT_NE, GT_GT, GT_GE, GT_LT, GT_LE, GT_TEST_EQ, GT_TEST_NE)
above on line 8975 and now use
OperIs(GT_EQ, GT_NE)
here?

[EgorBo]

@briansull for some reason the extended list here regresses the diffs (I didn't investigate why yet) but yeah fgExpandBoolReturns needs more work (probably it needs to access cost information to expand only simple ones)

[maryamariyan]

Thank you for your contribution. As announced in dotnet/coreclr#27549 this repository will be moving to dotnet/runtime on November 13. If you would like to continue working on this PR after this date, the easiest way to move the change to dotnet/runtime is:

1. In your coreclr repository clone, create patch by running git format-patch origin
2. In your runtime repository clone, apply the patch by running git apply --directory src/coreclr <path to the patch created in step 1>

[maryamariyan]

Thank you for your contribution. As announced in #27549 the dotnet/runtime repository will be used going forward for changes to this code base. Closing this PR as no more changes will be accepted into master for this repository. If you’d like to continue working on this change please move it to dotnet/runtime.