Skip to content

Commit

Permalink
Improve morphing of comparisons (#54204)
Browse files Browse the repository at this point in the history 
* Add SetIntegralValue helper to GenTreeIntCommon

* Fix a potential CSE issue

* Enable comparisons against GT_CNS_LNG

* Optimize unsigned compares with int/long.MaxValue

Replace LE_UN/GT_UN(OP, int/long.MaxValue) with GE/LT(OP, 0)
as that generates smaller code on all targets.

* Relocate fgOptimizeRelationalComparisonWithConst

So that it is next to fgOptimizeEqualityComparisonWithConst.

Purely cosmetic change.

* Use the fgUpdateConstTreeValueNumber helper

* Fix formatting...
  • Loading branch information
@SingleAccretion
SingleAccretion authored Sep 16, 2021
1 parent 4af0126 commit 96cb482
Show file tree
Hide file tree
Showing 3 changed files with 123 additions and 67 deletions.
1 change: 1 addition & 0 deletions src/coreclr/jit/compiler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6192,6 +6192,7 @@ class Compiler
GenTree* fgMorphForRegisterFP(GenTree* tree);
GenTree* fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac = nullptr);
GenTree* fgOptimizeEqualityComparisonWithConst(GenTreeOp* cmp);
GenTree* fgOptimizeRelationalComparisonWithConst(GenTreeOp* cmp);
GenTree* fgMorphRetInd(GenTreeUnOp* tree);
GenTree* fgMorphModToSubMulDiv(GenTreeOp* tree);
GenTree* fgMorphSmpOpOptional(GenTreeOp* tree);
Expand Down
18 changes: 18 additions & 0 deletions src/coreclr/jit/gentree.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3053,6 +3053,7 @@ struct GenTreeIntConCommon : public GenTree
inline ssize_t IconValue() const;
inline void SetIconValue(ssize_t val);
inline INT64 IntegralValue() const;
inline void SetIntegralValue(int64_t value);

template <typename T>
inline void SetValueTruncating(T value);
Expand Down Expand Up @@ -3250,6 +3251,23 @@ inline INT64 GenTreeIntConCommon::IntegralValue() const
#endif // TARGET_64BIT
}

inline void GenTreeIntConCommon::SetIntegralValue(int64_t value)
{
#ifdef TARGET_64BIT
SetIconValue(value);
#else
if (OperIs(GT_CNS_LNG))
{
SetLngValue(value);
}
else
{
assert(FitsIn<int32_t>(value));
SetIconValue(static_cast<int32_t>(value));
}
#endif // TARGET_64BIT
}

//------------------------------------------------------------------------
// SetValueTruncating: Set the value, truncating to TYP_INT if necessary.
//
Expand Down
171 changes: 104 additions & 67 deletions src/coreclr/jit/morph.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12191,75 +12191,14 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac)
case GT_GE:
case GT_GT:

if (op2->gtOper == GT_CNS_INT)
// op2's value may be changed, so it cannot be a CSE candidate.
if (op2->IsIntegralConst() && !gtIsActiveCSE_Candidate(op2))
{
cns2 = op2;
/* Check for "expr relop 1" */
if (cns2->IsIntegralConst(1))
{
/* Check for "expr >= 1" */
if (oper == GT_GE)
{
/* Change to "expr != 0" for unsigned and "expr > 0" for signed */
oper = (tree->IsUnsigned()) ? GT_NE : GT_GT;
goto SET_OPER;
}
/* Check for "expr < 1" */
else if (oper == GT_LT)
{
/* Change to "expr == 0" for unsigned and "expr <= 0" for signed */
oper = (tree->IsUnsigned()) ? GT_EQ : GT_LE;
goto SET_OPER;
}
}
/* Check for "expr relop -1" */
else if (!tree->IsUnsigned() && cns2->IsIntegralConst(-1))
{
/* Check for "expr <= -1" */
if (oper == GT_LE)
{
/* Change to "expr < 0" */
oper = GT_LT;
goto SET_OPER;
}
/* Check for "expr > -1" */
else if (oper == GT_GT)
{
/* Change to "expr >= 0" */
oper = GT_GE;

SET_OPER:
// IF we get here we should be changing 'oper'
assert(tree->OperGet() != oper);

// Keep the old ValueNumber for 'tree' as the new expr
// will still compute the same value as before
tree->SetOper(oper, GenTree::PRESERVE_VN);
cns2->AsIntCon()->gtIconVal = 0;
tree = fgOptimizeRelationalComparisonWithConst(tree->AsOp());
oper = tree->OperGet();

// vnStore is null before the ValueNumber phase has run
if (vnStore != nullptr)
{
// Update the ValueNumber for 'cns2', as we just changed it to 0
fgValueNumberTreeConst(cns2);
}
op2 = tree->AsOp()->gtOp2 = gtFoldExpr(op2);
}
}
else if (tree->IsUnsigned() && op2->IsIntegralConst(0))
{
if ((oper == GT_GT) || (oper == GT_LE))
{
// IL doesn't have a cne instruction so compilers use cgt.un instead. The JIT
// recognizes certain patterns that involve GT_NE (e.g (x & 4) != 0) and fails
// if GT_GT is used instead. Transform (x GT_GT.unsigned 0) into (x GT_NE 0)
// and (x GT_LE.unsigned 0) into (x GT_EQ 0). The later case is rare, it sometimes
// occurs as a result of branch inversion.
oper = (oper == GT_LE) ? GT_EQ : GT_NE;
tree->SetOper(oper, GenTree::PRESERVE_VN);
tree->gtFlags &= ~GTF_UNSIGNED;
}
}
assert(op1 == tree->AsOp()->gtGetOp1());
assert(op2 == tree->AsOp()->gtGetOp2());
}

COMPARE:
Expand Down Expand Up @@ -13844,6 +13783,104 @@ GenTree* Compiler::fgOptimizeEqualityComparisonWithConst(GenTreeOp* cmp)
return cmp;
}

//------------------------------------------------------------------------
// fgOptimizeRelationalComparisonWithConst: optimizes a comparison operation.
//
// Recognizes comparisons against various constant operands and morphs
// them, if possible, into comparisons against zero.
//
// Arguments:
// cmp - the GT_LE/GT_LT/GT_GE/GT_GT tree to morph.
//
// Return Value:
// The "cmp" tree, possibly with a modified oper.
// The second operand's constant value may be modified as well.
//
// Assumptions:
// The operands have been swapped so that any constants are on the right.
// The second operand is an integral constant.
//
GenTree* Compiler::fgOptimizeRelationalComparisonWithConst(GenTreeOp* cmp)
{
assert(cmp->OperIs(GT_LE, GT_LT, GT_GE, GT_GT));
assert(cmp->gtGetOp2()->IsIntegralConst());
assert(!gtIsActiveCSE_Candidate(cmp->gtGetOp2()));

GenTree* op1 = cmp->gtGetOp1();
GenTreeIntConCommon* op2 = cmp->gtGetOp2()->AsIntConCommon();

assert(genActualType(op1) == genActualType(op2));

genTreeOps oper = cmp->OperGet();
int64_t op2Value = op2->IntegralValue();

if (op2Value == 1)
{
// Check for "expr >= 1".
if (oper == GT_GE)
{
// Change to "expr != 0" for unsigned and "expr > 0" for signed.
oper = cmp->IsUnsigned() ? GT_NE : GT_GT;
}
// Check for "expr < 1".
else if (oper == GT_LT)
{
// Change to "expr == 0" for unsigned and "expr <= 0".
oper = cmp->IsUnsigned() ? GT_EQ : GT_LE;
}
}
// Check for "expr relop -1".
else if (!cmp->IsUnsigned() && (op2Value == -1))
{
// Check for "expr <= -1".
if (oper == GT_LE)
{
// Change to "expr < 0".
oper = GT_LT;
}
// Check for "expr > -1".
else if (oper == GT_GT)
{
// Change to "expr >= 0".
oper = GT_GE;
}
}
else if (cmp->IsUnsigned())
{
if ((oper == GT_LE) || (oper == GT_GT))
{
if (op2Value == 0)
{
// IL doesn't have a cne instruction so compilers use cgt.un instead. The JIT
// recognizes certain patterns that involve GT_NE (e.g (x & 4) != 0) and fails
// if GT_GT is used instead. Transform (x GT_GT.unsigned 0) into (x GT_NE 0)
// and (x GT_LE.unsigned 0) into (x GT_EQ 0). The later case is rare, it sometimes
// occurs as a result of branch inversion.
oper = (oper == GT_LE) ? GT_EQ : GT_NE;
cmp->gtFlags &= ~GTF_UNSIGNED;
}
// LE_UN/GT_UN(expr, int/long.MaxValue) => GE/LT(expr, 0).
else if (((op1->TypeIs(TYP_LONG) && (op2Value == INT64_MAX))) ||
((genActualType(op1) == TYP_INT) && (op2Value == INT32_MAX)))
{
oper = (oper == GT_LE) ? GT_GE : GT_LT;
cmp->gtFlags &= ~GTF_UNSIGNED;
}
}
}

if (!cmp->OperIs(oper))
{
// Keep the old ValueNumber for 'tree' as the new expr
// will still compute the same value as before.
cmp->SetOper(oper, GenTree::PRESERVE_VN);
op2->SetIntegralValue(0);
fgUpdateConstTreeValueNumber(op2);
}

return cmp;
}

//----------------------------------------------------------------------------------------------
// fgMorphRetInd: Try to get rid of extra IND(ADDR()) pairs in a return tree.
//
Expand Down

0 comments on commit 96cb482

Please sign in to comment.