Expand "return condition" into "return condition ? true : false"

src/coreclr/jit/compiler.h

@@ -6265,6 +6265,8 @@ class Compiler
 
     PhaseStatus fgDetermineFirstColdBlock();
 
+    bool fgDedupReturnComparison(BasicBlock* block);
+
     bool fgIsForwardBranch(BasicBlock* bJump, BasicBlock* bDest, BasicBlock* bSrc = nullptr);
 
     bool fgUpdateFlowGraph(bool doTailDup = false, bool isPhase = false, bool doAggressiveCompaction = true);

src/coreclr/jit/fgdiagnostic.cpp

@@ -3275,6 +3275,11 @@ void Compiler::fgDebugCheckTypes(GenTree* tree)
                 assert(node->TypeIs(TYP_VOID) && "GT_NOP should be TYP_VOID.");
             }
 
+            if (node->OperIs(GT_JTRUE))
+            {
+                assert(node->TypeIs(TYP_VOID) && "GT_JTRUE should be TYP_VOID.");
+            }
+
             if (varTypeIsSmall(node))
             {
                 if (node->OperIs(GT_COMMA))

src/coreclr/jit/fgopt.cpp

@@ -5178,6 +5178,75 @@ PhaseStatus Compiler::fgUpdateFlowGraphPhase()
     return madeChanges ? PhaseStatus::MODIFIED_EVERYTHING : PhaseStatus::MODIFIED_NOTHING;
 }
 
+//-------------------------------------------------------------
+// fgDedupReturnComparison: Expands BBJ_RETURN CMP into BBJ_COND CMP with two
+//   BBJ_RETURN blocks ("return true" and "return false"). Such transformation
+//   helps other phases to focus only on BBJ_COND CMP.
+//
+// Arguments:
+//    block - the BBJ_RETURN block to convert into BBJ_COND CMP
+//
+// Returns:
+//    true if the block was converted into BBJ_COND CMP
+//
+bool Compiler::fgDedupReturnComparison(BasicBlock* block)
+{
+#ifdef JIT32_GCENCODER
+    // JIT32_GCENCODER has a hard limit on the number of epilogues, let's not add more.
+    return false;
+#endif
+
+    assert(block->KindIs(BBJ_RETURN));
+
+    // We're only interested in boolean returns
+    if ((info.compRetType != TYP_UBYTE) || (block == genReturnBB) || (block->lastStmt() == nullptr))
+    {
+        return false;
+    }
+
+    GenTree* rootNode = block->lastStmt()->GetRootNode();
+    if (!rootNode->OperIs(GT_RETURN) || !rootNode->gtGetOp1()->OperIsCmpCompare())
+    {
+        return false;
+    }
+
+    GenTree* cmp    = rootNode->gtGetOp1();
+    GenTree* cmpOp1 = cmp->gtGetOp1();
+    GenTree* cmpOp2 = cmp->gtGetOp2();
+
+    // The following check is purely to improve TP and handle some size regressions we fail to handle
+    // Eventually, we should remove it.
+    bool profitable = (cmpOp1->IsLocal() && cmpOp2->IsCnsIntOrI()) || (cmpOp2->IsLocal() && cmpOp1->IsCnsIntOrI());
+    if (!profitable)
+    {
+        return false;
+    }
+
+    cmp->gtFlags |= (GTF_RELOP_JMP_USED | GTF_DONT_CSE);
+    rootNode->ChangeOper(GT_JTRUE);
+    rootNode->ChangeType(TYP_VOID);
+
+    GenTree* retTrue  = gtNewOperNode(GT_RETURN, TYP_INT, gtNewTrue());
+    GenTree* retFalse = gtNewOperNode(GT_RETURN, TYP_INT, gtNewFalse());
+
+    // Create RETURN 1/0 blocks. We expect fgHeadTailMerge to handle them if there are similar returns.
+    DebugInfo   dbgInfo    = block->lastStmt()->GetDebugInfo();
+    BasicBlock* retTrueBb  = fgNewBBFromTreeAfter(BBJ_RETURN, block, retTrue, dbgInfo);
+    BasicBlock* retFalseBb = fgNewBBFromTreeAfter(BBJ_RETURN, block, retFalse, dbgInfo);
+
+    FlowEdge* trueEdge  = fgAddRefPred(retTrueBb, block);
+    FlowEdge* falseEdge = fgAddRefPred(retFalseBb, block);
+    block->SetCond(trueEdge, falseEdge);
+
+    // We might want to instrument 'return <cond>' too in the future. For now apply 50%/50%.
+    trueEdge->setLikelihood(0.5);
+    falseEdge->setLikelihood(0.5);
+    retTrueBb->inheritWeightPercentage(block, 50);
+    retFalseBb->inheritWeightPercentage(block, 50);
+
+    return true;
+}
+
 //-------------------------------------------------------------
 // fgUpdateFlowGraph: Removes any empty blocks, unreachable blocks, and redundant jumps.
 // Most of those appear after dead store removal and folding of conditionals.
@@ -5274,6 +5343,13 @@ bool Compiler::fgUpdateFlowGraph(bool doTailDuplication /* = false */,
             bDest      = nullptr;
             bFalseDest = nullptr;
 
+            if (doTailDuplication && block->KindIs(BBJ_RETURN) && fgDedupReturnComparison(block))
+            {
+                change   = true;
+                modified = true;
+                bNext    = block->Next();
+            }
+
             if (block->KindIs(BBJ_ALWAYS))
             {
                 bDest = block->GetTarget();
@@ -6323,6 +6399,12 @@ PhaseStatus Compiler::fgHeadTailMerge(bool early)
         madeChanges |= fgHeadMerge(block, early);
     }
 
+    if (madeChanges && !early)
+    {
+        // Clean up potential unconditional jumps produced by tail merging
+        //fgUpdateFlowGraph();
+    }
+
     // If we altered flow, reset fgModified. Given where we sit in the
     // phase list, flow-dependent side data hasn't been built yet, so
     // nothing needs invalidation.

src/coreclr/jit/ifconversion.cpp

@@ -704,9 +704,27 @@ bool OptIfConversionDsc::optIfConvert()
         selectType       = genActualType(m_thenOperation.node);
     }
 
-    // Create a select node.
-    GenTreeConditional* select =
-        m_comp->gtNewConditionalNode(GT_SELECT, m_cond, selectTrueInput, selectFalseInput, selectType);
+    GenTree* select = nullptr;
+    if (selectTrueInput->TypeIs(TYP_INT) && selectFalseInput->TypeIs(TYP_INT))
+    {
+        if (selectTrueInput->IsIntegralConst(1) && selectFalseInput->IsIntegralConst(0))
+        {
+            // compare ? true : false  -->  compare
+            select = m_cond;
+        }
+        else if (selectTrueInput->IsIntegralConst(0) && selectFalseInput->IsIntegralConst(1))
+        {
+            // compare ? false : true  -->  reversed_compare
+            select = m_comp->gtReverseCond(m_cond);
+        }
+    }
+
+    if (select == nullptr)
+    {
+        // Create a select node
+        select = m_comp->gtNewConditionalNode(GT_SELECT, m_cond, selectTrueInput, selectFalseInput, selectType);
+    }
+
     m_thenOperation.node->AddAllEffectsFlags(select);
 
     // Use the select as the source of the Then operation.