Resolve review feedback

Signed-off-by: Mingwei Li <[email protected]>

compiler/optimizer/BenefitInliner.cpp

@@ -26,10 +26,6 @@
 #include <algorithm>
 
 
-static bool isWarm(TR::Compilation *comp)
-   {
-   return comp->getMethodHotness() >= warm;
-   }
 static bool isHot(TR::Compilation *comp)
    {
    return comp->getMethodHotness() >= hot;
@@ -80,7 +76,7 @@ void TR::BenefitInliner::buildInliningDependencyTree()
 
 void TR::BenefitInliner::inlinerPacking()
    {
-   if (_inliningDependencyTree->getTotalCost() <= unsigned(_budget))
+   if (_inliningDependencyTree->getTotalCost() <= _budget)
       {
       _inliningProposal = new (region()) TR::InliningProposal(region(), _inliningDependencyTree);
 
@@ -105,19 +101,19 @@ void TR::BenefitInliner::inlinerPacking()
 
    _inliningDependencyTree->flattenIDT();
 
-   const int32_t idtSize = _inliningDependencyTree->getNumNodes();
-   const int32_t budget = _budget;
+   const uint32_t idtSize = _inliningDependencyTree->getNumNodes();
+   const uint32_t budget = _budget;
 
    //initialize InliningProposal Table (idtSize x budget+1)
    TR::InliningProposalTable table(idtSize, budget + 1, comp()->trMemory()->currentStackRegion());
 
-   TR::IDTPriorityQueue preorderPQueue(_inliningDependencyTree, comp()->trMemory()->currentStackRegion());
-   for (uint32_t row = 0; row < unsigned(idtSize); row ++)
+   TR::IDTPriorityQueue pQueue(_inliningDependencyTree, comp()->trMemory()->currentStackRegion());
+   for (uint32_t row = 0; row < idtSize; row ++)
       {
-      for (uint32_t col = 1; col < unsigned(budget + 1); col ++)
+      for (uint32_t col = 1; col <= budget; col ++)
          {
          TR::InliningProposal currentSet(comp()->trMemory()->currentStackRegion(), _inliningDependencyTree); // []
-         TR::IDTNode* currentNode = preorderPQueue.get(row);
+         TR::IDTNode* currentNode = pQueue.get(row);
 
          currentSet.addNode(currentNode); //[ currentNode ]
 
@@ -139,7 +135,7 @@ void TR::BenefitInliner::inlinerPacking()
             }
 
          TR::InliningProposal* newProposal = new (comp()->trMemory()->currentStackRegion()) TR::InliningProposal(comp()->trMemory()->currentStackRegion(), _inliningDependencyTree);
-         newProposal->unionInPlace(table.get(offsetRow, col - currentSet.getCost()), &currentSet);
+         newProposal->merge(table.get(offsetRow, col - currentSet.getCost()), &currentSet);
 
          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);
@@ -149,7 +145,7 @@ void TR::BenefitInliner::inlinerPacking()
       }
 
    TR::InliningProposal* result = new (region()) TR::InliningProposal(region(), _inliningDependencyTree);
-   result->unionInPlace(result, table.get(idtSize-1, budget));
+   result->merge(result, table.get(idtSize-1, budget));
 
    if (comp()->getOption(TR_TraceBIProposal))
       {
@@ -223,25 +219,16 @@ bool TR::BenefitInlinerBase::inlineIntoIDTNode(TR::ResolvedMethodSymbol *symbol,
       if (!shouldInline)
          continue;
 
+      //set _nextIDTNodeToInlineInto because we expect to enter inlineCallTargets() recursively
       _nextIDTNodeToInlineInto = childToInline;
 
       bool success = analyzeCallSite(callStack, tt, parent, node, childToInline->getCallTarget());
 
-      _nextIDTNodeToInlineInto = _nextIDTNodeToInlineInto->getParent();
+      _nextIDTNodeToInlineInto = idtNode;
 
       if (success)
          {
          inlineCount++;
-
-#define MAX_INLINE_COUNT 1000
-         if (inlineCount >= MAX_INLINE_COUNT)
-            {
-            if (comp()->trace(OMR::inlining))
-               traceMsg(comp(), "TR::BenefitInliner: stopping inlining as max inline count of %d reached\n", MAX_INLINE_COUNT);
-            break;
-            }
-
-#undef MAX_INLINE_COUNT
          node->setVisitCount(_visitCount);
 
          }

compiler/optimizer/BenefitInliner.hpp

@@ -32,18 +32,18 @@ namespace TR {
 class BenefitInlinerWrapper : public TR::Optimization
    {
    public:
-   BenefitInlinerWrapper(TR::OptimizationManager* manager) : TR::Optimization(manager) {};
+   BenefitInlinerWrapper(TR::OptimizationManager* manager) : TR::Optimization(manager) {}
 
    static TR::Optimization* create(TR::OptimizationManager *manager)
       {
-      return  new (manager->allocator()) BenefitInlinerWrapper(manager);
+      return new (manager->allocator()) BenefitInlinerWrapper(manager);
       }
 
    virtual int32_t perform();
 
    virtual const char * optDetailString() const throw()
       {
-      return "O^O Benefit Inliner: ";
+      return "O^O BENEFIT INLINER: ";
       }
    };
 
@@ -53,10 +53,11 @@ class BenefitInlinerBase : public TR_InlinerBase
    BenefitInlinerBase(TR::Optimizer* optimizer, TR::Optimization* optimization) :
          TR_InlinerBase(optimizer, optimization),
          _inliningProposal(NULL),
+         _region(comp()->region()),
          _budget(getInliningBudget(comp()->getMethodSymbol())),
          _inliningDependencyTree(NULL),
-         _region(comp()->region())
-      {};
+         _nextIDTNodeToInlineInto(NULL)
+      {}
 
 
    virtual bool inlineCallTargets(TR::ResolvedMethodSymbol* symbol, TR_CallStack* callStack, TR_InnerPreexistenceInfo* info);
@@ -65,10 +66,9 @@ class BenefitInlinerBase : public TR_InlinerBase
 
    virtual bool exceedsSizeThreshold(TR_CallSite *callSite, int bytecodeSize, TR::Block * callNodeBlock, TR_ByteCodeInfo & bcInfo,  int32_t numLocals=0, TR_ResolvedMethod * caller = 0, TR_ResolvedMethod * calleeResolvedMethod = 0, TR::Node * callNode = 0, bool allConsts = false)
       {return false;}
-   virtual bool supportsMultipleTargetInlining() { return false; };
    virtual bool analyzeCallSite(TR_CallStack * callStack, TR::TreeTop * callNodeTreeTop, TR::Node * parent, TR::Node * callNode, TR_CallTarget *calltargetToInline);
 
-   TR::Region& region() { return _region; };
+   TR::Region& region() { return _region; }
 
    TR::InliningProposal* _inliningProposal;
 

compiler/optimizer/Dominators.cpp

@@ -128,87 +128,6 @@ TR_Dominators::TR_Dominators(TR::Compilation *c, bool post) :
    _info.clear();
    }
 
-TR_Dominators::TR_Dominators(TR::Compilation *c, TR::CFG* cfg, bool post) :
-   _region(c->trMemory()->heapMemoryRegion()),
-   _compilation(c),
-   _info(cfg->getNextNodeNumber()+1, BBInfo(_region), _region),
-   _dfNumbers(cfg->getNextNodeNumber()+1, 0, _region),
-   _dominators(cfg->getNextNodeNumber()+1, static_cast<TR::Block *>(NULL), _region)
-   {
-   LexicalTimer tlex("TR_Dominators::TR_Dominators", _compilation->phaseTimer());
-
-   _postDominators = post;
-   _isValid = true;
-   _topDfNum = 0;
-   _visitCount = c->incOrResetVisitCount();
-   _trace = comp()->getOption(TR_TraceDominators);
-
-   _cfg = cfg;
-   _numNodes = cfg->getNumberOfNodes()+1;
-
-   if (trace())
-      {
-      traceMsg(comp(), "Starting %sdominator calculation\n", _postDominators ? "post-" : "");
-      traceMsg(comp(), "   Number of nodes is %d\n", _numNodes-1);
-      }
-
-   if (_postDominators)
-      _dfNumbers[cfg->getStart()->getNumber()] = -1;
-   else
-      _dfNumbers[cfg->getEnd()->getNumber()] = -1;
-
-   findDominators(toBlock( _postDominators ? cfg->getEnd() : cfg->getStart() ));
-
-   int32_t i;
-   for (i = _topDfNum; i > 1; i--)
-      {
-      BBInfo &info = getInfo(i);
-      TR::Block *dominated = info._block;
-      TR::Block *dominator = getInfo(info._idom)._block;
-      _dominators[dominated->getNumber()] = dominator;
-      if (trace())
-         traceMsg(comp(), "   %sDominator of block_%d is block_%d\n", _postDominators ? "post-" : "",
-                                      dominated->getNumber(), dominator->getNumber());
-      }
-
-   // The exit block may not be reachable from the entry node. In this case just
-   // give the exit block the highest depth-first numbering.
-   // No other blocks should be unreachable.
-   //
-
-   if (_postDominators)
-      {
-      if (_dfNumbers[cfg->getStart()->getNumber()] < 0)
-         _dfNumbers[cfg->getStart()->getNumber()] = _topDfNum++;
-      }
-   else
-      {
-      if (_dfNumbers[cfg->getEnd()->getNumber()] < 0)
-         _dfNumbers[cfg->getEnd()->getNumber()] = _topDfNum++;
-      }
-
-   // Assert that we've found every node in the cfg.
-   //
-   if (_topDfNum != _numNodes-1)
-      {
-      _isValid = false;
-      return;
-      }
-
-   #if DEBUG
-      for (block = toBlock(cfg->getFirstNode()); block; block = toBlock(block->getNext()))
-         {
-         TR_ASSERT(_dfNumbers[block->getNumber()] >= 0, "Unreachable block in the CFG");
-         }
-   #endif
-
-   if (trace())
-      traceMsg(comp(), "End of %sdominator calculation\n", _postDominators ? "post-" : "");
-
-   // Release no-longer-used data
-   _info.clear();
-   }
-
 TR::Block * TR_Dominators::getDominator(TR::Block *block)
    {
    if (block->getNumber() >= _dominators.size())

compiler/optimizer/Dominators.hpp

@@ -53,13 +53,11 @@ class TR_Dominators
    TR_ALLOC(TR_Memory::Dominators)
 
    TR_Dominators(TR::Compilation *, bool post = false);
-   TR_Dominators(TR::Compilation *, TR::CFG* cfg, bool post = false);
    TR::Block       *getDominator(TR::Block *);
    int             dominates(TR::Block *block, TR::Block *other);
 
    TR::Compilation * comp()         { return _compilation; }
    bool trace() { return _trace; }
-   bool isValid() { return _isValid; }
 
    protected:
 

compiler/optimizer/StructuralAnalysis.cpp

@@ -168,50 +168,6 @@ void TR_RegionAnalysis::createLeafStructures(TR::CFG *cfg, TR::Region &region)
       }
    }
 
-/**
- * Mainline for performing Region Analysis.
- */
-TR_Structure *TR_RegionAnalysis::getRegions(TR::Compilation *comp, TR::CFG* cfg)
-   {
-   TR::StackMemoryRegion stackMemoryRegion(*comp->trMemory());
-
-   // Calculate dominators
-   // This has the side-effect of renumbering the blocks in depth-first order
-   //
-   TR_Dominators dominators = TR_Dominators(comp, cfg);
-
-   if (!dominators.isValid())
-      return NULL;
-
-   #if DEBUG
-   if (debug("verifyDominator"))
-      {
-      TR_DominatorVerifier verifyDominator(dominators);
-      }
-   #endif
-
-   TR_ASSERT(cfg, "cfg is NULL\n");
-
-   TR_RegionAnalysis ra(comp, dominators, cfg, stackMemoryRegion);
-   ra._trace = comp->getOption(TR_TraceSA);
-
-   ra._useNew = !comp->getOption(TR_DisableIterativeSA);
-   if (ra.trace())
-      {
-      traceMsg(comp, "Blocks before Region Analysis:\n");
-      comp->getDebug()->print(comp->getOutFile(), cfg);
-      }
-
-   ra.createLeafStructures(cfg, stackMemoryRegion);
-
-   // Loop through the node set until there is only one node left - this is the
-   // root of the control tree.
-   //
-   TR_Structure *result = ra.findRegions(stackMemoryRegion);
-
-   return result;
-   }
-
 /**
  * Mainline for performing Region Analysis.
  */
@@ -389,6 +345,7 @@ TR_RegionStructure *TR_RegionAnalysis::findNaturalLoop(StructInfo &node,
    regionNodes.set(node._nodeIndex);
    nodesInPath.empty();
    bool cyclesFound = false;
+
    int32_t numBackEdges = 0;
 
    TR_BitVectorIterator cursor(node._pred);
@@ -399,6 +356,7 @@ TR_RegionStructure *TR_RegionAnalysis::findNaturalLoop(StructInfo &node,
          {
          // A back-edge has been found. Add its loop nodes to the region
          //
+
          if (_useNew)
             {
 	         addNaturalLoopNodesIterativeVersion(backEdgeNode, regionNodes, nodesInPath, cyclesFound, node._originalBlock);

compiler/optimizer/StructuralAnalysis.hpp

@@ -52,7 +52,6 @@ class TR_RegionAnalysis
 
    static TR_Structure *getRegions(TR::Compilation *);
    static TR_Structure *getRegions(TR::Compilation *, TR::ResolvedMethodSymbol *);
-   static TR_Structure *getRegions(TR::Compilation *, TR::CFG *);
 
    friend class TR_Debug;
 

compiler/optimizer/abstractinterpreter/AbsVisitor.hpp

@@ -22,9 +22,11 @@
 #ifndef ABS_VISITOR_INCL
 #define ABS_VISITOR_INCL
 
-#include "optimizer/CallInfo.hpp"
 #include "infra/vector.hpp"
-#include "optimizer/abstractinterpreter/AbsValue.hpp"
+
+class TR_CallSite;
+namespace TR { class Block; }
+namespace TR { class AbsValue; }
 
 namespace TR {
 
@@ -34,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) {}
+   virtual void visitCallSite(TR_CallSite* callSite, int32_t callerIndex, TR::Block* callBlock, TR::vector<TR::AbsValue*, TR::Region&>* arguments) = 0;
    };
 }
 

compiler/optimizer/abstractinterpreter/IDTBuilder.hpp

@@ -24,16 +24,16 @@
 #define TR_IDT_BUILDER_INCL
 
 #include "optimizer/abstractinterpreter/OMRIDTBuilder.hpp"
-#include "il/ResolvedMethodSymbol.hpp"
-#include "env/Region.hpp"
-#include "compile/Compilation.hpp"
-#include "optimizer/Inliner.hpp"
 
+namespace TR { class ResolvedMethodSymbol; }
+namespace TR { class Region; }
+namespace TR { class Compilation; }
+class TR_InlinerBase;
 
 namespace TR
 {
 
-class IDTBuilder : public OMR::IDTBuilderConnector
+class OMR_EXTENSIBLE IDTBuilder : public OMR::IDTBuilderConnector
    {
    public:
    IDTBuilder(TR::ResolvedMethodSymbol* symbol, int32_t budget, TR::Region& region, TR::Compilation* comp, TR_InlinerBase* inliner) :