Skip to content

Commit a375009

Browse files
authored
[JIT] Always transform AND(X, CNS(-1)) to X (#82276)
* Always skip emitting 'and reg0, -1' on x86 * Formatting * Wrapping AreUpper32BitsZero and AreUpper32BitsSignExtended to be 64-bit only * Added TryLowerAndNegativeOne * minor tweak * Update lowerxarch.cpp * Remove peephole in favor of lowering opt * Feedback and moved 'TryLowerAndNegativeOne' to lower.cpp
1 parent b04d321 commit a375009

File tree

5 files changed

+61
-15
lines changed

5 files changed

+61
-15
lines changed

src/coreclr/jit/codegenxarch.cpp

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -998,17 +998,6 @@ void CodeGen::genCodeForBinary(GenTreeOp* treeNode)
998998
src = op2;
999999
}
10001000

1001-
// We can skip emitting 'and reg0, -1` if we know that the upper 32bits of 'reg0' are zero'ed.
1002-
if (compiler->opts.OptimizationEnabled())
1003-
{
1004-
if ((oper == GT_AND) && (targetType == TYP_INT) && !treeNode->gtSetFlags() && op2->IsIntegralConst(-1) &&
1005-
emit->AreUpper32BitsZero(targetReg))
1006-
{
1007-
genProduceReg(treeNode);
1008-
return;
1009-
}
1010-
}
1011-
10121001
// try to use an inc or dec
10131002
if (oper == GT_ADD && !varTypeIsFloating(treeNode) && src->isContainedIntOrIImmed() && !treeNode->gtOverflowEx())
10141003
{

src/coreclr/jit/emitxarch.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -476,6 +476,7 @@ bool emitter::IsFlagsAlwaysModified(instrDesc* id)
476476
return true;
477477
}
478478

479+
#ifdef TARGET_64BIT
479480
//------------------------------------------------------------------------
480481
// AreUpper32BitsZero: check if some previously emitted
481482
// instruction set the upper 32 bits of reg to zero.
@@ -594,12 +595,10 @@ bool emitter::AreUpper32BitsZero(regNumber reg)
594595
return false;
595596
}
596597

597-
#ifdef TARGET_AMD64
598598
if (id->idIns() == INS_movsxd)
599599
{
600600
return false;
601601
}
602-
#endif
603602

604603
// movzx always zeroes the upper 32 bits.
605604
if (id->idIns() == INS_movzx)
@@ -655,16 +654,15 @@ bool emitter::AreUpper32BitsSignExtended(regNumber reg)
655654
return true;
656655
}
657656

658-
#ifdef TARGET_AMD64
659657
// movsxd is always an 8 byte operation. W-bit is set.
660658
if (id->idIns() == INS_movsxd)
661659
{
662660
return true;
663661
}
664-
#endif
665662

666663
return false;
667664
}
665+
#endif // TARGET_64BIT
668666

669667
//------------------------------------------------------------------------
670668
// AreFlagsSetToZeroCmp: Checks if the previous instruction set the SZ, and optionally OC, flags to

src/coreclr/jit/emitxarch.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,8 +127,10 @@ bool IsRedundantStackMov(instruction ins, insFormat fmt, emitAttr size, regNumbe
127127
static bool IsJccInstruction(instruction ins);
128128
static bool IsJmpInstruction(instruction ins);
129129

130+
#ifdef TARGET_64BIT
130131
bool AreUpper32BitsZero(regNumber reg);
131132
bool AreUpper32BitsSignExtended(regNumber reg);
133+
#endif // TARGET_64BIT
132134

133135
bool AreFlagsSetToZeroCmp(regNumber reg, emitAttr opSize, GenCondition cond);
134136
bool AreFlagsSetForSignJumpOpt(regNumber reg, emitAttr opSize, GenCondition cond);

src/coreclr/jit/lower.cpp

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,7 +366,18 @@ GenTree* Lowering::LowerNode(GenTree* node)
366366
case GT_AND:
367367
case GT_OR:
368368
case GT_XOR:
369+
{
370+
if (comp->opts.OptimizationEnabled() && node->OperIs(GT_AND))
371+
{
372+
GenTree* replacementNode = TryLowerAndNegativeOne(node->AsOp());
373+
if (replacementNode != nullptr)
374+
{
375+
return replacementNode->gtNext;
376+
}
377+
}
378+
369379
return LowerBinaryArithmetic(node->AsOp());
380+
}
370381

371382
case GT_MUL:
372383
case GT_MULHI:
@@ -7784,6 +7795,51 @@ void Lowering::TryRetypingFloatingPointStoreToIntegerStore(GenTree* store)
77847795
}
77857796
}
77867797

7798+
//----------------------------------------------------------------------------------------------
7799+
// Lowering::TryLowerAndNegativeOne:
7800+
// If safe, lowers a tree AND(X, CNS(-1)) to X.
7801+
//
7802+
// Arguments:
7803+
// node - GT_AND node of integral type
7804+
//
7805+
// Return Value:
7806+
// Returns the replacement node if one is created else nullptr indicating no replacement
7807+
GenTree* Lowering::TryLowerAndNegativeOne(GenTreeOp* node)
7808+
{
7809+
assert(node->OperIs(GT_AND));
7810+
7811+
if (!varTypeIsIntegral(node))
7812+
return nullptr;
7813+
7814+
if (node->gtSetFlags())
7815+
return nullptr;
7816+
7817+
if (node->isContained())
7818+
return nullptr;
7819+
7820+
GenTree* op2 = node->gtGetOp2();
7821+
7822+
if (!op2->IsIntegralConst(-1))
7823+
return nullptr;
7824+
7825+
#ifndef TARGET_64BIT
7826+
assert(op2->TypeIs(TYP_INT));
7827+
#endif // !TARGET_64BIT
7828+
7829+
LIR::Use use;
7830+
if (!BlockRange().TryGetUse(node, &use))
7831+
return nullptr;
7832+
7833+
GenTree* op1 = node->gtGetOp1();
7834+
7835+
use.ReplaceWith(op1);
7836+
7837+
BlockRange().Remove(op2);
7838+
BlockRange().Remove(node);
7839+
7840+
return op1;
7841+
}
7842+
77877843
#if defined(FEATURE_HW_INTRINSICS)
77887844
//----------------------------------------------------------------------------------------------
77897845
// Lowering::InsertNewSimdCreateScalarUnsafeNode: Inserts a new simd CreateScalarUnsafe node

src/coreclr/jit/lower.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,7 @@ class Lowering final : public Phase
307307
void LowerStoreIndir(GenTreeStoreInd* node);
308308
GenTree* LowerAdd(GenTreeOp* node);
309309
GenTree* LowerMul(GenTreeOp* mul);
310+
GenTree* TryLowerAndNegativeOne(GenTreeOp* node);
310311
GenTree* LowerBinaryArithmetic(GenTreeOp* binOp);
311312
bool LowerUnsignedDivOrMod(GenTreeOp* divMod);
312313
GenTree* LowerConstIntDivOrMod(GenTree* node);

0 commit comments

Comments
 (0)