Merge pull request #4817 from cathyzhyi/deadtree

Fix DeadTreeElimination for volatile load

compiler/optimizer/DeadTreesElimination.cpp

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2019 IBM Corp. and others
+ * Copyright (c) 2000, 2020 IBM Corp. and others
  *
  * This program and the accompanying materials are made available under
  * the terms of the Eclipse Public License 2.0 which accompanies this
@@ -228,26 +228,46 @@ static int32_t getLongestPathOfDAG(TR::Node *node, LongestPathMap &memo)
    return value;
    }
 
-static bool containsNode(TR::Node *containerNode, TR::Node *node, vcount_t visitCount, TR::Compilation *comp, int32_t *height, int32_t *maxHeight, bool &canMoveIfVolatile)
+/*
+ * \brief This function does 2 things:
+ *        1. check if the \p nodeToSwingDown is in the subtree of \p containerNode
+ *        2. only allowing volatile load being swung down across temps and constant string
+ *
+ * \parm nodeToSwingDown
+ *    The node hanging under a treetop which can potentially be removed
+ *
+ * \parm containerNode
+ *    The subtree to check
+ */
+static bool containsNode(TR::Node *containerNode, TR::Node *nodeToSwingDown, vcount_t visitCount, TR::Compilation *comp, int32_t *height, int32_t *maxHeight, bool &canMoveIfVolatile)
    {
-   if (containerNode == node)
+   if (containerNode == nodeToSwingDown)
       return true;
 
    vcount_t oldVisitCount = containerNode->getVisitCount();
    if ((oldVisitCount == visitCount) || (oldVisitCount == comp->getVisitCount()))
       return false;
    containerNode->setVisitCount(comp->getVisitCount());
 
-   if (containerNode->getOpCode().hasSymbolReference() &&
-       (containerNode->getSymbol()->isShadow() || containerNode->getSymbol()->isStatic()))
-      canMoveIfVolatile = false;
+   if (containerNode->getOpCode().hasSymbolReference())
+      {
+      static const bool relaxedConditionForSwingingDownVolatile = feGetEnv("TR_relaxedConditionForSwingingDownVolatile") ? true: false;
+      if (!relaxedConditionForSwingingDownVolatile)
+         {
+         if (!(containerNode->getSymbol()->isAutoOrParm() ||
+               containerNode->getSymbol()->isConstString()))
+            canMoveIfVolatile = false;
+         }
+      else if (containerNode->getSymbol()->isShadow() || containerNode->getSymbol()->isStatic())
+         canMoveIfVolatile = false;
+      }
 
    (*height)++;
    if (*height > *maxHeight)
      *maxHeight = *height;
    for (int32_t i = 0; i < containerNode->getNumChildren(); ++i)
       {
-      if (containsNode(containerNode->getChild(i), node, visitCount, comp, height, maxHeight, canMoveIfVolatile))
+      if (containsNode(containerNode->getChild(i), nodeToSwingDown, visitCount, comp, height, maxHeight, canMoveIfVolatile))
          return true;
       }
    (*height)--;
@@ -258,7 +278,7 @@ static bool containsNode(TR::Node *containerNode, TR::Node *node, vcount_t visit
 #define MAX_ALLOWED_HEIGHT 50
 
 static bool isSafeToReplaceNode(TR::Node *currentNode, TR::TreeTop *curTreeTop, bool *seenConditionalBranch,
-      vcount_t visitCount, TR::Compilation *comp, List<OMR::TreeInfo> *targetTrees, bool &cannotBeEliminated,
+      vcount_t visitCount, TR::Compilation *comp, TR::Optimization *opt, List<OMR::TreeInfo> *targetTrees, bool &cannotBeEliminated,
       LongestPathMap &longestPaths)
    {
    LexicalTimer tx("safeToReplace", comp->phaseTimer());
@@ -308,8 +328,6 @@ static bool isSafeToReplaceNode(TR::Node *currentNode, TR::TreeTop *curTreeTop,
       return false;
 
    bool mayBeVolatileReference = currentNode->mightHaveVolatileSymbolReference();
-   //if (mayBeVolatileReference)
-   //   return false;
 
    // Now scan forwards through the trees looking for the next use and checking
    // to see if any symbols in the subtree are getting modified; if so it is not
@@ -318,10 +336,9 @@ static bool isSafeToReplaceNode(TR::Node *currentNode, TR::TreeTop *curTreeTop,
 
    comp->incVisitCount();
    for (TR::TreeTop *treeTop = curTreeTop->getNextTreeTop(); treeTop; treeTop = treeTop->getNextTreeTop())
-
       {
       TR::Node *node = treeTop->getNode();
-      if(node->getOpCodeValue() == TR::treetop)
+      if (node->getOpCodeValue() == TR::treetop)
           node = node->getFirstChild();
 
       if (node->getOpCodeValue() == TR::BBStart &&
@@ -339,7 +356,20 @@ static bool isSafeToReplaceNode(TR::Node *currentNode, TR::TreeTop *curTreeTop,
       int32_t tempHeight = 0;
       int32_t maxHeight = 0;
       bool canMoveIfVolatile = true;
-      if (containsNode(node, currentNode, visitCount, comp, &tempHeight, &maxHeight, canMoveIfVolatile))
+      bool nodeInSubTree = containsNode(node, currentNode, visitCount, comp, &tempHeight, &maxHeight, canMoveIfVolatile);
+      /*
+       * Restrictions for swinging down volatile applies even when the volatile node is in the current sub-tree:
+       * treetop
+       *    a.volatileField
+       * someOpcode someSymRef
+       *    ...
+       *    => xload/xloadi a.volatileField
+       *    ...
+       */
+      if (mayBeVolatileReference && !canMoveIfVolatile)
+         return false;
+
+      if (nodeInSubTree)
          {
          // TEMPORARY
          // Disable moving an unresolved reference down to the middle of a
@@ -368,12 +398,11 @@ static bool isSafeToReplaceNode(TR::Node *currentNode, TR::TreeTop *curTreeTop,
             treeInfo->setHeight(height);
             }
 
+         if (mayBeVolatileReference)
+            dumpOptDetails(opt->comp(), "%sit is safe to remove volatile field load tree n%dn\n", opt->optDetailString(), currentNode->getGlobalIndex());
          return true;
          }
 
-      if (mayBeVolatileReference && !canMoveIfVolatile)
-         return false;
-
       if ((node->getOpCode().isBranch() &&
            (node->getOpCodeValue() != TR::Goto)) ||
            (node->getOpCode().isJumpWithMultipleTargets() && node->getOpCode().hasBranchChildren()))
@@ -863,6 +892,7 @@ int32_t TR::DeadTreesElimination::process(TR::TreeTop *startTree, TR::TreeTop *e
                   &seenConditionalBranch,
                   visitCount,
                   comp(),
+                  this,
                   &_targetTrees,
                   _cannotBeEliminated,
                   longestPaths);