Merge pull request #6221 from pgavlin/RemoveDeadCodeInRationalize

Remove dead code in Rationalize.

2 files changed, 3 insertions, 309 deletions

diff --git a/src/jit/rationalize.cpp b/src/jit/rationalize.cpp
index a321c55f23..7f8576e890 100644
--- a/src/jit/rationalize.cpp
+++ b/src/jit/rationalize.cpp
@@ -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
diff --git a/src/jit/rationalize.h b/src/jit/rationalize.h
index 90f39c32d0..6d4f54a9c2 100644
--- a/src/jit/rationalize.h
+++ b/src/jit/rationalize.h
@@ -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);