From 6b039d0e62c563d2d217ece14acbd94c14a23eb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Petryka?= <35800402+MichalPetryka@users.noreply.github.com> Date: Thu, 18 Jun 2026 21:58:03 +0200 Subject: [PATCH] Fix PGO consistency asserts with Then only QMarks with likelyhood in AO Removed explicit Then handling in favour of canonicalizing to else first which fixes the issue --- src/coreclr/jit/morph.cpp | 55 +++++++++++++++------------------------ 1 file changed, 21 insertions(+), 34 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 8f32ee00f0d34c..752e259f4c1d4f 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -14807,17 +14807,31 @@ bool Compiler::fgExpandQmarkStmt(BasicBlock* block, Statement* stmt, bool onlyEa } #endif // DEBUG + GenTreeColon* colon = qmark->gtGetOp2()->AsColon(); + bool hasTrueExpr = !colon->ThenNode()->OperIs(GT_NOP); + bool hasFalseExpr = !colon->ElseNode()->OperIs(GT_NOP); + assert(hasTrueExpr || hasFalseExpr); // We expect to have at least one arm of the qmark! + + // canonicalize to else + if (hasTrueExpr && !hasFalseExpr) + { + qmark->gtOp1 = gtReverseCond(qmark->gtGetOp1()); + qmark->SetThenNodeLikelihood(qmark->ElseNodeLikelihood()); + + GenTree* op2 = colon->gtOp2; + colon->gtOp2 = colon->gtOp1; + colon->gtOp1 = op2; + hasTrueExpr = false; + hasFalseExpr = true; + } + assert(hasFalseExpr); // we should always have false now + // Retrieve the operands. + GenTree* trueExpr = colon->ThenNode(); + GenTree* falseExpr = colon->ElseNode(); GenTree* condExpr = qmark->gtGetOp1(); - GenTree* trueExpr = qmark->gtGetOp2()->AsColon()->ThenNode(); - GenTree* falseExpr = qmark->gtGetOp2()->AsColon()->ElseNode(); - assert(!varTypeIsFloating(condExpr->TypeGet())); - bool hasTrueExpr = !trueExpr->OperIs(GT_NOP); - bool hasFalseExpr = !falseExpr->OperIs(GT_NOP); - assert(hasTrueExpr || hasFalseExpr); // We expect to have at least one arm of the qmark! - // Create remainder, cond and "else" blocks. After this, the blocks are in this order: // block ... condBlock ... elseBlock ... remainderBlock // @@ -14895,33 +14909,6 @@ bool Compiler::fgExpandQmarkStmt(BasicBlock* block, Statement* stmt, bool onlyEa thenEdge->setLikelihood(thenLikelihood / 100.0); elseEdge->setLikelihood(elseLikelihood / 100.0); } - else if (hasTrueExpr) - { - // false - // S0 -->-- ~C -->-- T -->-- S1 - // | | - // +-->-------------+ - // bbj_cond(true) - // - // TODO: Remove unnecessary condition reversal - qmark->gtOp1 = condExpr = gtReverseCond(condExpr); - - const unsigned thenLikelihood = qmark->ThenNodeLikelihood(); - const unsigned elseLikelihood = qmark->ElseNodeLikelihood(); - - assert(condBlock->TargetIs(elseBlock)); - FlowEdge* const thenEdge = fgAddRefPred(remainderBlock, condBlock); - FlowEdge* const elseEdge = condBlock->GetTargetEdge(); - condBlock->SetCond(thenEdge, elseEdge); - - // Since we have no false expr, use the one we'd already created. - thenBlock = elseBlock; - elseBlock = nullptr; - - thenBlock->inheritWeightPercentage(condBlock, thenLikelihood); - thenEdge->setLikelihood(thenLikelihood / 100.0); - elseEdge->setLikelihood(elseLikelihood / 100.0); - } else if (hasFalseExpr) { // false