Better codegen for (T)x | (T)y (#58727)

* Better codegen for `(T)x | (T)y`

I added a morph pass to fold expressions like `(T)x | (T)y`
into `(T)(x | y)`. This results in fewer `movzx` instructions
in the asm.

Fixes #13816

* Code review updates

* Rename function to fgMorphCastedBitwiseOp
* Don't fold checked arithmetic
* Reuse op1 node for return value
* Don't run outside global morphing
* Various code style and comment tweaks

* Don't call gtGetOp2 if tree was folded.

If it was folded, it was folded to a unary (cast) operation
and gtGetOp2() will crash.

I also tweaked fgMorphCastedBitwiseOp to return nullptr
if it didn't do anything (to match behaviour of fgMorphCommutative)

* Code review changes for tests

* Removed all but one csproj
* Added tests for scenarios with overflow, compound exprs, side effects

* Add in some asserts and remove a redundant call

* fix typo

* Code review changes:

* Formatting
* Use getters instead of fields
* set flags on op1

* Fix formatting

src/coreclr/jit/compiler.h

@@ -6382,6 +6382,7 @@ class Compiler
 
     GenTreeLclVar* fgMorphTryFoldObjAsLclVar(GenTreeObj* obj);
     GenTree* fgMorphCommutative(GenTreeOp* tree);
+    GenTree* fgMorphCastedBitwiseOp(GenTreeOp* tree);
 
 public:
     GenTree* fgMorphTree(GenTree* tree, MorphAddrContext* mac = nullptr);

src/coreclr/jit/morph.cpp

@@ -10872,6 +10872,70 @@ GenTree* Compiler::fgMorphCommutative(GenTreeOp* tree)
     return op1;
 }
 
+//------------------------------------------------------------------------------
+// fgMorphCastedBitwiseOp : Try to simplify "(T)x op (T)y" to "(T)(x op y)".
+//
+// Arguments:
+//     tree - node to fold
+//
+// Return Value:
+//     A folded GenTree* instance, or nullptr if it couldn't be folded
+GenTree* Compiler::fgMorphCastedBitwiseOp(GenTreeOp* tree)
+{
+    assert(varTypeIsIntegralOrI(tree));
+    assert(tree->OperIs(GT_OR, GT_AND, GT_XOR));
+
+    GenTree*   op1  = tree->gtGetOp1();
+    GenTree*   op2  = tree->gtGetOp2();
+    genTreeOps oper = tree->OperGet();
+
+    // see whether both ops are casts, with matching to and from types.
+    if (op1->OperIs(GT_CAST) && op2->OperIs(GT_CAST))
+    {
+        // bail if either operand is a checked cast
+        if (op1->gtOverflow() || op2->gtOverflow())
+        {
+            return nullptr;
+        }
+
+        var_types fromType   = op1->AsCast()->CastOp()->TypeGet();
+        var_types toType     = op1->AsCast()->CastToType();
+        bool      isUnsigned = op1->IsUnsigned();
+
+        if ((op2->CastFromType() != fromType) || (op2->CastToType() != toType) || (op2->IsUnsigned() != isUnsigned))
+        {
+            return nullptr;
+        }
+
+        // Reuse gentree nodes:
+        //
+        //     tree             op1
+        //     /   \             |
+        //   op1   op2   ==>   tree
+        //    |     |          /   \
+        //    x     y         x     y
+        //
+        // (op2 becomes garbage)
+
+        tree->gtOp1  = op1->AsCast()->CastOp();
+        tree->gtOp2  = op2->AsCast()->CastOp();
+        tree->gtType = fromType;
+
+        op1->gtType                 = genActualType(toType);
+        op1->AsCast()->gtOp1        = tree;
+        op1->AsCast()->CastToType() = toType;
+        op1->SetAllEffectsFlags(tree);
+        // no need to update isUnsigned
+
+        DEBUG_DESTROY_NODE(op2);
+        INDEBUG(op1->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED);
+
+        return op1;
+    }
+
+    return nullptr;
+}
+
 /*****************************************************************************
  *
  *  Transform the given GTK_SMPOP tree for code generation.
@@ -12516,6 +12580,22 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac)
                 op2  = tree->AsOp()->gtOp2;
             }
 
+            if (fgGlobalMorph && varTypeIsIntegralOrI(tree) && tree->OperIs(GT_AND, GT_OR, GT_XOR))
+            {
+                GenTree* result = fgMorphCastedBitwiseOp(tree->AsOp());
+                if (result != nullptr)
+                {
+                    assert(result->OperIs(GT_CAST));
+                    assert(result->AsOp()->gtOp2 == nullptr);
+                    // tree got folded to a unary (cast) op
+                    tree = result;
+                    oper = tree->OperGet();
+                    typ  = tree->TypeGet();
+                    op1  = tree->AsOp()->gtGetOp1();
+                    op2  = nullptr;
+                }
+            }
+
             if (varTypeIsIntegralOrI(tree->TypeGet()) && tree->OperIs(GT_ADD, GT_MUL, GT_AND, GT_OR, GT_XOR))
             {
                 GenTree* foldedTree = fgMorphCommutative(tree->AsOp());

src/tests/JIT/CodeGenBringUpTests/CastThenBinop.cs

@@ -0,0 +1,83 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+//
+using System;
+using System.Runtime.CompilerServices;
+
+// Test for https://github.com/dotnet/runtime/issues/13816
+public class Test
+{
+    [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)]
+    static int DowncastOr(int a, int b)
+    {
+        return (byte)a | (byte)b;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)]
+    static long UpcastAnd(int a, int b)
+    {
+        return (long)a & (long)b;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)]
+    static long UpcastAnd_ComplexExpression(int a, int b)
+    {
+        return (long)(a - 2) & (long)(b + 1);
+    }
+    [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)]
+    static long UpcastAnd_SideEffect(int a, int b, out int a1, out int b1)
+    {
+        return (long)(a1 = a) & (long)(b1 = b);
+    }
+    [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)]
+    static int DowncastAnd_Overflow(int a, int b)
+    {
+        checked
+        {
+            return (byte)a & (byte)b;
+        }
+    }
+
+    public static int Main()
+    {
+        const int Pass = 100;
+        const int Fail = -1;
+
+        if (DowncastOr(0x0F, 0xF0) != 0xFF)
+        {
+            return Fail;
+        }
+        if (UpcastAnd(0x0FF, 0xFF0) != 0xF0)
+        {
+            return Fail;
+        }
+
+        try
+        {
+            DowncastAnd_Overflow(0x100, 0xFF);
+            // should throw
+            return Fail;
+        }
+        catch (OverflowException)
+        {
+            // expected
+        }
+
+        {
+            var result = UpcastAnd_ComplexExpression(0x0FF, 0xFF0);
+            if (result != 0xF1)
+            {
+                return Fail;
+            }
+        }
+        {
+            var result = UpcastAnd_SideEffect(0x0FF, 0xFF0, out var out1, out var out2);
+            if (result != 0xF0 || out1 != 0x0FF || out2 != 0xFF0)
+            {
+                return Fail;
+            }
+        }
+
+        return Pass;
+    }
+}

src/tests/JIT/CodeGenBringUpTests/CastThenBinop.csproj

@@ -0,0 +1,10 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <DebugType>None</DebugType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="CastThenBinop.cs" />
+  </ItemGroup>
+</Project>