diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 740182cdf2b7e..85a0b2a05ee9c 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -6311,6 +6311,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); diff --git a/src/coreclr/jit/fgdiagnostic.cpp b/src/coreclr/jit/fgdiagnostic.cpp index 1d600864c2000..0e56d79cd5c7a 100644 --- a/src/coreclr/jit/fgdiagnostic.cpp +++ b/src/coreclr/jit/fgdiagnostic.cpp @@ -3260,6 +3260,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)) diff --git a/src/coreclr/jit/fgopt.cpp b/src/coreclr/jit/fgopt.cpp index 231eeb22c4824..4c1056a56c5e2 100644 --- a/src/coreclr/jit/fgopt.cpp +++ b/src/coreclr/jit/fgopt.cpp @@ -5511,6 +5511,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 ' 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. @@ -5607,6 +5676,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(); @@ -6683,6 +6759,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. diff --git a/src/coreclr/jit/ifconversion.cpp b/src/coreclr/jit/ifconversion.cpp index 6008bb432550b..178a4c3f670cb 100644 --- a/src/coreclr/jit/ifconversion.cpp +++ b/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.