From 1c6892fbb013a95bf4f1c86c75dde32e23a5630a Mon Sep 17 00:00:00 2001 From: BoyBaykiller Date: Wed, 25 Mar 2026 03:08:19 +0100 Subject: [PATCH 1/3] * Transform '(cmp & x) | (cmp & y)' to 'cmp & (x | y)' --- src/coreclr/jit/compiler.h | 1 + src/coreclr/jit/morph.cpp | 31 +++++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index d9e256c164e345..c27922b307fd6e 100644 --- a/src/coreclr/jit/compiler.h +++ b/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); diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 9456da4e99c5fb..ed928add95f531 100644 --- a/src/coreclr/jit/morph.cpp +++ b/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,33 @@ 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(); + + // Fold "(cmp & x) | (cmp & y)" to "cmp & (x | y)". + if (orOp->TypeIs(TYP_INT) && op1->OperIs(GT_AND) && op2->OperIs(GT_AND)) + { + if (GenTree::Compare(op1->gtGetOp1(), op2->gtGetOp1())) + { + 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.: From 5ab2bef0f445322ffeea8581d427c8b936742c32 Mon Sep 17 00:00:00 2001 From: BoyBaykiller Date: Wed, 25 Mar 2026 03:40:04 +0100 Subject: [PATCH 2/3] * allow TYP_LONG --- src/coreclr/jit/morph.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index ed928add95f531..7325897da2751c 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -10715,7 +10715,7 @@ GenTree* Compiler::fgOptimizeBitwiseOr(GenTreeOp* orOp) GenTree* op2 = orOp->gtGetOp2(); // Fold "(cmp & x) | (cmp & y)" to "cmp & (x | y)". - if (orOp->TypeIs(TYP_INT) && op1->OperIs(GT_AND) && op2->OperIs(GT_AND)) + if (varTypeIsIntegralOrI(orOp) && op1->OperIs(GT_AND) && op2->OperIs(GT_AND)) { if (GenTree::Compare(op1->gtGetOp1(), op2->gtGetOp1())) { From 205c820fb048066d4f64ac5996d9ecdd7260c864 Mon Sep 17 00:00:00 2001 From: BoyBaykiller Date: Sat, 28 Mar 2026 00:51:51 +0100 Subject: [PATCH 3/3] * add option to ingore IND flags in GenTree::Compare * add check for GTF_PERSISTENT_SIDE_EFFECTS so we dont remove a tree when we shouldnt (volatile check still missing) --- src/coreclr/jit/gentree.cpp | 13 ++++++++----- src/coreclr/jit/gentree.h | 2 +- src/coreclr/jit/morph.cpp | 15 ++++++++++++++- 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 3dfca0ba725a0a..a21ab89df1bf17 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/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; diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index ed72dd9f8c87a5..c85a2857874970 100644 --- a/src/coreclr/jit/gentree.h +++ b/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); //--------------------------------------------------------------------- diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 7325897da2751c..07fda4bb93cd73 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -10714,11 +10714,24 @@ GenTree* Compiler::fgOptimizeBitwiseOr(GenTreeOp* orOp) 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())) + 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());