Removed OMR::IDTBuilder::copyDescendants() and resolved some new fe…

…edbacks.

Signed-off-by: Mingwei Li <mingweiarthurli@gmail.com>

compiler/optimizer/BenefitInliner.cpp

@@ -3,7 +3,7 @@
  *
  * This program and the accompanying materials are made available under
  * the terms of the Eclipse Public License 2.0 which accompanies this
- * distribution and is available at http://eclipse.org/legal/epl-2.0
+ * distribution and is available at https://www.eclipse.org/legal/epl-2.0/
  * or the Apache License, Version 2.0 which accompanies this distribution
  * and is available at https://www.apache.org/licenses/LICENSE-2.0.
  *
@@ -76,6 +76,7 @@ void TR::BenefitInliner::buildInliningDependencyTree()
 
 void TR::BenefitInliner::inlinerPacking()
    {
+   // if get enough budget to inline all options
    if (_inliningDependencyTree->getTotalCost() <= _budget)
       {
       _inliningProposal = new (region()) TR::InliningProposal(region(), _inliningDependencyTree);
@@ -99,6 +100,15 @@ void TR::BenefitInliner::inlinerPacking()
       return;
       }
 
+   /**
+    * An implementation of knapsack packing algorithm (a modified dynamic programming algorithm)
+    *
+    * This algorithm is described in following patent:
+    * https://patents.google.com/patent/US10055210B2/en
+    * and this is an implementation of the algorithm on the flowchart FIG.9.
+    *
+    * The following comments label which part they belong to the algorithm on the flowchart FIG.9.
+    */
    _inliningDependencyTree->flattenIDT();
 
    const uint32_t idtSize = _inliningDependencyTree->getNumNodes();
@@ -107,6 +117,7 @@ void TR::BenefitInliner::inlinerPacking()
    //initialize InliningProposal Table (idtSize x budget+1)
    TR::InliningProposalTable table(idtSize, budget + 1, comp()->trMemory()->currentStackRegion());
 
+   // prepare preorder of inlining options
    TR::IDTPriorityQueue pQueue(_inliningDependencyTree, comp()->trMemory()->currentStackRegion());
    for (uint32_t row = 0; row < idtSize; row ++)
       {
@@ -119,31 +130,37 @@ void TR::BenefitInliner::inlinerPacking()
 
          uint32_t offsetRow = row - 1;
 
+         // check if proposal is valid
          while (!currentNode->isRoot()
-            && !table.get(offsetRow, col-currentSet.getCost())->isNodeInProposal(currentNode->getParent()))
+            && !table.get(offsetRow, col - currentSet.getCost())->isNodeInProposal(currentNode->getParent()))
             {
+            // if no, add proposal predecessor to proposal
             currentSet.addNode(currentNode->getParent());
             currentNode = currentNode->getParent();
             }
 
+         // check if intersects base or if is invalid solution
          while ( currentSet.intersects(table.get(offsetRow, col - currentSet.getCost()))
             || ( !(currentNode->getParent() && table.get(offsetRow, col - currentSet.getCost())->isNodeInProposal(currentNode->getParent()) )
                   && !table.get(offsetRow, col - currentSet.getCost())->isEmpty()
                   ))
             {
+            // if yes, consider prior base at same budget
             offsetRow--;
             }
 
          TR::InliningProposal* newProposal = new (comp()->trMemory()->currentStackRegion()) TR::InliningProposal(comp()->trMemory()->currentStackRegion(), _inliningDependencyTree);
          newProposal->merge(table.get(offsetRow, col - currentSet.getCost()), &currentSet);
 
+         // check if cost less than budget and if previous proposal is better
          if (newProposal->getCost() <= col && newProposal->getBenefit() > table.get(row-1, col)->getBenefit()) //only set the new proposal if it fits the budget and has more benefits
             table.set(row, col, newProposal);
          else
             table.set(row, col, table.get(row-1, col));
          }
       }
 
+   // read solution from table at last row and max budget
    TR::InliningProposal* result = new (region()) TR::InliningProposal(region(), _inliningDependencyTree);
    result->merge(result, table.get(idtSize-1, budget));
 

compiler/optimizer/BenefitInliner.hpp

@@ -3,7 +3,7 @@
  *
  * This program and the accompanying materials are made available under
  * the terms of the Eclipse Public License 2.0 which accompanies this
- * distribution and is available at http://eclipse.org/legal/epl-2.0
+ * distribution and is available at https://www.eclipse.org/legal/epl-2.0/s
  * or the Apache License, Version 2.0 which accompanies this distribution
  * and is available at https://www.apache.org/licenses/LICENSE-2.0.
  *

compiler/optimizer/abstractinterpreter/AbsOpArray.cpp

@@ -3,7 +3,7 @@
  *
  * This program and the accompanying materials are made available under
  * the terms of the Eclipse Public License 2.0 which accompanies this
- * distribution and is available at http://eclipse.org/legal/epl-2.0
+ * distribution and is available at https://www.eclipse.org/legal/epl-2.0/
  * or the Apache License, Version 2.0 which accompanies this distribution
  * and is available at https://www.apache.org/licenses/LICENSE-2.0.
  *
@@ -40,19 +40,19 @@ void TR::AbsOpArray::merge(const TR::AbsOpArray* other, TR::Region& region)
       TR::AbsValue *selfValue = at(i);
       TR::AbsValue *otherValue = other->at(i);
 
-      if (!selfValue && !otherValue) 
+      if (!selfValue && !otherValue)
          {
          continue;
          }
-      else if (selfValue && otherValue) 
+      else if (selfValue && otherValue)
          {
          TR::AbsValue* mergedVal = selfValue->merge(otherValue);
          set(i, mergedVal);
-         } 
-      else if (selfValue) 
+         }
+      else if (selfValue)
          {
          set(i, selfValue);
-         } 
+         }
       else
          {
          set(i, otherValue->clone(region));
@@ -69,7 +69,7 @@ void TR::AbsOpArray::set(uint32_t index, TR::AbsValue *value)
 TR::AbsValue* TR::AbsOpArray::at(uint32_t index) const
    {
    TR_ASSERT_FATAL(index < size(), "Index out of range! Max array size: %d, Index: %d\n", size(), index);
-   return _container[index]; 
+   return _container[index];
    }
 
 void TR::AbsOpArray::print(TR::Compilation* comp) const
@@ -80,7 +80,7 @@ void TR::AbsOpArray::print(TR::Compilation* comp) const
       traceMsg(comp, "A[%d] = ", i);
       if (!at(i))
          traceMsg(comp, "Uninitialized");
-      else 
+      else
          at(i)->print(comp);
 
       traceMsg(comp, "\n");

compiler/optimizer/abstractinterpreter/AbsOpArray.hpp

@@ -3,7 +3,7 @@
  *
  * This program and the accompanying materials are made available under
  * the terms of the Eclipse Public License 2.0 which accompanies this
- * distribution and is available at http://eclipse.org/legal/epl-2.0
+ * distribution and is available at https://www.eclipse.org/legal/epl-2.0/
  * or the Apache License, Version 2.0 which accompanies this distribution
  * and is available at https://www.apache.org/licenses/LICENSE-2.0.
  *
@@ -41,21 +41,21 @@ class AbsOpArray
 
    /**
     * @brief Clone the operand array
-    * 
+    *
     * @param region The memory region where the cloned operand array should be allocated.
     * @return the cloned operand array
     */
    TR::AbsOpArray *clone(TR::Region& region) const;
 
    /**
     * @brief Perform an in-place merge with another operand array.
-    * The merge operation does not modify the state of another state 
+    * The merge operation does not modify the state of another state
     * or store any references of abstract values from another state to be merged with
     *
     * @param other The operand array to be merged with.
     */
    void merge(const TR::AbsOpArray* other, TR::Region& region);
-   
+
    /**
     * @brief Get the abstract value at index i.
     *

compiler/optimizer/abstractinterpreter/AbsOpStack.cpp

@@ -3,7 +3,7 @@
  *
  * This program and the accompanying materials are made available under
  * the terms of the Eclipse Public License 2.0 which accompanies this
- * distribution and is available at http://eclipse.org/legal/epl-2.0
+ * distribution and is available at https://www.eclipse.org/legal/epl-2.0/
  * or the Apache License, Version 2.0 which accompanies this distribution
  * and is available at https://www.apache.org/licenses/LICENSE-2.0.
  *
@@ -55,24 +55,24 @@ void TR::AbsOpStack::merge(const TR::AbsOpStack* other, TR::Region& region)
 void TR::AbsOpStack::print(TR::Compilation* comp) const
    {
    traceMsg(comp, "Contents of Abstract Operand Stack:\n");
-   
+
    const size_t stackSize = size();
 
    if (stackSize == 0)
       {
       traceMsg(comp, "<empty>\n\n");
       return;
       }
-   
+
    traceMsg(comp, "<top>\n");
 
-   for (size_t i = 0; i < stackSize; i++) 
+   for (size_t i = 0; i < stackSize; i++)
       {
       TR::AbsValue *value = _container[stackSize - i -1 ];
       traceMsg(comp, "S[%d] = ", stackSize - i - 1);
       if (value)
          value->print(comp);
-      else 
+      else
          traceMsg(comp, "Uninitialized");
       traceMsg(comp, "\n");
       }

compiler/optimizer/abstractinterpreter/AbsOpStack.hpp

@@ -3,7 +3,7 @@
  *
  * This program and the accompanying materials are made available under
  * the terms of the Eclipse Public License 2.0 which accompanies this
- * distribution and is available at http://eclipse.org/legal/epl-2.0
+ * distribution and is available at https://www.eclipse.org/legal/epl-2.0/
  * or the Apache License, Version 2.0 which accompanies this distribution
  * and is available at https://www.apache.org/licenses/LICENSE-2.0.
  *
@@ -37,18 +37,18 @@ class AbsOpStack
    explicit AbsOpStack(TR::Region& region) :
          _container(region)
       {}
-      
+
    /**
     * @brief Clone an operand stack
-    * 
+    *
     * @param region The memory region where the cloned operand stack should be allocated.
     * @return the cloned operand stack
     */
    TR::AbsOpStack* clone(TR::Region& region) const;
 
    /**
     * @brief Perform an in-place merge another operand stack.
-    * The merge operation does not modify the state of another state 
+    * The merge operation does not modify the state of another state
     * or store any references of abstract values from another state to be merged with
     *
     * @param other the operand stack to be merged with
@@ -65,26 +65,26 @@ class AbsOpStack
    /**
     * @brief Peek the top value on the operand stack (stack top; not lattice top).
     * @note the stack must not be empty
-    * 
+    *
     * @return the abstract value
     */
    TR::AbsValue* peek() const { TR_ASSERT_FATAL(size() > 0, "Peek an empty stack!"); return _container.back(); }
 
    /**
     * @brief Get and pop a value off of the operand stack.
     * @note the stack must not be empty.
-    * 
+    *
     * @return the abstract value
     */
    TR::AbsValue* pop();
 
    bool empty() const { return _container.empty(); }
    size_t size() const { return _container.size(); }
-  
+
    void print(TR::Compilation* comp) const;
 
    private:
-   TR::vector<TR::AbsValue*, TR::Region&> _container; 
+   TR::vector<TR::AbsValue*, TR::Region&> _container;
    };
 
 }

compiler/optimizer/abstractinterpreter/AbsValue.cpp

@@ -3,7 +3,7 @@
  *
  * This program and the accompanying materials are made available under
  * the terms of the Eclipse Public License 2.0 which accompanies this
- * distribution and is available at http://eclipse.org/legal/epl-2.0
+ * distribution and is available at https://www.eclipse.org/legal/epl-2.0/
  * or the Apache License, Version 2.0 which accompanies this distribution
  * and is available at https://www.apache.org/licenses/LICENSE-2.0.
  *
@@ -23,19 +23,19 @@
 
 TR::AbsValue* TR::AbsVPValue::clone(TR::Region& region) const
    {
-   TR::AbsVPValue* copy = new (region) TR::AbsVPValue(_vp, _constraint, _dataType, _paramPos);   
+   TR::AbsVPValue* copy = new (region) TR::AbsVPValue(_vp, _constraint, _dataType, _paramPos);
    return copy;
    }
 
 TR::AbsValue* TR::AbsVPValue::merge(const TR::AbsValue *other)
    {
    if (other == NULL)
       return this;
-   
-   if (_paramPos != other->getParameterPosition()) 
+
+   if (_paramPos != other->getParameterPosition())
       _paramPos = -1;
 
-   if (other->getDataType() != _dataType) 
+   if (other->getDataType() != _dataType)
       {
       _dataType = TR::NoType;
       setToTop();
@@ -45,7 +45,7 @@ TR::AbsValue* TR::AbsVPValue::merge(const TR::AbsValue *other)
    if (isTop())
       return this;
 
-   if (other->isTop()) 
+   if (other->isTop())
       {
       setToTop();
       return this;
@@ -57,16 +57,16 @@ TR::AbsValue* TR::AbsVPValue::merge(const TR::AbsValue *other)
    return this;
    }
 
-void TR::AbsVPValue::print(TR::Compilation* comp) const  
+void TR::AbsVPValue::print(TR::Compilation* comp) const
    {
    traceMsg(comp, "AbsValue: Type: %s ", TR::DataType::getName(_dataType));
-   
+
    if (_constraint)
       {
       traceMsg(comp, "Constraint: ");
       _constraint->print(_vp);
       }
-   else 
+   else
       {
       traceMsg(comp, "TOP (unknown) ");
       }

compiler/optimizer/abstractinterpreter/AbsValue.hpp

@@ -3,7 +3,7 @@
  *
  * This program and the accompanying materials are made available under
  * the terms of the Eclipse Public License 2.0 which accompanies this
- * distribution and is available at http://eclipse.org/legal/epl-2.0
+ * distribution and is available at https://www.eclipse.org/legal/epl-2.0/
  * or the Apache License, Version 2.0 which accompanies this distribution
  * and is available at https://www.apache.org/licenses/LICENSE-2.0.
  *
@@ -44,18 +44,18 @@ class AbsValue
 
    /**
     * @brief Clone an abstract value
-    * 
+    *
     * @param region The region where the cloned value will be allocated on.
     * @return the cloned abstract value
     */
    virtual TR::AbsValue* clone(TR::Region& region) const =0;
 
    /**
-    * @brief Merge with another AbsValue. 
+    * @brief Merge with another AbsValue.
     * @note This is an in-place merge. Self should modify other.
-    * Also, self should not store any mutable references from other during the merge 
+    * Also, self should not store any mutable references from other during the merge
     * but immutable references are allowed.
-    * 
+    *
     *
     * @param other Another AbsValue to be merged with
     * @return Self after the merge
@@ -96,7 +96,7 @@ class AbsValue
          _paramPos(paramPos)
       {}
 
-   int32_t _paramPos; 
+   int32_t _paramPos;
    TR::DataType _dataType;
    };
 

compiler/optimizer/abstractinterpreter/AbsVisitor.hpp

@@ -3,7 +3,7 @@
  *
  * This program and the accompanying materials are made available under
  * the terms of the Eclipse Public License 2.0 which accompanies this
- * distribution and is available at http://eclipse.org/legal/epl-2.0
+ * distribution and is available at https://www.eclipse.org/legal/epl-2.0/
  * or the Apache License, Version 2.0 which accompanies this distribution
  * and is available at https://www.apache.org/licenses/LICENSE-2.0.
  *
@@ -36,7 +36,7 @@ namespace TR {
 class AbsVisitor
    {
    public:
-   virtual void visitCallSite(TR_CallSite* callSite, int32_t callerIndex, TR::Block* callBlock, TR::vector<TR::AbsValue*, TR::Region&>* arguments) = 0;
+   virtual void visitCallSite(TR_CallSite* callSite, TR::Block* callBlock, TR::vector<TR::AbsValue*, TR::Region&>* arguments) = 0;
    };
 }