-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Changes from 5 commits
8760304
257e1c6
87ee2ec
03f5404
6d1279b
3708cd3
3ed0ac5
13a7e54
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10313,6 +10313,21 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA | |
|
||
#endif // defined(TARGET_ARM64) || defined(TARGET_LOONGARCH64) | ||
|
||
#if defined(TARGET_XARCH) | ||
case GT_DIV: | ||
case GT_UDIV: | ||
if (opts.OptimizationEnabled() && fgGlobalMorph) | ||
{ | ||
GenTree* optimizedTree = fgOptimizeDivision(tree->AsOp()); | ||
if (optimizedTree != nullptr) | ||
{ | ||
tree = optimizedTree; | ||
if (tree->IsIntegralConst()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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: | ||
|
@@ -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()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. potential division by zero in jit code if |
||
{ | ||
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. | ||
// | ||
|
There was a problem hiding this comment.
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?