From 7ebe63677bb1fab80120332eec11822adb83cd98 Mon Sep 17 00:00:00 2001 From: Henry Zongaro Date: Tue, 13 Jun 2023 07:27:22 -0700 Subject: [PATCH] Ensure DIVCHK and division/remainder simplifiers uphold contract The divchkSimplifier was first simplifying its child node and then checking whether the DIVCHK is still needed after having simplified its child. It did that by checking whether the simplification of its child results in a node that is not the original child, or the simplified child is not a division or remainder operation. However, it might be that the simplified child is still a division or remainder operation that needs to be checked for division by zero, but is not the original child node. That leads divchkSimplifier to replace the DIVCHK with a treetop operation incorrectly. This change introduces a _nodeToDivchk field in OMR::Simplifier that the integer division and remainder simplifiers use to indicate whether the result of attempting the simplification results in a node that would still need to be checked for division by zero if the original node needed to be checked for division by zero. The divchkSimplifier, after simplifying its child division or remainder operation, checks _nodeToDivchk as a first step in determining whether the DIVCHK is still needed. This change also updates the fold*Constant methods to return true to indicate that they were able to perform the requested transformations, which are guarded by calls to performTransformation, or return false if they were unable to. --- compiler/optimizer/OMRSimplifier.cpp | 2 + compiler/optimizer/OMRSimplifier.hpp | 9 + compiler/optimizer/OMRSimplifierHandlers.cpp | 429 +++++++++++++++---- compiler/optimizer/OMRSimplifierHelpers.cpp | 55 ++- compiler/optimizer/OMRSimplifierHelpers.hpp | 18 +- 5 files changed, 412 insertions(+), 101 deletions(-) diff --git a/compiler/optimizer/OMRSimplifier.cpp b/compiler/optimizer/OMRSimplifier.cpp index 980d5014944..b852cd3aa3e 100644 --- a/compiler/optimizer/OMRSimplifier.cpp +++ b/compiler/optimizer/OMRSimplifier.cpp @@ -167,6 +167,8 @@ OMR::Simplifier::Simplifier(TR::OptimizationManager *manager) _reassociate = comp()->getOption(TR_EnableReassociation); _containingStructure = NULL; + + _nodeToDivchk = NULL; } TR::Optimization *OMR::Simplifier::create(TR::OptimizationManager *manager) diff --git a/compiler/optimizer/OMRSimplifier.hpp b/compiler/optimizer/OMRSimplifier.hpp index bc21ab03797..805db19fdf8 100644 --- a/compiler/optimizer/OMRSimplifier.hpp +++ b/compiler/optimizer/OMRSimplifier.hpp @@ -292,6 +292,15 @@ class Simplifier : public TR::Optimization TR::TreeTop *_performLowerTreeSimplifier; TR::Node *_performLowerTreeNode; TR::list > _performLowerTreeNodePairs; + + /** + * This field is used as part of a handshake between the \ref divchkSimplifier + * and the integer division and remainder simplifiers. If simplifying the + * child of a \c DIVCHK leaves us with a node that still must have a \c DIVCHK + * applied, the division or remainder simplifier will set this field to refer + * to that node; otherwise, it will set this field is set to \c NULL. + */ + TR::Node *_nodeToDivchk; }; } diff --git a/compiler/optimizer/OMRSimplifierHandlers.cpp b/compiler/optimizer/OMRSimplifierHandlers.cpp index 711e17aa6e2..dffd427ed8b 100644 --- a/compiler/optimizer/OMRSimplifierHandlers.cpp +++ b/compiler/optimizer/OMRSimplifierHandlers.cpp @@ -8568,6 +8568,13 @@ TR::Node *idivSimplifier(TR::Node * node, TR::Block * block, TR::Simplifier * s) { s->simplifyChildren(node, block); + // Handshake with divchkSimplifier: If simplifying the child of a DIVCHK + // results in a node that still needs to have a DIVCHK applied, place that + // node in _nodeToDivchk. If the simplification leaves no node that needs + // to have a DIVCHK applied, set _nodeToDivchk to NULL. + // + s->_nodeToDivchk = node; + if (node->getOpCodeValue() == TR::iudiv) { if (!node->getChild(0)->isNonNegative()) @@ -8619,17 +8626,53 @@ TR::Node *idivSimplifier(TR::Node * node, TR::Block * block, TR::Simplifier * s) uquotient = dividend / divisor; } } - foldUIntConstant(node, uquotient, s, false /* !anchorChildren*/); + + if (foldUIntConstant(node, uquotient, s, false /* !anchorChildren*/)) + { + // If folding to a constant actually happened, any parent DIVCHK is no longer needed + s->_nodeToDivchk = NULL; + } } else { if (divisor == -1 && dividend == TR::getMinSigned()) - return s->replaceNode(node, firstChild, s->_curTree); - foldIntConstant(node, dividend/divisor, s, false /* !anchorChildren*/); + { + TR::Node *replacementNode = s->replaceNode(node, firstChild, s->_curTree); + + // Verify that replaceNode has actually replaced the idiv with its first + // child - replacement is guarded by performTransformation. If replacement + // happened, any parent DIVCHK is no longer needed. + // + if (replacementNode == firstChild) + { + s->_nodeToDivchk = NULL; + } + + return replacementNode; + } + + if (foldIntConstant(node, dividend/divisor, s, false /* !anchorChildren*/)) + { + // If folding to a constant actually happened, any parent DIVCHK is no longer needed + s->_nodeToDivchk = NULL; + } } } // first child is constant else if (divisor == 1) - return s->replaceNode(node, firstChild, s->_curTree); + { + TR::Node *replacementNode = s->replaceNode(node, firstChild, s->_curTree, block); + + // Verify that replaceNode has actually replaced the idiv with its first + // child - replacement is guarded by performTransformation. If replacement + // happened, any parent DIVCHK is no longer needed. + // + if (replacementNode == firstChild) + { + s->_nodeToDivchk = NULL; + } + + return replacementNode; + } else if (!secondChild->getOpCode().isUnsigned() && divisor == -1) { if (performTransformation(s->comp(), "%sReduced idiv by -1 with ineg in node [%s]\n", s->optDetailString(), node->getName(s->getDebug()))) @@ -8641,6 +8684,9 @@ TR::Node *idivSimplifier(TR::Node * node, TR::Block * block, TR::Simplifier * s) TR::Node::recreate(node, TR::ineg); node->setChild(0, firstChild); node->setNumChildren(1); + + // Division has been removed, so any parent DIVCHK is no longer needed + s->_nodeToDivchk = NULL; } } // Design 1790 @@ -8689,6 +8735,9 @@ TR::Node *idivSimplifier(TR::Node * node, TR::Block * block, TR::Simplifier * s) node->setFirst(newNode4); } node->getFirstChild()->incReferenceCount(); + + // Division has been removed, so any parent DIVCHK is no longer needed + s->_nodeToDivchk = NULL; } else if (s->cg()->getSupportsLoweringConstIDiv() && !isPowerOf2(divisor) && performTransformation(s->comp(), "%sMagic number idiv opt in node %p\n", s->optDetailString(), node)) @@ -8772,6 +8821,9 @@ TR::Node *idivSimplifier(TR::Node * node, TR::Block * block, TR::Simplifier * s) node->setSecond(quotient->getSecondChild()); node->setNumChildren(2); #endif // Disabled pending approval of 1055. + + // Division has been removed, so any parent DIVCHK is no longer needed + s->_nodeToDivchk = NULL; } } // non-zero constant } // constant @@ -8782,6 +8834,13 @@ TR::Node *ldivSimplifier(TR::Node * node, TR::Block * block, TR::Simplifier * s) { s->simplifyChildren(node, block); + // Handshake with divchkSimplifier: If simplifying the child of a DIVCHK + // results in a node that still needs to have a DIVCHK applied, place that + // node in _nodeToDivchk. If the simplification leaves no node that needs + // to have a DIVCHK applied, set _nodeToDivchk to NULL. + // + s->_nodeToDivchk = node; + if (node->getOpCodeValue() == TR::ludiv) { if (!node->getChild(0)->isNonNegative()) @@ -8806,11 +8865,42 @@ TR::Node *ldivSimplifier(TR::Node * node, TR::Block * block, TR::Simplifier * s) { int64_t dividend = firstChild->getLongInt(); if (divisor == -1 && dividend == TR::getMinSigned()) - return s->replaceNode(node, firstChild, s->_curTree); - foldLongIntConstant(node, dividend / divisor, s, false /* !anchorChildren */); + { + TR::Node *replacementNode = s->replaceNode(node, firstChild, s->_curTree); + + // Verify that replaceNode has actually replaced the ldiv with its first + // child - replacement is guarded by performTransformation. If replacement + // happened, any parent DIVCHK is no longer needed. + // + if (replacementNode == firstChild) + { + s->_nodeToDivchk = NULL; + } + + return replacementNode; + } + + if (foldLongIntConstant(node, dividend / divisor, s, false /* !anchorChildren */)) + { + // If folding to a constant actually happened, any parent DIVCHK is no longer needed + s->_nodeToDivchk = NULL; + } } else if (divisor == 1) - return s->replaceNode(node, firstChild, s->_curTree); + { + TR::Node *replacementNode = s->replaceNode(node, firstChild, s->_curTree, block); + + // Verify that replaceNode has actually replaced the ldiv with its first + // child - replacement is guarded by performTransformation. If replacement + // happened, any parent DIVCHK is no longer needed. + // + if (replacementNode == firstChild) + { + s->_nodeToDivchk = NULL; + } + + return replacementNode; + } else if (divisor == -1) { if (performTransformation(s->comp(), "%sReduced ldiv by -1 with lneg in node [%p]\n", s->optDetailString(), node)) @@ -8822,6 +8912,9 @@ TR::Node *ldivSimplifier(TR::Node * node, TR::Block * block, TR::Simplifier * s) TR::Node::recreate(node, TR::lneg); node->setChild(0, firstChild); node->setNumChildren(1); + + // Division has been removed, so any parent DIVCHK is no longer needed + s->_nodeToDivchk = NULL; } } else if (s->cg()->getSupportsLoweringConstLDivPower2() && isPowerOf2(divisor)) @@ -8850,6 +8943,9 @@ TR::Node *ldivSimplifier(TR::Node * node, TR::Block * block, TR::Simplifier * s) TR::Node::recreate(node, TR::lneg); node->setAndIncChild(0, node1); node->setNumChildren(1); + + // Division has been removed, so any parent DIVCHK is no longer needed + s->_nodeToDivchk = NULL; } } else @@ -8870,6 +8966,9 @@ TR::Node *ldivSimplifier(TR::Node * node, TR::Block * block, TR::Simplifier * s) } secondChild->setInt(shiftAmount); s->_alteredBlock = true; + + // Division has been removed, so any parent DIVCHK is no longer needed + s->_nodeToDivchk = NULL; } } } // end positive dividend @@ -8889,6 +8988,9 @@ TR::Node *ldivSimplifier(TR::Node * node, TR::Block * block, TR::Simplifier * s) node->setAndIncChild(0, node1); node->setAndIncChild(1, shiftNode); node->setNumChildren(2); + + // Division has been removed, so any parent DIVCHK is no longer needed + s->_nodeToDivchk = NULL; } } else if ( divisor >= 0 ) @@ -8906,6 +9008,9 @@ TR::Node *ldivSimplifier(TR::Node * node, TR::Block * block, TR::Simplifier * s) node->setAndIncChild(0, node1); node->setAndIncChild(1, shiftNode); node->setNumChildren(2); + + // Division has been removed, so any parent DIVCHK is no longer needed + s->_nodeToDivchk = NULL; } else { @@ -8931,6 +9036,9 @@ TR::Node *ldivSimplifier(TR::Node * node, TR::Block * block, TR::Simplifier * s) node_const0or1 = TR::Node::create(TR::lneg, 1, node_sign); node->setAndIncChild(1, node_const0or1); node->setNumChildren(2); + + // Division has been removed, so any parent DIVCHK is no longer needed + s->_nodeToDivchk = NULL; } } } @@ -8979,6 +9087,9 @@ TR::Node *ldivSimplifier(TR::Node * node, TR::Block * block, TR::Simplifier * s) node->setFirst(newNode4); } node->getFirstChild()->incReferenceCount(); + + // Division has been removed, so any parent DIVCHK is no longer needed + s->_nodeToDivchk = NULL; } } // end power of 2 else if (s->cg()->getSupportsLoweringConstLDiv() && !isPowerOf2(divisor)) @@ -9037,6 +9148,9 @@ TR::Node *ldivSimplifier(TR::Node * node, TR::Block * block, TR::Simplifier * s) node->setAndIncChild(0, node3); node->setAndIncChild(1, node4); node->setNumChildren(2); + + // Division has been removed, so any parent DIVCHK is no longer needed + s->_nodeToDivchk = NULL; } // divisor not power of 2 } // non-zero divisor } // second child is constant @@ -9051,10 +9165,6 @@ TR::Node *ldivSimplifier(TR::Node * node, TR::Block * block, TR::Simplifier * s) performTransformation(s->comp(), "%sReduced ldiv [%p] of two i2l children to i2l of idiv \n", s->optDetailString(), node)) { TR::TreeTop *curTree = s->_curTree; - TR::Node *divCheckParent = NULL; - if ((curTree->getNode()->getOpCodeValue() == TR::DIVCHK) && - (curTree->getNode()->getFirstChild() == node)) - divCheckParent = curTree->getNode(); TR::Node *divNode = TR::Node::create(TR::idiv, 2, firstChild->getFirstChild(), secondChild->getFirstChild()); @@ -9066,12 +9176,9 @@ TR::Node *ldivSimplifier(TR::Node * node, TR::Block * block, TR::Simplifier * s) node->setAndIncChild(0, divNode); node->setNumChildren(1); - if (divCheckParent) - { - divCheckParent->setAndIncChild(0, divNode); - node->recursivelyDecReferenceCount(); - return divNode; - } + // Division has been transformed, but still needs to be checked for division by zero + // if the current node was the child of a DIVCHK + s->_nodeToDivchk = divNode; } if (secondChild->getOpCode().isLoadConst() && secondChild->getLongInt() == 10 && @@ -9079,23 +9186,14 @@ TR::Node *ldivSimplifier(TR::Node * node, TR::Block * block, TR::Simplifier * s) node->getFirstChild()->getOpCode().isLoadVar() && //transformToLongDiv creates a tonne of nodes. Not good to do this recursively if you have chained divides performTransformation(s->comp(), "%sReduced ldiv by 10 [%p] to bitwise ops\n", s->optDetailString(), node)) { - TR::TreeTop *curTree = s->_curTree; - TR::Node *divCheckParent = NULL; - if ((curTree->getNode()->getOpCodeValue() == TR::DIVCHK) && - (curTree->getNode()->getFirstChild() == node)) - divCheckParent = curTree->getNode(); - transformToLongDivBy10Bitwise(node, node, s); TR::Node::recreate(node, TR::ladd); firstChild->recursivelyDecReferenceCount(); secondChild->recursivelyDecReferenceCount(); - if (divCheckParent) - { - divCheckParent->setAndIncChild(0, node); - node->recursivelyDecReferenceCount(); - return node; - } + // Division has been removed, so any parent DIVCHK is no longer needed + s->_nodeToDivchk = NULL; + return node; } } @@ -9205,11 +9303,22 @@ TR::Node *bdivSimplifier(TR::Node * node, TR::Block * block, TR::Simplifier * s) { s->simplifyChildren(node, block); + // Handshake with divchkSimplifier: If simplifying the child of a DIVCHK + // results in a node that still needs to have a DIVCHK applied, place that + // node in _nodeToDivchk. If the simplification leaves no node that needs + // to have a DIVCHK applied, set _nodeToDivchk to NULL. + s->_nodeToDivchk = node; + TR::Node * firstChild = node->getFirstChild(), * secondChild = node->getSecondChild(); if (firstChild->getOpCode().isLoadConst() && secondChild->getOpCode().isLoadConst()) { - foldByteConstant(node, (int8_t)(firstChild->getByte() / secondChild->getByte()), s, false /* !anchorChildren*/); + if (foldByteConstant(node, (int8_t)(firstChild->getByte() / secondChild->getByte()), s, false /* !anchorChildren*/)) + { + // If folding to a constant actually happened, any parent DIVCHK is no longer needed + s->_nodeToDivchk = NULL; + } + return node; } @@ -9221,11 +9330,22 @@ TR::Node *sdivSimplifier(TR::Node * node, TR::Block * block, TR::Simplifier * s) { s->simplifyChildren(node, block); + // Handshake with divchkSimplifier: If simplifying the child of a DIVCHK + // results in a node that still needs to have a DIVCHK applied, place that + // node in _nodeToDivchk. If the simplification leaves no node that needs + // to have a DIVCHK applied, set _nodeToDivchk to NULL. + s->_nodeToDivchk = node; + TR::Node * firstChild = node->getFirstChild(), * secondChild = node->getSecondChild(); if (firstChild->getOpCode().isLoadConst() && secondChild->getOpCode().isLoadConst()) { - foldShortIntConstant(node, (int16_t)(firstChild->getShortInt() / secondChild->getShortInt()), s, false /* !anchorChildren */); + if (foldShortIntConstant(node, (int16_t)(firstChild->getShortInt() / secondChild->getShortInt()), s, false /* !anchorChildren */)) + { + // If folding to a constant actually happened, any parent DIVCHK is no longer needed + s->_nodeToDivchk = NULL; + } + return node; } @@ -9242,6 +9362,12 @@ TR::Node *iremSimplifier(TR::Node * node, TR::Block * block, TR::Simplifier * s) bool isUnsigned = node->getOpCode().isUnsigned(); s->simplifyChildren(node, block); + // Handshake with divchkSimplifier: If simplifying the child of a DIVCHK + // results in a node that still needs to have a DIVCHK applied, place that + // node in _nodeToDivchk. If the simplification leaves no node that needs + // to have a DIVCHK applied, set _nodeToDivchk to NULL. + s->_nodeToDivchk = node; + TR::Node * firstChild = node->getFirstChild(), * secondChild = node->getSecondChild(); static const char * disableILRemPwr2Opt = feGetEnv("TR_DisableILRemPwr2Opt"); @@ -9258,7 +9384,12 @@ TR::Node *iremSimplifier(TR::Node * node, TR::Block * block, TR::Simplifier * s) { if (divisor == 1 || (!isUnsigned && (divisor == -1))) { - foldIntConstant(node, 0, s, true /* anchorChildren */); + if (foldIntConstant(node, 0, s, true /* anchorChildren */)) + { + // If folding to a constant actually happened, any parent DIVCHK is no longer needed + s->_nodeToDivchk = NULL; + } + return node; } else if (firstChild->getOpCode().isLoadConst()) @@ -9267,7 +9398,12 @@ TR::Node *iremSimplifier(TR::Node * node, TR::Block * block, TR::Simplifier * s) (uint32_t)dividend%(uint32_t)divisor : dividend%divisor; - foldIntConstant(node, result, s, false /* !anchorChildren */); + if (foldIntConstant(node, result, s, false /* !anchorChildren */)) + { + // If folding to a constant actually happened, any parent DIVCHK is no longer needed + s->_nodeToDivchk = NULL; + } + return node; } else if ((!disableILRemPwr2Opt)&& (!isUnsigned || isNonNegativePowerOf2(divisor)) && @@ -9318,6 +9454,10 @@ TR::Node *iremSimplifier(TR::Node * node, TR::Block * block, TR::Simplifier * s) node->setSecond(newNode4); node->getFirstChild()->incReferenceCount(); node->getSecondChild()->incReferenceCount(); + + // Division has been removed, so any parent DIVCHK is no longer needed + s->_nodeToDivchk = NULL; + return node; } } @@ -9361,6 +9501,10 @@ TR::Node *iremSimplifier(TR::Node * node, TR::Block * block, TR::Simplifier * s) node->setAndIncChild(0,firstChild); node->setAndIncChild(1,node1); node->setNumChildren(2); + + // Division has been removed, so any parent DIVCHK is no longer needed + s->_nodeToDivchk = NULL; + return node; } } @@ -9375,6 +9519,13 @@ TR::Node *lremSimplifier(TR::Node * node, TR::Block * block, TR::Simplifier * s) { s->simplifyChildren(node, block); + // Handshake with divchkSimplifier: If simplifying the child of a DIVCHK + // results in a node that still needs to have a DIVCHK applied, place that + // node in _nodeToDivchk. If the simplification leaves no node that needs + // to have a DIVCHK applied, set _nodeToDivchk to NULL. + // + s->_nodeToDivchk = node; + TR::Node * firstChild = node->getFirstChild(), * secondChild = node->getSecondChild(); static const char * disableILRemPwr2Opt = feGetEnv("TR_DisableILRemPwr2Opt"); @@ -9392,16 +9543,29 @@ TR::Node *lremSimplifier(TR::Node * node, TR::Block * block, TR::Simplifier * s) { if (divisor == 1 || (divisor == -1)) { - foldLongIntConstant(node, 0, s, true /* anchorChildren */); + if (foldLongIntConstant(node, 0, s, true /* anchorChildren */)) + { + // If folding to a constant actually happened, any parent DIVCHK is no longer needed + s->_nodeToDivchk = NULL; + } return node; } else if (firstChild->getOpCode().isLoadConst()) { int64_t dividend = firstChild->getLongInt(); + bool folded = false; + if (divisor == -1 && dividend == TR::getMinSigned()) - foldLongIntConstant(node, 0, s, false /* !anchorChildren */); + folded = foldLongIntConstant(node, 0, s, false /* !anchorChildren */); else - foldLongIntConstant(node, dividend % divisor, s, false /* !anchorChildren */); + folded = foldLongIntConstant(node, dividend % divisor, s, false /* !anchorChildren */); + + if (folded) + { + // If folding to a constant actually happened, any parent DIVCHK is no longer needed + s->_nodeToDivchk = NULL; + } + return node; } // Design 1792 @@ -9443,6 +9607,10 @@ TR::Node *lremSimplifier(TR::Node * node, TR::Block * block, TR::Simplifier * s) node->setSecond(newNode4); node->getFirstChild()->incReferenceCount(); node->getSecondChild()->incReferenceCount(); + + // Division has been removed, so any parent DIVCHK is no longer needed + s->_nodeToDivchk = NULL; + return node; } // Disabled pending approval of design 1055. @@ -9478,6 +9646,10 @@ TR::Node *lremSimplifier(TR::Node * node, TR::Block * block, TR::Simplifier * s) node->setAndIncChild(0,firstChild); node->setAndIncChild(1,node1); node->setNumChildren(2); + + // Division has been removed, so any parent DIVCHK is no longer needed + s->_nodeToDivchk = NULL; + return node; } // divisor not power of 2 #endif // Disabled pending approval of design 1055. @@ -9493,10 +9665,6 @@ TR::Node *lremSimplifier(TR::Node * node, TR::Block * block, TR::Simplifier * s) performTransformation(s->comp(), "%sReduced lrem [%p] of two i2l children to i2l of irem \n", s->optDetailString(), node)) { TR::TreeTop *curTree = s->_curTree; - TR::Node *divCheckParent = NULL; - if ((curTree->getNode()->getOpCodeValue() == TR::DIVCHK) && - (curTree->getNode()->getFirstChild() == node)) - divCheckParent = curTree->getNode(); TR::Node *remNode = TR::Node::create(TR::irem, 2, firstChild->getFirstChild(), secondChild->getFirstChild()); @@ -9507,12 +9675,10 @@ TR::Node *lremSimplifier(TR::Node * node, TR::Block * block, TR::Simplifier * s) node->setAndIncChild(0, remNode); node->setNumChildren(1); - if (divCheckParent) - { - divCheckParent->setAndIncChild(0, remNode); - node->recursivelyDecReferenceCount(); - return remNode; - } + // Remainder operation has been transformed, but still needs to be checked + // for division by zero if the current node was the child of a DIVCHK + s->_nodeToDivchk = remNode; + return node; } @@ -9522,13 +9688,9 @@ TR::Node *lremSimplifier(TR::Node * node, TR::Block * block, TR::Simplifier * s) performTransformation(s->comp(), "%sReduced lrem by 10 [%p] to sequence of bitwise operations\n", s->optDetailString(), node)) { TR::TreeTop *curTree = s->_curTree; - TR::Node *divCheckParent = NULL; - if ((curTree->getNode()->getOpCodeValue() == TR::DIVCHK) && - (curTree->getNode()->getFirstChild() == node)) - divCheckParent = curTree->getNode(); // Create the long divide tree - TR::Node * ldivNode = TR::Node::create(node, TR::ladd); + TR::Node * ldivNode = TR::Node::create(node, TR::ladd); transformToLongDivBy10Bitwise(node, ldivNode, s); // Modify the lrem node @@ -9539,12 +9701,9 @@ TR::Node *lremSimplifier(TR::Node * node, TR::Block * block, TR::Simplifier * s) firstChild->recursivelyDecReferenceCount(); secondChild->recursivelyDecReferenceCount(); - if (divCheckParent) - { - divCheckParent->setAndIncChild(0, node); - node->recursivelyDecReferenceCount(); - return node; - } + // Division has been removed, so any parent DIVCHK is no longer needed + s->_nodeToDivchk = NULL; + return node; } } @@ -9613,11 +9772,22 @@ TR::Node *bremSimplifier(TR::Node * node, TR::Block * block, TR::Simplifier * s) { s->simplifyChildren(node, block); + // Handshake with divchkSimplifier: If simplifying the child of a DIVCHK + // results in a node that still needs to have a DIVCHK applied, place that + // node in _nodeToDivchk. If the simplification leaves no node that needs + // to have a DIVCHK applied, set _nodeToDivchk to NULL. + s->_nodeToDivchk = node; + TR::Node * firstChild = node->getFirstChild(), * secondChild = node->getSecondChild(); if (firstChild->getOpCode().isLoadConst() && secondChild->getOpCode().isLoadConst()) { - foldByteConstant(node, (int8_t)(firstChild->getByte() % secondChild->getByte()), s, false /* !anchorChildren*/); + if (foldByteConstant(node, (int8_t)(firstChild->getByte() % secondChild->getByte()), s, false /* !anchorChildren*/)) + { + // If folding to a constant actually happened, any parent DIVCHK is no longer needed + s->_nodeToDivchk = NULL; + } + return node; } @@ -9628,11 +9798,22 @@ TR::Node *sremSimplifier(TR::Node * node, TR::Block * block, TR::Simplifier * s) { s->simplifyChildren(node, block); + // Handshake with divchkSimplifier: If simplifying the child of a DIVCHK + // results in a node that still needs to have a DIVCHK applied, place that + // node in _nodeToDivchk. If the simplification leaves no node that needs + // to have a DIVCHK applied, set _nodeToDivchk to NULL. + s->_nodeToDivchk = node; + TR::Node * firstChild = node->getFirstChild(), * secondChild = node->getSecondChild(); if (firstChild->getOpCode().isLoadConst() && secondChild->getOpCode().isLoadConst()) { - foldShortIntConstant(node, (int16_t)(firstChild->getShortInt() % secondChild->getShortInt()), s, false /* !anchorChildren */); + if (foldShortIntConstant(node, (int16_t)(firstChild->getShortInt() % secondChild->getShortInt()), s, false /* !anchorChildren */)) + { + // If folding to a constant actually happened, any parent DIVCHK is no longer needed + s->_nodeToDivchk = NULL; + } + return node; } @@ -16534,25 +16715,123 @@ TR::Node *nullchkSimplifier(TR::Node * node, TR::Block * block, TR::Simplifier * // TR::Node *divchkSimplifier(TR::Node * node, TR::Block * block, TR::Simplifier * s) { - // Remember the original child of the divchk. If simplification of this - // child causes it to be replaced by another node, the divchk is no longer - // needed. - // The child of the divchk may no longer be a divide node, due to commoning. - // In this case the divchk is no longer needed. + TR::Node * child = node->getFirstChild(); + TR::Node * origChild = child; + TR::ILOpCode &childOpCode = child->getOpCode(); + + // Check whether the child has already been simplified // - TR::Node *originalChild = node->getFirstChild(); - /////TR::ILOpCodes originalChildOpCode = originalChild->getOpCode().getOpCodeValue(); - /////TR_ASSERT(originalChild->getVisitCount() != s->comp()->getVisitCount(),"Simplifier, bad divchk node"); - TR::Node * child = originalChild; - if (originalChild->getVisitCount() != s->comp()->getVisitCount()) - child = s->simplify(originalChild, block); + if (child->getVisitCount() != s->comp()->getVisitCount()) + { + if (childOpCode.isDiv() || childOpCode.isRem()) + { + s->_nodeToDivchk = NULL; + + child = s->simplify(child, block); - if (child != originalChild || - !(child->getOpCode().isDiv() || child->getOpCode().isRem())) + // Handshake with division and remainder simplifiers: If simplifying the + // child of a DIVCHK results in a node that still needs to have a DIVCHK + // applied, those simplififiers will place that node in _nodeToDivchk. If + // the simplification leaves no node that needs to have a DIVCHK applied, + // they will set _nodeToDivchk to NULL. If _nodeToDivchk is non-null, that + // is used node as the child of the DIVCHK; otherwise, the DIVCHK is + // simplified to a treetop node. + // + // Note that the division and remainder simplifiers will set _nodeToDivchk + // only after recursively simplifying their own children, so any setting of + // _nodeToDivchk by division or remainder operations that are deeper in the + // trees will not be seen upon returning to this method. + // + // For example, before simplifying the DIVCHK child, n98n, we might have the + // trees on the left and after, the trees on the right. Notice that the + // ldiv child of the DIVCHK has been replaced with an idiv that must still + // be checked for division by zero. The ldivSimplifier will set + // _nodeToDivchk to refer to node n100n so that divchkSimplifier + // can update the DIVCHK node to refer to the correct child node. + // + // n99n DIVCHK n99n DIVCHK + // n98n ldiv n100n idiv + // n97n i2l + // n96n iload a n96n iload a + // n95n i2l + // n94n iload b n94n iload b + // n93n lstore c n93n lstore c + // n98n ==> ldiv n98n i2l + // n100n ==> idiv + // + // If the numerator of a division is itself a division operation and the + // denominator is unity (1), the trees might look like those on the left, + // while if the numerator was simply the value of a variable, it might + // look like those on the right: + // + // n199n DIVCHK n299n DIVCHK + // n198n idiv n298n idiv + // n197n idiv n297n iload a + // n196n iload a n296n iconst 1 + // n195n iload b + // n194n iconst 1 + // + // For the trees on the left, the result of simplifying the child of the + // DIVCHK would be n197n, which is itself an idev, while for the trees on + // the right, the result would be iload, n297n. In both cases, the + // iremSimplifier would set _nodeToDivchk to NULL to indicate that the + // DIVCHK is no longer needed. + // + if (s->_nodeToDivchk == NULL) + { + if (s->trace()) + { + traceMsg(s->comp(), "Simplifying DIVCHK n%un %p child resulted in no node to DIVCHK - replacing DIVCHK with treetop\n", + node->getGlobalIndex(), node); + } + + TR::Node::recreate(node, TR::treetop); + node->setChild(0, child); + return node; + } + else + { + if (s->trace()) + { + traceMsg(s->comp(), "Simplifying DIVCHK child has left us with a node to DIVCHK - replacing child with n%un [%p]\n", + s->_nodeToDivchk->getGlobalIndex(), s->_nodeToDivchk); + } + + // Simplifying the child has left us with a node that still needs to + // have a DIVCHK applied. Replace the original child with the node + // that must have a DIVCHK - which could still be the original child. + // + node->setAndIncChild(0, s->_nodeToDivchk); + origChild->recursivelyDecReferenceCount(); + s->_nodeToDivchk = NULL; + } + } + else + { + // Child of DIVCHK must be a division or remainder operation. If it's not, + // eliminate the DIVCHK. + // + if (s->trace()) + { + traceMsg(s->comp(), "DIVCHK n%un %p child is not a division or remainder operation - replacing DIVCHK with treetop\n", node->getGlobalIndex(), node); + } + + TR::Node::recreate(node, TR::treetop); + return node; + } + } + else { - TR::Node::recreate(node, TR::treetop); - node->setFirst(child); - return node; + // The child node has already been visited, so it must have been anchored + // at some earlier TR::TreeTop. If it needed to be protected by a DIVCHK + // that DIVCHK would have appeared at that earlier point, so the current + // DIVCHK is no longer needed. + // + if (performTransformation(s->comp(), "%sRemoved DIVCHK for commoned division operation in node[%s]\n", s->optDetailString(), node->getName(s->getDebug()))) + { + TR::Node::recreate(node, TR::treetop); + return node; + } } // If the divisor is a non-zero constant, this check is redundant and diff --git a/compiler/optimizer/OMRSimplifierHelpers.cpp b/compiler/optimizer/OMRSimplifierHelpers.cpp index 8d90b9e5545..b403c182cf0 100644 --- a/compiler/optimizer/OMRSimplifierHelpers.cpp +++ b/compiler/optimizer/OMRSimplifierHelpers.cpp @@ -103,7 +103,7 @@ static bool swapChildren(TR::Node * node, TR::Simplifier * s) // bool performTransformationSimplifier(TR::Node * node, TR::Simplifier * s) { - return performTransformation(s->comp(), "%sConstant folding node [%s] %s", s->optDetailString(), node->getName(s->getDebug()), node->getOpCode().getName()); + return performTransformation(s->comp(), "%sConstant folding node [%s] %s\n", s->optDetailString(), node->getName(s->getDebug()), node->getOpCode().getName()); } void setIsHighWordZero(TR::Node * node, TR::Simplifier * s) @@ -141,9 +141,9 @@ TR::Node *_gotoSimplifier(TR::Node * node, TR::Block * block, TR::TreeTop* curTr return node; } -void foldIntConstant(TR::Node * node, int32_t value, TR::Simplifier * s, bool anchorChildrenP) +bool foldIntConstant(TR::Node * node, int32_t value, TR::Simplifier * s, bool anchorChildrenP) { - if (!performTransformationSimplifier(node, s)) return; + if (!performTransformationSimplifier(node, s)) return false; if (anchorChildrenP) s->anchorChildren(node, s->_curTree); @@ -163,22 +163,26 @@ void foldIntConstant(TR::Node * node, int32_t value, TR::Simplifier * s, bool an node->setInt(value); dumpOptDetails(s->comp(), " to %s %d\n", node->getOpCode().getName(), node->getInt()); } + + return true; } -void foldUIntConstant(TR::Node * node, uint32_t value, TR::Simplifier * s, bool anchorChildrenP) +bool foldUIntConstant(TR::Node * node, uint32_t value, TR::Simplifier * s, bool anchorChildrenP) { - if (!performTransformationSimplifier(node, s)) return; + if (!performTransformationSimplifier(node, s)) return false; if (anchorChildrenP) s->anchorChildren(node, s->_curTree); s->prepareToReplaceNode(node, TR::iconst); node->setUnsignedInt(value); dumpOptDetails(s->comp(), " to %s %d\n", node->getOpCode().getName(), node->getInt()); + + return true; } -void foldLongIntConstant(TR::Node * node, int64_t value, TR::Simplifier * s, bool anchorChildrenP) +bool foldLongIntConstant(TR::Node * node, int64_t value, TR::Simplifier * s, bool anchorChildrenP) { - if (!performTransformationSimplifier(node, s)) return; + if (!performTransformationSimplifier(node, s)) return false; if (anchorChildrenP) s->anchorChildren(node, s->_curTree); @@ -197,31 +201,41 @@ void foldLongIntConstant(TR::Node * node, int64_t value, TR::Simplifier * s, boo dumpOptDetails(s->comp(), " 0x%x\n", node->getLongIntLow()); else dumpOptDetails(s->comp(), " 0x%x%08x\n", node->getLongIntHigh(), node->getLongIntLow()); + + return true; } -void foldFloatConstant(TR::Node * node, float value, TR::Simplifier * s) +bool foldFloatConstant(TR::Node * node, float value, TR::Simplifier * s) { if (performTransformationSimplifier(node, s)) { s->prepareToReplaceNode(node, TR::fconst); node->setFloat(value); dumpOptDetails(s->comp(), " to %s %f\n", node->getOpCode().getName(), node->getFloat()); + + return true; } + + return false; } -void foldDoubleConstant(TR::Node * node, double value, TR::Simplifier * s) +bool foldDoubleConstant(TR::Node * node, double value, TR::Simplifier * s) { if (performTransformationSimplifier(node, s)) { s->prepareToReplaceNode(node, TR::dconst); node->setDouble(value); dumpOptDetails(s->comp(), " to %s %f\n", node->getOpCode().getName(), node->getDouble()); + + return true; } + + return false; } -void foldByteConstant(TR::Node * node, int8_t value, TR::Simplifier * s, bool anchorChildrenP) +bool foldByteConstant(TR::Node * node, int8_t value, TR::Simplifier * s, bool anchorChildrenP) { - if (!performTransformationSimplifier(node, s)) return; + if (!performTransformationSimplifier(node, s)) return false; if (anchorChildrenP) s->anchorChildren(node, s->_curTree); @@ -229,40 +243,47 @@ void foldByteConstant(TR::Node * node, int8_t value, TR::Simplifier * s, bool an node->setByte(value); dumpOptDetails(s->comp(), " to %s %d\n", node->getOpCode().getName(), node->getByte()); + return true; } -void foldShortIntConstant(TR::Node * node, int16_t value, TR::Simplifier * s, bool anchorChildrenP) +bool foldShortIntConstant(TR::Node * node, int16_t value, TR::Simplifier * s, bool anchorChildrenP) { if (!performTransformationSimplifier(node, s)) - return; + return false; if (anchorChildrenP) s->anchorChildren(node, s->_curTree); s->prepareToReplaceNode(node, TR::sconst); node->setShortInt(value); dumpOptDetails(s->comp(), " to %s %d\n", node->getOpCode().getName(), node->getShortInt()); + + return true; } -void foldUByteConstant(TR::Node * node, uint8_t value, TR::Simplifier * s, bool anchorChildrenP) +bool foldUByteConstant(TR::Node * node, uint8_t value, TR::Simplifier * s, bool anchorChildrenP) { - if (!performTransformationSimplifier(node, s)) return; + if (!performTransformationSimplifier(node, s)) return false; if (anchorChildrenP) s->anchorChildren(node, s->_curTree); s->prepareToReplaceNode(node, TR::bconst); node->setUnsignedByte(value); dumpOptDetails(s->comp(), " to %s %d\n", node->getOpCode().getName(), node->getUnsignedByte()); + + return true; } -void foldCharConstant(TR::Node * node, uint16_t value, TR::Simplifier * s, bool anchorChildrenP) +bool foldCharConstant(TR::Node * node, uint16_t value, TR::Simplifier * s, bool anchorChildrenP) { - if (!performTransformationSimplifier(node, s)) return; + if (!performTransformationSimplifier(node, s)) return false; if (anchorChildrenP) s->anchorChildren(node, s->_curTree); s->prepareToReplaceNode(node, TR::sconst); node->setConst(value); dumpOptDetails(s->comp(), " to %s %d\n", node->getOpCode().getName(), node->getConst()); + + return true; } bool swapChildren(TR::Node * node, TR::Node * & firstChild, TR::Node * & secondChild, TR::Simplifier * s) diff --git a/compiler/optimizer/OMRSimplifierHelpers.hpp b/compiler/optimizer/OMRSimplifierHelpers.hpp index 5614fdc78fc..5f53b7dfc86 100644 --- a/compiler/optimizer/OMRSimplifierHelpers.hpp +++ b/compiler/optimizer/OMRSimplifierHelpers.hpp @@ -169,15 +169,15 @@ inline OMR::TR_ConditionCodeNumber calculateUnsignedCC(uint64_t result, bool car bool performTransformationSimplifier(TR::Node * node, TR::Simplifier * s); void setIsHighWordZero(TR::Node * node, TR::Simplifier * s); TR::Node *_gotoSimplifier(TR::Node * node, TR::Block * block, TR::TreeTop* curTree, TR::Optimization * s); -void foldIntConstant(TR::Node * node, int32_t value, TR::Simplifier * s, bool anchorChildrenP); -void foldUIntConstant(TR::Node * node, uint32_t value, TR::Simplifier * s, bool anchorChildrenP); -void foldLongIntConstant(TR::Node * node, int64_t value, TR::Simplifier * s, bool anchorChildrenP); -void foldFloatConstant(TR::Node * node, float value, TR::Simplifier * s); -void foldDoubleConstant(TR::Node * node, double value, TR::Simplifier * s); -void foldByteConstant(TR::Node * node, int8_t value, TR::Simplifier * s, bool anchorChildrenP); -void foldUByteConstant(TR::Node * node, uint8_t value, TR::Simplifier * s, bool anchorChildrenP); -void foldCharConstant(TR::Node * node, uint16_t value, TR::Simplifier * s, bool anchorChildrenP); -void foldShortIntConstant(TR::Node * node, int16_t value, TR::Simplifier * s, bool anchorChildrenP); +bool foldIntConstant(TR::Node * node, int32_t value, TR::Simplifier * s, bool anchorChildrenP); +bool foldUIntConstant(TR::Node * node, uint32_t value, TR::Simplifier * s, bool anchorChildrenP); +bool foldLongIntConstant(TR::Node * node, int64_t value, TR::Simplifier * s, bool anchorChildrenP); +bool foldFloatConstant(TR::Node * node, float value, TR::Simplifier * s); +bool foldDoubleConstant(TR::Node * node, double value, TR::Simplifier * s); +bool foldByteConstant(TR::Node * node, int8_t value, TR::Simplifier * s, bool anchorChildrenP); +bool foldUByteConstant(TR::Node * node, uint8_t value, TR::Simplifier * s, bool anchorChildrenP); +bool foldCharConstant(TR::Node * node, uint16_t value, TR::Simplifier * s, bool anchorChildrenP); +bool foldShortIntConstant(TR::Node * node, int16_t value, TR::Simplifier * s, bool anchorChildrenP); bool swapChildren(TR::Node * node, TR::Node * & firstChild, TR::Node * & secondChild, TR::Simplifier * s); bool isExprInvariant(TR_RegionStructure *region, TR::Node *node); void orderChildren(TR::Node * node, TR::Node * & firstChild, TR::Node * & secondChild, TR::Simplifier * s);