Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimizing a series of divisions (#74020) #74314

Closed
wants to merge 8 commits into from
1 change: 1 addition & 0 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -5798,6 +5798,7 @@ class Compiler
GenTree* fgOptimizeCommutativeArithmetic(GenTreeOp* tree);
GenTree* fgOptimizeRelationalComparisonWithCasts(GenTreeOp* cmp);
GenTree* fgOptimizeAddition(GenTreeOp* add);
GenTree* fgOptimizeDivision(GenTreeOp* div);
GenTree* fgOptimizeMultiply(GenTreeOp* mul);
GenTree* fgOptimizeBitwiseAnd(GenTreeOp* andOp);
GenTree* fgOptimizeBitwiseXor(GenTreeOp* xorOp);
Expand Down
77 changes: 77 additions & 0 deletions src/coreclr/jit/morph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10313,6 +10313,21 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA

#endif // defined(TARGET_ARM64) || defined(TARGET_LOONGARCH64)

#if defined(TARGET_XARCH)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why it's limited to xarch only?

case GT_DIV:
case GT_UDIV:
if (opts.OptimizationEnabled() && fgGlobalMorph)
{
GenTree* optimizedTree = fgOptimizeDivision(tree->AsOp());
if (optimizedTree != nullptr)
{
tree = optimizedTree;
if (tree->IsIntegralConst())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it doesn't look like this condition can possibly be true?

return tree;
}
}
break;
#endif
case GT_ADD:

CM_OVF_OP:
Expand Down Expand Up @@ -11565,6 +11580,68 @@ GenTree* Compiler::fgOptimizeHWIntrinsic(GenTreeHWIntrinsic* node)

#endif

//------------------------------------------------------------------------
// fgOptimizeDivision: Optimizes division operations.
//
// Arguments:
// tree - the unchecked GT_DIV/GT_UDIV tree to optimize.
//
// Return Value:
// The optimized tree that can have any shape.
//
GenTree* Compiler::fgOptimizeDivision(GenTreeOp* div)
{
assert(div->OperIs(GT_DIV, GT_UDIV));

GenTree* op1 = div->gtGetOp1();
GenTree* op2 = div->gtGetOp2();

// Folds cascade of DIVs/UDIVs into one DIV/UDIV and a multiplication
// of root and child values
if (op1->OperIs(GT_UDIV, GT_DIV) && op2->IsIntegralConst() && op1->gtGetOp2()->IsIntegralConst())
{

GenTree* chOp1 = op1->gtGetOp1();
GenTree* chOp2 = op1->gtGetOp2();

IntegralRange boundRange = IntegralRange::ForNode(op2, this);
int64_t upperBound = IntegralRange::SymbolicToRealValue(boundRange.GetUpperBound());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not clear to me why IntegralRange is used here - you already have child_val and root_val, don't you?


int64_t child_val = chOp2->AsIntConCommon()->IntegralValue();
int64_t root_val = op2->AsIntConCommon()->IntegralValue();

// Make sure it is not folded into overflow
// If it is going to overflow, then result of such a division
// is 0
if ((upperBound / child_val) < root_val)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

potential division by zero in jit code if child_val is 0

{
if (gtTreeHasSideEffects(op1, GTF_SIDE_EFFECT))
{
return nullptr;
}

GenTree* ret = gtNewZeroConNode(TYP_INT);

DEBUG_DESTROY_NODE(div);

INDEBUG(ret->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED);

return ret;
}

div->gtOp1 = chOp1;
div->gtOp2 = chOp2;
div->gtOp2->AsIntConCommon()->SetIntegralValue(root_val * child_val);

DEBUG_DESTROY_NODE(op1);
DEBUG_DESTROY_NODE(op2);

return div;
}

return nullptr;
}

//------------------------------------------------------------------------
// fgOptimizeCommutativeArithmetic: Optimizes commutative operations.
//
Expand Down