Remove dead code in Rationalize.
authorPat Gavlin <pagavlin@microsoft.com>
Mon, 11 Jul 2016 20:38:30 +0000 (13:38 -0700)
committerPat Gavlin <pagavlin@microsoft.com>
Mon, 11 Jul 2016 20:38:30 +0000 (13:38 -0700)
Most of the removed code deals with QMARK rewriting. These nodes
are now rewritten as part of fgMorph, so the rationalizer does not
need to deal with them (other than asserting that they do not
exist).

Other dead code:
- The `Rationalizations` type
- A few split-related declarations and helpers

src/jit/rationalize.cpp
src/jit/rationalize.h

index a321c55..7f8576e 100644 (file)
@@ -70,134 +70,6 @@ struct SplitData
     
     bool continueSubtrees; // whether to continue after splitting off a tree (in pre-order)
 };
-   
-//------------------------------------------------------------------------------
-// RewriteOneQuestion - split a question op into three parts: the test and branch, 
-//                      and true and false parts, with accompanying flow
-//------------------------------------------------------------------------------
-
-Location Rationalizer::RewriteOneQuestion(BasicBlock *block, GenTree *qmarkTree, GenTree *stmt, GenTree *dst)
-{
-    // First create all the blocks this is going to turn into.
-    // We end the current block here and insert a diamond
-    // consisting of the then/else blocks and the remainder, which is
-    // the point where flow merges back and the rest of the current block will end up
-
-    // TODO-Cleanup: avoid creating one of these blocks if that part of the qmark is a NOP
-    BasicBlock* remainderBlock = comp->fgSplitBlockAfterStatement(block, stmt);
-    BasicBlock*      elseBlock = comp->fgSplitBlockAfterStatement(block, stmt);
-    BasicBlock*      thenBlock = comp->fgSplitBlockAfterStatement(block, stmt);
-
-    // wire up the flow between the blocks and adjust their preds
-    block->bbJumpKind = BBJ_COND;
-    block->bbJumpDest = elseBlock;
-        
-    thenBlock->bbJumpKind = BBJ_ALWAYS;
-    thenBlock->bbJumpDest = remainderBlock;
-    thenBlock->bbFlags &= ~BBF_JMP_TARGET;
-
-    elseBlock->bbJumpKind = BBJ_NONE; // falls through to remainder
-    elseBlock->bbFlags |= (BBF_JMP_TARGET | BBF_HAS_LABEL);
-    comp->fgAddRefPred(elseBlock, block);
-
-    //JITDUMP("after splitting Q1");
-    //dumpMethod();
-
-    remainderBlock->bbFlags |= (BBF_JMP_TARGET | BBF_HAS_LABEL);
-    comp->fgAddRefPred(remainderBlock, thenBlock);
-    comp->fgRemoveRefPred(elseBlock, thenBlock);
-
-    //JITDUMP("after splitting Q2");
-    //dumpMethod();
-
-    // remove flag marking this as a question conditional
-    GenTree *conditionExpr = qmarkTree->gtGetOp1();
-    assert(conditionExpr->gtFlags & GTF_RELOP_QMARK);
-    conditionExpr->gtFlags &= ~GTF_RELOP_QMARK;
-    
-    comp->gtReverseCond(conditionExpr);
-    
-    // Wire up the jmp part.
-    // Note that, unlike later Rationalizer passes, the QMarks are done prior to the comma
-    // processing, and therefore prior to the creation of embedded statements.
-    // So we can safely re-sequence.
-    GenTree *jmpStmt = comp->fgNewStmtFromTree(comp->gtNewOperNode(GT_JTRUE, TYP_VOID, qmarkTree->gtGetOp1()), qmarkTree->gtStmt.gtStmtILoffsx);
-    comp->fgInsertStmtAtEnd(block, jmpStmt);
-
-    DBEXEC(TRUE, ValidateStatement(Location(jmpStmt, block)));
-
-    //JITDUMP("before splitting Q3");
-    //dumpMethod();
-    //DBEXEC(TRUE, comp->fgDebugCheckBBlist());
-
-    GenTree *trueExpr = qmarkTree->gtGetOp2()->gtGetOp2();
-    GenTree *falseExpr = qmarkTree->gtGetOp2()->gtGetOp1();
-    IL_OFFSETX ilOffset = stmt->gtStmt.gtStmtILoffsx;
-    comp->fgRemoveStmt(block, stmt, false);
-
-    unsigned lclNum = 0;
-    bool resultUsed = false;
-
-    // if the dst of the qmark was a local then we can write directly to it
-    // otherwise make a temp and then do the indir/field/whatever writeback
-    if (dst && dst->IsLocal())
-    {
-        resultUsed = true;
-        lclNum = dst->gtLclVarCommon.gtLclNum;
-
-        // Increment its lvRefCnt and lvRefCntWtd
-        comp->lvaTable[lclNum].incRefCnts(block->getBBWeight(comp), comp);
-    }
-    else if (qmarkTree->TypeGet() != TYP_VOID)
-    {
-        resultUsed = true; // just guessing here
-        lclNum = comp->lvaGrabTemp(true DEBUGARG("lower question"));
-        comp->lvaTable[lclNum].lvType = qmarkTree->TypeGet();
-
-        // Increment its lvRefCnt and lvRefCntWtd twice, one for the def and one for the use
-        comp->lvaTable[lclNum].incRefCnts(block->getBBWeight(comp), comp);
-        comp->lvaTable[lclNum].incRefCnts(block->getBBWeight(comp), comp);
-    }
-
-    // assign the trueExpr into the dst or tmp, insert in thenBlock
-    if (trueExpr->OperGet() != GT_NOP)
-    {
-        if (trueExpr->TypeGet() != TYP_VOID)
-        {
-            assert(resultUsed);
-            trueExpr = CreateTempAssignment(comp, lclNum, trueExpr);
-        }
-        GenTree *trueStmt = comp->fgNewStmtFromTree(trueExpr, thenBlock, ilOffset);
-        comp->fgInsertStmtAtEnd(thenBlock, trueStmt);
-    }
-
-    // assign the falseExpr into the dst or tmp, insert in elseBlock
-    if (falseExpr->OperGet() != GT_NOP)
-    {
-        if (falseExpr->TypeGet() != TYP_VOID)
-        {
-            assert(resultUsed);
-            falseExpr = CreateTempAssignment(comp, lclNum, falseExpr);
-        }
-        GenTree *falseStmt = comp->fgNewStmtFromTree(falseExpr, elseBlock, ilOffset);
-        comp->fgInsertStmtAtEnd(elseBlock, falseStmt);
-    }
-
-    // if the dst is a local we have just written it out 
-    // but if not (like an indir or something) then we copy from the temp we allocated
-    if (dst && !dst->IsLocal())
-    {
-        GenTree *writeback = comp->gtNewAssignNode(dst, comp->gtNewLclvNode(lclNum, qmarkTree->TypeGet()));
-        GenTree *writeStmt = comp->fgNewStmtFromTree(writeback, remainderBlock, ilOffset);
-        comp->fgInsertStmtAtBeg(remainderBlock, writeStmt);
-    }
-
-    //JITDUMP("after splitting all");
-    //dumpMethod();
-
-    return Location(jmpStmt, block);
-}
 
 //------------------------------------------------------------------------------
 // isNodeCallArg - given a context (stack of parent nodes), determine if the TOS is an arg to a call
@@ -239,68 +111,6 @@ GenTree *isNodeCallArg(ArrayStack<GenTree *> *parentStack)
 }
 
 //------------------------------------------------------------------------------
-// shouldSplitRationalPre - invoked in preorder in a tree walk to determine if 
-//                          we should split at this point in the tree, and at this 
-//                          point in the walk
-//------------------------------------------------------------------------------
-
-bool shouldSplitRationalPre(GenTree *tree, GenTree *parent, 
-                            Compiler::fgWalkData *data)
-{
-    if (!parent || parent->gtOper == GT_STMT)
-        return false;
-    
-    // the only thing we split in preorder are qmark ops
-    if ((tree->OperGet() == GT_QMARK)
-        && parent->gtOper != GT_ASG)
-        return true;
-
-    return false;
-}
-
-
-//------------------------------------------------------------------------------
-// shouldSplitRationalPost - invoked in postorder in a tree walk to determine if 
-//                           we should split at this point in the tree, and at this 
-//                           point in the walk
-//------------------------------------------------------------------------------
-
-bool shouldSplitRationalPost(GenTree *tree, GenTree *parent, 
-                             Compiler::fgWalkData *data)
-{
-    if (!parent || parent->gtOper == GT_STMT)
-        return false;
-    
-    SplitData *splitData = (SplitData *) data->pCallbackData;
-    auto phase = splitData->thisPhase;
-
-#ifndef _TARGET_AMD64_
-    if (tree->OperIsAssignment() 
-        // late args are not truly embedded assigns... or at least they are very limited ones
-        && !( isNodeCallArg(data->parentStack) && (tree->gtFlags & GTF_LATE_ARG)))
-    {
-        JITDUMP("splitting at assignment\n");
-        return true;
-    }
-
-    if (tree->IsCall() && !parent->OperIsAssignment())
-    {
-        JITDUMP("splitting a nested call\n");
-        return true;
-    }
-#endif
-
-    if ((tree->OperGet() == GT_QMARK)
-        && parent->gtOper != GT_ASG)
-    {
-        JITDUMP("splitting a question\n");
-        return true;
-    }
-
-    return false;
-}
-
-//------------------------------------------------------------------------------
 // fgSpliceTreeBefore - insert the given subtree 'tree' as a top level statement 
 //                      placed before top level statement 'insertionPoint'
 //------------------------------------------------------------------------------
@@ -714,40 +524,6 @@ void copyFlags(GenTree *dst, GenTree *src, unsigned mask)
 }
 
 
-//------------------------------------------------------------------------------
-// RewriteQuestions - transform qmark ops, expanding them into multiple blocks
-//                    They should all be at the top level or immediately under an
-//                    assignment at this point
-//------------------------------------------------------------------------------
-
-Location Rationalizer::RewriteQuestions(Location loc)
-{
-    GenTree *topNode = loc.tree->gtStmt.gtStmtExpr;
-    // we should have things broken up so all questions are at top level
-    if (topNode->gtOper == GT_QMARK)
-    {
-        loc = RewriteOneQuestion(loc.block, topNode, loc.tree, NULL);
-        return loc;
-    }
-    else if (topNode->gtOper == GT_ASG)
-    {
-        // hope there's never a QMARK on the LHS
-        assert(topNode->gtGetOp1()->gtOper != GT_QMARK);
-
-        GenTree *questionOp = topNode->gtGetOp2();
-        if (questionOp->gtOper != GT_QMARK)
-            return loc;
-
-        loc = RewriteOneQuestion(loc.block, questionOp, loc.tree, topNode->gtGetOp1());
-        return loc;
-    }
-    else
-    {
-        return loc;
-    }
-}
-
-
 //--------------------------------------------------------------------------------------
 // RewriteTopLevelComma - split a top-level comma into two top level statements.
 //                                returns (as out params) the two new locations
@@ -784,8 +560,6 @@ Location Rationalizer::TreeTransformRationalization(Location loc)
 top:
     assert(loc.tree);
 
-    DBEXEC(TRUE, didSplit = false);
-
     JITDUMP("Tree Transform Rationalization: BB%02u\n", loc.block->bbNum);
     DISPTREE(loc.tree);
     JITDUMP("\n");
@@ -804,14 +578,6 @@ top:
     }
 
     DBEXEC(TRUE, loc.Validate());
-
-#ifdef LEGACY_BACKEND
-    if (comp->compQmarkUsed)
-    {
-        loc = RewriteQuestions(loc);
-    }
-#endif // LEGACY_BACKEND
-
     DBEXEC(TRUE, ValidateStatement(loc));
 
     loc = RewriteSimpleTransforms(loc);
@@ -822,8 +588,6 @@ top:
     JITDUMP("\n");
 
     DuplicateCommaProcessOneTree(comp, this, loc.block, loc.tree);
-    
-    DBEXEC(didSplit, comp->fgDebugCheckBBlist());
             
     return loc;
 }
@@ -1903,31 +1667,6 @@ Compiler::fgWalkResult Rationalizer::SimpleTransformHelper(GenTree **ppTree, Com
         JITDUMP("\n");
         return SimpleTransformHelper(ppTree, data);
     }
-    else if (tree->gtOper == GT_QMARK)
-    {
-        // only certain forms of qmarks are allowed
-        // qmark(conditionExpr, 1, 0) is equivalent to conditionExpr
-        GenTree* colonNode = tree->gtOp.gtOp2;
-        GenTree* thenNode = colonNode->AsColon()->ThenNode();
-        GenTree* elseNode = colonNode->AsColon()->ElseNode();
-        assert(thenNode->IsCnsIntOrI());
-        assert(elseNode->IsCnsIntOrI());
-        assert(thenNode->gtIntConCommon.IconValue() == 1);
-        assert(elseNode->gtIntConCommon.IconValue() == 0);
-
-        Compiler::fgSnipNode(tmpState->root->AsStmt(), elseNode);
-        Compiler::fgSnipNode(tmpState->root->AsStmt(), thenNode);
-        Compiler::fgSnipNode(tmpState->root->AsStmt(), colonNode);
-        Compiler::fgSnipNode(tmpState->root->AsStmt(), tree);
-
-        *ppTree = tree->gtOp.gtOp1;
-        (*ppTree)->gtFlags &= ~GTF_RELOP_QMARK;
-        comp->fgFixupIfCallArg(data->parentStack, tree, *ppTree);
-
-        JITDUMP("Rewriting GT_QMARK(conditionExpr, 1, 0) to conditionExpr:\n");
-        DISPTREE(*ppTree);
-        JITDUMP("\n");
-    }
 #ifdef _TARGET_XARCH_
     else if (tree->gtOper == GT_CLS_VAR)
     {
@@ -2174,6 +1913,9 @@ void Rationalizer::SanityCheck()
                  tree; 
                  tree = tree->gtNext)
             {
+                // QMARK nodes should have been removed before this phase.
+                assert(tree->OperGet() != GT_QMARK);
+
                 if (tree->OperGet() == GT_ASG)
                 {
                     if (tree->gtGetOp1()->OperGet() == GT_LCL_VAR)
@@ -2206,34 +1948,6 @@ void Rationalizer::DoPhase()
     comp->compCurBB = NULL;
     comp->fgOrder = Compiler::FGOrderLinear;
 
-    // If the first block is BBF_INTERNAL, it is special.  Zero-inits must be placed in 
-    // this block, and it must fall through to the next block.  
-    // If there is a question op in the block (as can be the case with a just-my-code helper)
-    // then the rationalizer will expand that to flow and break the fallthrough invariant.  
-    // However, we need to still keep the zero-inits in the original block, so only split before
-    // the statement containing the qmark.
-
-    if (comp->fgFirstBB->bbFlags & BBF_INTERNAL)
-    {
-        BasicBlock* const block = comp->fgFirstBB;
-        for (GenTree* stmt = block->bbTreeList; stmt; stmt = stmt->gtNext)
-        {
-            GenTreePtr node;
-            foreach_treenode_execution_order(node, stmt)
-            {
-                if (node->gtOper == GT_QMARK)
-                {
-                    BasicBlock* newBlock;
-                    if (stmt == block->bbTreeList)
-                        newBlock = comp->fgSplitBlockAtBeginning(comp->fgFirstBB);
-                    else
-                        newBlock = comp->fgSplitBlockAfterStatement(block, stmt);
-                    newBlock->bbFlags &= ~BBF_INTERNAL;
-                }
-            }
-        }
-    }
-
     use     = hashBv::Create(this->comp); // is used
     usedef  = hashBv::Create(this->comp); // is used and then defined
     rename  = hashBv::Create(this->comp); // is used, defined and used again
index 90f39c3..6d4f54a 100644 (file)
@@ -5,14 +5,6 @@
 //===============================================================================
 #include "phase.h"
 
-enum Rationalizations
-{
-    Questions = 0x1,
-    NestedCalls = 0x2,
-    NestedAssigns = 0x4,
-    Commas = 0x8
-};
-
 //------------------------------------------------------------------------------
 // Location - (tree, block) tuple is minimum context required to manipulate trees in the JIT
 //------------------------------------------------------------------------------
@@ -99,11 +91,6 @@ class Rationalizer : public Phase
     //===============================================================================
     // Data members
 
-#ifdef DEBUG
-    // keep track of whether a split happened so we can avoid expensive debug checks
-    bool didSplit;
-#endif
-
     // used for renaming updated variables
     hashBv *use;
     hashBv *usedef;
@@ -115,7 +102,6 @@ class Rationalizer : public Phase
     // Methods
 public:
     Rationalizer(Compiler* comp);
-    Location TreeSplitRationalization     (Location loc);
     Location TreeTransformRationalization (Location loc);
 
     void RenameUpdatedVars(Location loc);
@@ -145,7 +131,6 @@ private:
     static bool                   RewriteArrElem       (GenTree** ppTree, Compiler::fgWalkData* data);
 
     static Compiler::fgWalkResult SimpleTransformHelper(GenTree** ppTree, Compiler::fgWalkData* data);
-    static Compiler::fgWalkResult QuestionHelper       (GenTree** ppTree, Compiler::fgWalkData* data);
 
     static void       DuplicateCommaProcessOneTree (Compiler* comp, Rationalizer* irt, BasicBlock* block, GenTree* tree);
 
@@ -159,13 +144,8 @@ private:
                                                     unsigned lclNum,
                                                     GenTreePtr rhs);
 
-    // Question related
-    Location   RewriteQuestions         (Location loc);
     void       RewriteTopLevelComma     (Location loc, Location* out1, Location* out2);
     Location   RewriteSimpleTransforms  (Location loc);
-    Location   RewriteOneQuestion       (BasicBlock* block, GenTree* op, GenTree* stmt, GenTree* dest);
-    void       RewriteQuestions         (BasicBlock* block, GenTree* stmt);
-    bool       BreakFirstLevelQuestions (BasicBlock* block, GenTree* tree);
     
     // SIMD related transformations
     static void RewriteObj(GenTreePtr* ppTree, Compiler::fgWalkData* data);