JIT: Transform '(cmp & x) | (cmp & y)' to 'cmp & (x | y)'

src/coreclr/jit/compiler.h

@@ -6666,6 +6666,7 @@ class Compiler
     GenTree* fgOptimizeAddition(GenTreeOp* add);
     GenTree* fgOptimizeMultiply(GenTreeOp* mul);
     GenTree* fgOptimizeBitwiseAnd(GenTreeOp* andOp);
+    GenTree* fgOptimizeBitwiseOr(GenTreeOp* orOp);
     GenTree* fgOptimizeBitwiseXor(GenTreeOp* xorOp);
     GenTree* fgPropagateCommaThrow(GenTree* parent, GenTreeOp* commaThrow, GenTreeFlags precedingSideEffects);
     GenTree* fgMorphRetInd(GenTreeOp* tree);

src/coreclr/jit/gentree.cpp

@@ -2592,7 +2592,7 @@ void CallArgs::ResetFinalArgsAndABIInfo()
  *  Returns non-zero if the two trees are identical.
  */
 
-bool GenTree::Compare(GenTree* op1, GenTree* op2, bool swapOK)
+bool GenTree::Compare(GenTree* op1, GenTree* op2, bool swapOK, bool ignoreIndFlags)
 {
     genTreeOps oper;
     unsigned   kind;
@@ -2773,9 +2773,12 @@ bool GenTree::Compare(GenTree* op1, GenTree* op2, bool swapOK)
                 return false;
             }
 
-            if ((op1->gtFlags & GTF_IND_FLAGS) != (op2->gtFlags & GTF_IND_FLAGS))
+            if (!ignoreIndFlags)
             {
-                return false;
+                if ((op1->gtFlags & GTF_IND_FLAGS) != (op2->gtFlags & GTF_IND_FLAGS))
+                {
+                    return false;
+                }
             }
         }
 
@@ -2915,14 +2918,14 @@ bool GenTree::Compare(GenTree* op1, GenTree* op2, bool swapOK)
 
         if (op1->AsOp()->gtOp2)
         {
-            if (!Compare(op1->AsOp()->gtOp1, op2->AsOp()->gtOp1, swapOK))
+            if (!Compare(op1->AsOp()->gtOp1, op2->AsOp()->gtOp1, swapOK, ignoreIndFlags))
             {
                 if (swapOK && OperIsCommutative(oper) &&
                     ((op1->AsOp()->gtOp1->gtFlags | op1->AsOp()->gtOp2->gtFlags | op2->AsOp()->gtOp1->gtFlags |
                       op2->AsOp()->gtOp2->gtFlags) &
                      GTF_ALL_EFFECT) == 0)
                 {
-                    if (Compare(op1->AsOp()->gtOp1, op2->AsOp()->gtOp2, swapOK))
+                    if (Compare(op1->AsOp()->gtOp1, op2->AsOp()->gtOp2, swapOK, ignoreIndFlags))
                     {
                         op1 = op1->AsOp()->gtOp2;
                         op2 = op2->AsOp()->gtOp1;

src/coreclr/jit/gentree.h

@@ -2079,7 +2079,7 @@ struct GenTree
 
     //---------------------------------------------------------------------
 
-    static bool Compare(GenTree* op1, GenTree* op2, bool swapOK = false);
+    static bool Compare(GenTree* op1, GenTree* op2, bool swapOK = false, bool ignoreIndFlags = false);
 
     //---------------------------------------------------------------------
 

src/coreclr/jit/morph.cpp

@@ -10328,6 +10328,10 @@ GenTree* Compiler::fgOptimizeCommutativeArithmetic(GenTreeOp* tree)
     {
         optimizedTree = fgOptimizeBitwiseAnd(tree);
     }
+    else if (tree->OperIs(GT_OR))
+    {
+        optimizedTree = fgOptimizeBitwiseOr(tree);
+    }
     else if (tree->OperIs(GT_XOR))
     {
         optimizedTree = fgOptimizeBitwiseXor(tree);
@@ -10703,6 +10707,46 @@ GenTree* Compiler::fgOptimizeBitwiseAnd(GenTreeOp* andOp)
     return nullptr;
 }
 
+GenTree* Compiler::fgOptimizeBitwiseOr(GenTreeOp* orOp)
+{
+    assert(orOp->OperIs(GT_OR));
+
+    GenTree* op1 = orOp->gtGetOp1();
+    GenTree* op2 = orOp->gtGetOp2();
+
+    // TODO: We are missing something that reorders operations if it allows the transformation to kick in.
+    // E.g `foo | ((flags & 256) | (flags & 512))` works but `(foo | (flags & 256)) | (flags & 512)` doesnt.
+    // See https://github.com/llvm/llvm-project/blob/c4847d250bf958eaa7415fa7d480d17f8132719b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp#L500
+
+    // Fold "(cmp & x) | (cmp & y)" to "cmp & (x | y)".
+    if (varTypeIsIntegralOrI(orOp) && op1->OperIs(GT_AND) && op2->OperIs(GT_AND))
+    {
+        if (GenTree::Compare(op1->gtGetOp1(), op2->gtGetOp1(), false, true))
+        {
+            // TODO: Add missing volatile check to catch e.g: (flags & 256) | (Volatile.Read(ref flags) & 512).
+            // Volatile INDs are expressed through `GTF_ORDER_SIDEEFF`, but checking for that unnecessarily
+            // pessimizes many other cases so it's not wanted.
+            // See https://discord.com/channels/143867839282020352/312132327348240384/1487216542906061034
+            if ((op2->gtFlags & (GTF_PERSISTENT_SIDE_EFFECTS)) != 0)
+            {
+                return nullptr;
+            }
+
+            orOp->ChangeOper(GT_AND, GenTree::ValueNumberUpdate::PRESERVE_VN);
+            orOp->AsOp()->gtOp1 = op1->gtGetOp1();
+            orOp->AsOp()->gtOp2 = gtNewOperNode(GT_OR, orOp->TypeGet(), op1->gtGetOp2(), op2->gtGetOp2());
+            fgMorphTreeDone(orOp->AsOp()->gtOp2);
+
+            DEBUG_DESTROY_NODE(op1);
+            DEBUG_DESTROY_NODE(op2);
+
+            return orOp;
+        }
+    }
+
+    return nullptr;
+}
+
 //------------------------------------------------------------------------
 // fgOptimizeRelationalComparisonWithCasts: Recognizes comparisons against
 //   various cast operands and tries to remove them. E.g.: