From ea2f3d52e58da3b5f1ad3caf2c827a9acb22d71b Mon Sep 17 00:00:00 2001
From: Alan Hayward <alan.hayward@arm.com>
Date: Fri, 15 Jul 2022 11:21:11 +0100
Subject: [PATCH 01/38] Arm64: Add If conversion pass

---
 src/coreclr/jit/assertionprop.cpp |  16 ++
 src/coreclr/jit/compiler.cpp      |   9 ++
 src/coreclr/jit/compiler.h        |  10 ++
 src/coreclr/jit/compphases.h      |   1 +
 src/coreclr/jit/gentree.cpp       | 121 ++++++++++++++
 src/coreclr/jit/jitconfigvalues.h |   1 +
 src/coreclr/jit/morph.cpp         |  16 ++
 src/coreclr/jit/optimizer.cpp     | 261 ++++++++++++++++++++++++++++++
 src/coreclr/jit/valuenum.cpp      |  21 +++
 9 files changed, 456 insertions(+)

diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp
index a066b23403609..8ca2f4c6f8dc7 100644
--- a/src/coreclr/jit/assertionprop.cpp
+++ b/src/coreclr/jit/assertionprop.cpp
@@ -4388,6 +4388,18 @@ GenTree* Compiler::optAssertionPropLocal_RelOp(ASSERT_VALARG_TP assertions, GenT
     return optAssertionProp_Update(op2, tree, stmt);
 }
 
+/*****************************************************************************
+ *
+ *  Given a tree ConditionalOp, check using standard morph function.
+ */
+
+GenTree* Compiler::optAssertionProp_ConditionalOp(ASSERT_VALARG_TP assertions, GenTree* tree, Statement* stmt)
+{
+    assert(tree->OperIsConditional());
+    GenTree* newTree = fgMorphTree(tree);
+    return optAssertionProp_Update(newTree, tree, stmt);
+}
+
 //------------------------------------------------------------------------
 // optAssertionProp_Cast: Propagate assertion for a cast, possibly removing it.
 //
@@ -5074,6 +5086,9 @@ GenTree* Compiler::optAssertionProp(ASSERT_VALARG_TP assertions, GenTree* tree,
             }
             return nullptr;
 
+        case GT_SELECT:
+            return optAssertionProp_ConditionalOp(assertions, tree, stmt);
+
         default:
             return nullptr;
     }
@@ -6011,6 +6026,7 @@ Compiler::fgWalkResult Compiler::optVNConstantPropCurStmt(BasicBlock* block, Sta
         case GT_NEG:
         case GT_CAST:
         case GT_INTRINSIC:
+        case GT_SELECT:
             break;
 
         case GT_JTRUE:
diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp
index 1042c1800324f..b6389d708f8f8 100644
--- a/src/coreclr/jit/compiler.cpp
+++ b/src/coreclr/jit/compiler.cpp
@@ -4740,6 +4740,7 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
         bool doCse           = true;
         bool doAssertionProp = true;
         bool doRangeAnalysis = true;
+        bool doIfConversion  = true;
         int  iterations      = 1;
 
 #if defined(OPT_CONFIG)
@@ -4752,6 +4753,7 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
         doCse           = doValueNum;
         doAssertionProp = doValueNum && (JitConfig.JitDoAssertionProp() != 0);
         doRangeAnalysis = doAssertionProp && (JitConfig.JitDoRangeAnalysis() != 0);
+        doIfConversion  = doIfConversion && (JitConfig.JitDoIfConversion() != 0);
 
         if (opts.optRepeat)
         {
@@ -4820,6 +4822,13 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
                 DoPhase(this, PHASE_ASSERTION_PROP_MAIN, &Compiler::optAssertionPropMain);
             }
 
+            if (doIfConversion)
+            {
+                // If conversion
+                //
+                DoPhase(this, PHASE_IF_CONVERSION, &Compiler::optIfConversion);
+            }
+
             if (doRangeAnalysis)
             {
                 // Bounds check elimination via range analysis
diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h
index 03910bbb60340..104cf54ab220f 100644
--- a/src/coreclr/jit/compiler.h
+++ b/src/coreclr/jit/compiler.h
@@ -2359,6 +2359,9 @@ class Compiler
     GenTreeLclVar* gtNewLclVarAddrNode(unsigned lclNum, var_types type = TYP_I_IMPL);
     GenTreeLclFld* gtNewLclFldAddrNode(unsigned lclNum, unsigned lclOffs, var_types type = TYP_I_IMPL);
 
+    GenTreeConditional* gtNewConditionalNode(
+        genTreeOps oper, GenTree* cond, GenTree* op1, GenTree* op2, var_types type);
+
 #ifdef FEATURE_SIMD
     GenTreeSIMD* gtNewSIMDNode(
         var_types type, GenTree* op1, SIMDIntrinsicID simdIntrinsicID, CorInfoType simdBaseJitType, unsigned simdSize);
@@ -2795,6 +2798,7 @@ class Compiler
     GenTree* gtFoldExprSpecial(GenTree* tree);
     GenTree* gtFoldBoxNullable(GenTree* tree);
     GenTree* gtFoldExprCompare(GenTree* tree);
+    GenTree* gtFoldExprConditional(GenTree* tree);
     GenTree* gtCreateHandleCompare(genTreeOps             oper,
                                    GenTree*               op1,
                                    GenTree*               op2,
@@ -6091,6 +6095,7 @@ class Compiler
     void optEnsureUniqueHead(unsigned loopInd, weight_t ambientWeight);
     PhaseStatus optUnrollLoops(); // Unrolls loops (needs to have cost info)
     void        optRemoveRedundantZeroInits();
+    void        optIfConversion(); // If conversion
 
 protected:
     // This enumeration describes what is killed by a call.
@@ -6474,6 +6479,10 @@ class Compiler
 
     bool optInvertWhileLoop(BasicBlock* block);
 
+    bool optIfConvert(BasicBlock* block);
+    bool optIfConvertSelect(GenTree* original_condition, GenTree* asg_node);
+    bool optIfConvertCCmp(GenTree* original_condition, GenTree* asg_node);
+
 private:
     static bool optIterSmallOverflow(int iterAtExit, var_types incrType);
     static bool optIterSmallUnderflow(int iterAtExit, var_types decrType);
@@ -7376,6 +7385,7 @@ class Compiler
     GenTree* optAssertionProp_Cast(ASSERT_VALARG_TP assertions, GenTreeCast* cast, Statement* stmt);
     GenTree* optAssertionProp_Call(ASSERT_VALARG_TP assertions, GenTreeCall* call, Statement* stmt);
     GenTree* optAssertionProp_RelOp(ASSERT_VALARG_TP assertions, GenTree* tree, Statement* stmt);
+    GenTree* optAssertionProp_ConditionalOp(ASSERT_VALARG_TP assertions, GenTree* tree, Statement* stmt);
     GenTree* optAssertionProp_Comma(ASSERT_VALARG_TP assertions, GenTree* tree, Statement* stmt);
     GenTree* optAssertionProp_BndsChk(ASSERT_VALARG_TP assertions, GenTree* tree, Statement* stmt);
     GenTree* optAssertionPropGlobal_RelOp(ASSERT_VALARG_TP assertions, GenTree* tree, Statement* stmt);
diff --git a/src/coreclr/jit/compphases.h b/src/coreclr/jit/compphases.h
index a67f6d52d9fa9..b2912cfeac43a 100644
--- a/src/coreclr/jit/compphases.h
+++ b/src/coreclr/jit/compphases.h
@@ -86,6 +86,7 @@ CompPhaseNameMacro(PHASE_OPTIMIZE_VALNUM_CSES,       "Optimize Valnum CSEs",
 CompPhaseNameMacro(PHASE_VN_COPY_PROP,               "VN based copy prop",             "CP-PROP",    false, -1, false)
 CompPhaseNameMacro(PHASE_OPTIMIZE_BRANCHES,          "Redundant branch opts",          "OPT-BR",     false, -1, false)
 CompPhaseNameMacro(PHASE_ASSERTION_PROP_MAIN,        "Assertion prop",                 "AST-PROP",   false, -1, false)
+CompPhaseNameMacro(PHASE_IF_CONVERSION,              "If conversion",                  "IF_CONV",    false, -1, false)
 CompPhaseNameMacro(PHASE_OPT_UPDATE_FLOW_GRAPH,      "Update flow graph opt pass",     "UPD-FG-O",   false, -1, false)
 CompPhaseNameMacro(PHASE_COMPUTE_EDGE_WEIGHTS2,      "Compute edge weights (2, false)","EDG-WGT2",   false, -1, false)
 CompPhaseNameMacro(PHASE_INSERT_GC_POLLS,            "Insert GC Polls",                "GC-POLLS",   false, -1, true)
diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp
index dc0b11d6571eb..7c15869d101b6 100644
--- a/src/coreclr/jit/gentree.cpp
+++ b/src/coreclr/jit/gentree.cpp
@@ -7360,6 +7360,17 @@ GenTreeLclFld* Compiler::gtNewLclFldAddrNode(unsigned lclNum, unsigned lclOffs,
     return node;
 }
 
+GenTreeConditional* Compiler::gtNewConditionalNode(
+    genTreeOps oper, GenTree* cond, GenTree* op1, GenTree* op2, var_types type)
+{
+    assert(GenTree::OperIsConditional(oper));
+    GenTreeConditional* node = new (this, oper) GenTreeConditional(oper, type, cond, op1, op2);
+    node->gtFlags |= (cond->gtFlags & GTF_ALL_EFFECT);
+    node->gtFlags |= (op1->gtFlags & GTF_ALL_EFFECT);
+    node->gtFlags |= (op2->gtFlags & GTF_ALL_EFFECT);
+    return node;
+}
+
 GenTreeLclFld* Compiler::gtNewLclFldNode(unsigned lnum, var_types type, unsigned offset)
 {
     GenTreeLclFld* node = new (this, GT_LCL_FLD) GenTreeLclFld(GT_LCL_FLD, type, lnum, offset);
@@ -12587,6 +12598,10 @@ GenTree* Compiler::gtFoldExpr(GenTree* tree)
 
     if (!(kind & GTK_SMPOP))
     {
+        if (tree->OperIsConditional())
+        {
+            return gtFoldExprConditional(tree);
+        }
         return tree;
     }
 
@@ -12855,6 +12870,112 @@ GenTree* Compiler::gtFoldExprCompare(GenTree* tree)
     return cons;
 }
 
+/*****************************************************************************
+ *
+ * Some conditionals can be folded:
+ *   SELECT TRUE  X Y  ->  X
+ *   SELECT FALSE X Y  ->  Y
+ *   SELECT COND X X   ->  X
+ *
+ */
+
+GenTree* Compiler::gtFoldExprConditional(GenTree* tree)
+{
+    GenTree* cond = tree->AsConditional()->gtCond;
+    GenTree* op1  = tree->AsConditional()->gtOp1;
+    GenTree* op2  = tree->AsConditional()->gtOp2;
+
+    assert(tree->OperIsConditional());
+
+    // Check for a constant conditional
+    if (cond->OperIsConst())
+    {
+        // Constant conditions must be folded away.
+
+        JITDUMP("\nFolding conditional op with constant condition:\n");
+        DISPTREE(tree);
+
+        assert(cond->TypeGet() == TYP_INT);
+        assert((tree->gtFlags & GTF_SIDE_EFFECT & ~GTF_ASG) == 0);
+        assert((tree->gtFlags & GTF_ORDER_SIDEEFF) == 0);
+
+        GenTree* replacement = nullptr;
+        if (cond->AsIntConCommon()->IntegralValue() == 0)
+        {
+            JITDUMP("Bashed to false path:\n");
+            replacement = op2;
+        }
+        else
+        {
+            // Condition should never be a constant other than 0 or 1
+            assert(cond->AsIntConCommon()->IntegralValue() == 1);
+
+            JITDUMP("Bashed to true path:\n");
+            replacement = op1;
+        }
+
+        if (fgGlobalMorph)
+        {
+            fgMorphTreeDone(replacement);
+        }
+        else
+        {
+            replacement->gtNext = tree->gtNext;
+            replacement->gtPrev = tree->gtPrev;
+        }
+        DISPTREE(replacement);
+        JITDUMP("\n");
+
+        // If we bashed to a compare, try to fold that.
+        if (replacement->OperIsCmpCompare())
+        {
+            return gtFoldExprCompare(replacement);
+        }
+
+        return replacement;
+    }
+
+    assert(cond->OperIsCmpCompare());
+
+    if (((tree->gtFlags & GTF_SIDE_EFFECT) != 0) || !GenTree::Compare(op1, op2, true))
+    {
+        // No folding.
+        return tree;
+    }
+
+    // GTF_ORDER_SIDEEFF here may indicate volatile subtrees.
+    // Or it may indicate a non-null assertion prop into an indir subtree.
+    if ((tree->gtFlags & GTF_ORDER_SIDEEFF) != 0)
+    {
+        // If op1 is "volatile" and op2 is not, we can still fold.
+        const bool op1MayBeVolatile = (op1->gtFlags & GTF_ORDER_SIDEEFF) != 0;
+        const bool op2MayBeVolatile = (op2->gtFlags & GTF_ORDER_SIDEEFF) != 0;
+
+        if (!op1MayBeVolatile || op2MayBeVolatile)
+        {
+            // No folding.
+            return tree;
+        }
+    }
+
+    JITDUMP("Bashed to first of two identical paths:\n");
+    GenTree* replacement = op1;
+
+    if (fgGlobalMorph)
+    {
+        fgMorphTreeDone(replacement);
+    }
+    else
+    {
+        replacement->gtNext = tree->gtNext;
+        replacement->gtPrev = tree->gtPrev;
+    }
+    DISPTREE(replacement);
+    JITDUMP("\n");
+
+    return replacement;
+}
+
 //------------------------------------------------------------------------
 // gtCreateHandleCompare: generate a type handle comparison
 //
diff --git a/src/coreclr/jit/jitconfigvalues.h b/src/coreclr/jit/jitconfigvalues.h
index fd32276e8b5cf..343657eb8c3c5 100644
--- a/src/coreclr/jit/jitconfigvalues.h
+++ b/src/coreclr/jit/jitconfigvalues.h
@@ -411,6 +411,7 @@ CONFIG_INTEGER(JitDoValueNumber, W("JitDoValueNumber"), 1) // Perform value numb
 
 CONFIG_METHODSET(JitOptRepeat, W("JitOptRepeat"))            // Runs optimizer multiple times on the method
 CONFIG_INTEGER(JitOptRepeatCount, W("JitOptRepeatCount"), 2) // Number of times to repeat opts when repeating
+CONFIG_INTEGER(JitDoIfConversion, W("JitDoIfConversion"), 1) // Perform If conversion
 #endif                                                       // defined(OPT_CONFIG)
 
 CONFIG_INTEGER(JitTelemetry, W("JitTelemetry"), 1) // If non-zero, gather JIT telemetry data
diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp
index 8bee0e2cfd914..fee1085c23e06 100644
--- a/src/coreclr/jit/morph.cpp
+++ b/src/coreclr/jit/morph.cpp
@@ -14240,6 +14240,22 @@ GenTree* Compiler::fgMorphTree(GenTree* tree, MorphAddrContext* mac)
             tree = fgMorphStoreDynBlock(tree->AsStoreDynBlk());
             break;
 
+        case GT_SELECT:
+            tree->AsConditional()->gtCond = fgMorphTree(tree->AsConditional()->gtCond);
+            tree->AsConditional()->gtOp1  = fgMorphTree(tree->AsConditional()->gtOp1);
+            tree->AsConditional()->gtOp2  = fgMorphTree(tree->AsConditional()->gtOp2);
+
+            tree->gtFlags &= (~GTF_EXCEPT & ~GTF_CALL);
+
+            tree->gtFlags |= tree->AsConditional()->gtCond->gtFlags & GTF_ALL_EFFECT;
+            tree->gtFlags |= tree->AsConditional()->gtOp1->gtFlags & GTF_ALL_EFFECT;
+            tree->gtFlags |= tree->AsConditional()->gtOp2->gtFlags & GTF_ALL_EFFECT;
+
+            // Try to fold away any constants etc.
+            tree = gtFoldExpr(tree);
+
+            break;
+
         default:
 #ifdef DEBUG
             gtDispTree(tree);
diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp
index c34d599d817cb..90441a7932aac 100644
--- a/src/coreclr/jit/optimizer.cpp
+++ b/src/coreclr/jit/optimizer.cpp
@@ -4564,6 +4564,267 @@ PhaseStatus Compiler::optUnrollLoops()
 #pragma warning(pop)
 #endif
 
+//-----------------------------------------------------------------------------
+// optIfConvert
+//
+// Find blocks representing simple if statements represented by conditional jumps
+// over another block. Try to replace the jumps by use of conditional nodes.
+//
+// Arguments:
+//   block -- block that may represent the conditional jump in an if statement.
+//
+// Returns:
+//   true if any IR changes possibly made.
+//
+bool Compiler::optIfConvert(BasicBlock* block)
+{
+#ifndef TARGET_ARM64
+    return false;
+#else
+
+    // Don't optimise the block if it is inside a loop
+    // When inside a loop, branches are quicker than selects.
+    // Detect via the block weight as that will be high when inside a loop.
+    if (block->getBBWeight(this) > BB_UNITY_WEIGHT)
+    {
+        return false;
+    }
+
+    // Does the block end by branching via a JTRUE after a compare?
+    if (block->bbJumpKind != BBJ_COND || block->NumSucc() != 2)
+    {
+        return false;
+    }
+
+    // Verify the test block ends with a conditional that we can manipulate.
+    GenTree* last = block->lastStmt()->GetRootNode();
+    noway_assert(last->gtOper == GT_JTRUE);
+    if (!last->AsOp()->gtOp1->OperIsCmpCompare() || (last->gtFlags & GTF_SIDE_EFFECT) != 0 ||
+        (last->AsOp()->gtOp1->gtFlags & GTF_COLON_COND) != 0)
+    {
+        return false;
+    }
+
+    BasicBlock* middle_block = block;
+    GenTree*    asg_node     = nullptr;
+    Statement*  asg_stmt     = nullptr;
+    bool        found_select = false;
+
+    // Check the the block is followed by a block or chain of blocks that only contain NOPs and
+    // a single ASG statement. The destination of the final block must point to the same as the
+    // true path of the JTRUE block.
+    bool found_middle = false;
+    while (!found_middle)
+    {
+        middle_block = middle_block->bbNext;
+        noway_assert(middle_block != nullptr);
+
+        if (middle_block->bbNext == block->bbJumpDest)
+        {
+            // This is our final middle block.
+            found_middle = true;
+        }
+
+        // Make sure there is only one block which jumps to the middle block,
+        // and the middle block is not the start of a TRY block or an exception handler.
+        noway_assert(!fgCheapPredsValid);
+        if (middle_block->NumSucc() != 1 || middle_block->bbJumpKind != BBJ_NONE ||
+            middle_block->bbPreds->flNext != nullptr || middle_block->bbCatchTyp != BBCT_NONE ||
+            ((middle_block->bbFlags & (BBF_TRY_BEG | BBF_DONT_REMOVE)) != 0))
+        {
+            return false;
+        }
+
+        // Can all the nodes within the middle block be made to conditionally execute?
+        for (Statement* const stmt : middle_block->Statements())
+        {
+            GenTree* tree = stmt->GetRootNode();
+            switch (tree->gtOper)
+            {
+                case GT_ASG:
+                {
+                    GenTree* op1 = tree->AsOp()->gtOp1;
+                    GenTree* op2 = tree->AsOp()->gtOp2;
+
+                    // Only one per assignment per block can be conditionally executed.
+                    // Ensure the destination of the assign is a local variable with integer type,
+                    // and the nodes of the assign won't cause any additional side effects.
+                    if (asg_node != nullptr || op1->gtOper != GT_LCL_VAR || !varTypeIsIntegralOrI(op1->TypeGet()) ||
+                        (op1->gtFlags & GTF_SIDE_EFFECT) != 0 || (op2->gtFlags & GTF_SIDE_EFFECT) != 0)
+                    {
+                        return false;
+                    }
+                    asg_node = tree;
+                    asg_stmt = stmt;
+
+                    if (op2->gtOper == GT_SELECT)
+                    {
+                        found_select = true;
+                    }
+                    break;
+                }
+
+                // These do not need conditional execution.
+                case GT_NOP:
+                    break;
+
+                // Cannot optimise this block.
+                default:
+                    return false;
+            }
+        }
+    }
+    if (asg_node == nullptr)
+    {
+        // The blocks checked didn't contain any ASG nodes.
+        return false;
+    }
+
+#ifdef DEBUG
+    if (verbose)
+    {
+        JITDUMP("Attempting to conditionally execute " FMT_BB " from " FMT_BB "\n", middle_block->bbNum, block->bbNum);
+        for (BasicBlock* dump_block = block; dump_block != middle_block->bbNext; dump_block = dump_block->bbNext)
+        {
+            fgDumpBlock(dump_block);
+        }
+        JITDUMP("\n");
+    }
+#endif
+
+    if (found_select)
+    {
+        // The assign is already conditional. Try adding another condition.
+        if (!optIfConvertCCmp(last->gtGetOp1(), asg_node))
+        {
+            return false;
+        }
+    }
+    else
+    {
+        // Make the assign conditional.
+        if (!optIfConvertSelect(last->gtGetOp1(), asg_node))
+        {
+            return false;
+        }
+    }
+
+    // Remove the JTRUE statement.
+    last->ReplaceWith(gtNewNothingNode(), this);
+    fgSetStmtSeq(block->lastStmt());
+    gtSetEvalOrder(last);
+    fgSetStmtSeq(asg_stmt);
+
+    // Update the flow.
+    fgRemoveAllRefPreds(block->bbJumpDest, block);
+    block->bbJumpKind = BBJ_NONE;
+    block->bbJumpDest = middle_block;
+
+#ifdef DEBUG
+    if (verbose)
+    {
+        JITDUMP("After if conversion\n");
+        for (BasicBlock* dump_block = block; dump_block != middle_block->bbNext; dump_block = dump_block->bbNext)
+        {
+            fgDumpBlock(dump_block);
+        }
+        JITDUMP("\n");
+    }
+#endif
+
+    return true;
+#endif
+}
+
+bool Compiler::optIfConvertSelect(GenTree* original_condition, GenTree* asg_node)
+{
+    assert(original_condition->OperIsCmpCompare());
+    assert(asg_node->gtOper == GT_ASG);
+
+    // Duplicate the input of the assign.
+    // This will be used as the false result of the select node.
+    GenTree* current_value = gtCloneExpr(asg_node->AsOp()->gtOp1);
+    current_value->gtFlags &= GTF_EMPTY;
+
+    // Duplicate the condition and invert it
+    GenTree* cond = gtCloneExpr(original_condition);
+    cond->gtFlags |= GTF_DONT_CSE;
+    cond->gtFlags ^= GTF_RELOP_JMP_USED;
+    cond->gtFlags ^= GTF_RELOP_NAN_UN;
+    cond->gtOper = GenTree::ReverseRelop(cond->gtOper);
+
+    // Create a select node.
+    GenTreeConditional* select =
+        gtNewConditionalNode(GT_SELECT, cond, asg_node->AsOp()->gtOp2, current_value, asg_node->TypeGet());
+
+    // Use the select as the input to the assignment.
+    asg_node->AsOp()->gtOp2 = select;
+    asg_node->AsOp()->gtFlags |= (select->gtFlags & GTF_ALL_EFFECT);
+
+    // Calculate costs.
+    gtSetEvalOrder(select);
+
+    return true;
+}
+
+bool Compiler::optIfConvertCCmp(GenTree* original_condition, GenTree* asg_node)
+{
+    assert(original_condition->OperIsCmpCompare());
+    assert(asg_node->gtOper == GT_ASG);
+
+    // For now, don't handle floats.
+    if (!varTypeIsIntegralOrI(original_condition->AsOp()->gtOp1->TypeGet()))
+    {
+        return false;
+    }
+
+    // Note: Limiting the maximum number of chained compares may give a performance
+    //       boost, at the cost of more emitted code.
+
+    GenTreeConditional* select_node      = asg_node->AsOp()->gtOp2->AsConditional();
+    GenTree*            select_node_cond = select_node->gtCond;
+
+    // Duplicate the condition and invert it
+    GenTree* new_cond = gtCloneExpr(original_condition);
+    new_cond->gtFlags |= GTF_DONT_CSE;
+    new_cond->gtFlags ^= GTF_RELOP_JMP_USED;
+    new_cond->gtFlags ^= GTF_RELOP_NAN_UN;
+    new_cond->gtOper = GenTree::ReverseRelop(new_cond->gtOper);
+
+    // Link together the two conditions.
+    GenTree* const link = gtNewOperNode(GT_AND, TYP_INT, select_node_cond, new_cond);
+    link->gtFlags |= (select_node_cond->gtFlags & GTF_ALL_EFFECT);
+    link->gtFlags |= (new_cond->gtFlags & GTF_ALL_EFFECT);
+
+    // Use the link as the conditional input to the select.
+    select_node->gtCond = link;
+    select_node->gtFlags |= (link->gtFlags & GTF_ALL_EFFECT);
+    select_node->gtFlags ^= GTF_RELOP_NAN_UN;
+    asg_node->AsOp()->gtFlags |= (select_node->gtFlags & GTF_ALL_EFFECT);
+
+    // Calculate costs.
+    gtSetEvalOrder(link);
+
+    return true;
+}
+
+//-----------------------------------------------------------------------------
+// optIfConversion: If conversion
+//
+// Returns:
+//   suitable phase status
+//
+void Compiler::optIfConversion()
+{
+    // Reverse iterate through the blocks.
+    BasicBlock* block = fgLastBB;
+    while (block != nullptr)
+    {
+        optIfConvert(block);
+        block = block->bbPrev;
+    }
+}
+
 /*****************************************************************************
  *
  *  Return false if there is a code path from 'topBB' to 'botBB' that might
diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp
index b18ebd746e27c..aed856ff6c172 100644
--- a/src/coreclr/jit/valuenum.cpp
+++ b/src/coreclr/jit/valuenum.cpp
@@ -9075,6 +9075,27 @@ void Compiler::fgValueNumberTree(GenTree* tree)
                 }
                 break;
 
+            case GT_SELECT:
+            {
+                GenTreeConditional* const conditional = tree->AsConditional();
+
+                ValueNumPair vnpExcSet = ValueNumStore::VNPForEmptyExcSet();
+
+                // Collect the exception sets from our operands
+                vnpExcSet = vnStore->VNPUnionExcSet(conditional->gtCond->gtVNPair, vnpExcSet);
+                vnpExcSet = vnStore->VNPUnionExcSet(conditional->gtOp1->gtVNPair, vnpExcSet);
+                vnpExcSet = vnStore->VNPUnionExcSet(conditional->gtOp2->gtVNPair, vnpExcSet);
+
+                // The normal value is a new unique VN.
+                ValueNumPair normalPair;
+                normalPair.SetBoth(vnStore->VNForExpr(compCurBB, tree->TypeGet()));
+
+                // Attach the combined exception set
+                tree->gtVNPair = vnStore->VNPWithExc(normalPair, vnpExcSet);
+
+                break;
+            }
+
             default:
                 assert(!"Unhandled special node in fgValueNumberTree");
                 tree->gtVNPair.SetBoth(vnStore->VNForExpr(compCurBB, tree->TypeGet()));

From 8ee167fe848c9255dd7112d21f0bc959e0f6a645 Mon Sep 17 00:00:00 2001
From: Alan Hayward <alan.hayward@arm.com>
Date: Wed, 10 Aug 2022 16:38:28 +0100
Subject: [PATCH 02/38] Minor review fixups

---
 src/coreclr/jit/assertionprop.cpp |  16 ----
 src/coreclr/jit/compiler.h        |   4 +-
 src/coreclr/jit/gentree.cpp       |   7 +-
 src/coreclr/jit/optimizer.cpp     | 129 ++++++++++++++++--------------
 4 files changed, 72 insertions(+), 84 deletions(-)

diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp
index 8ca2f4c6f8dc7..a066b23403609 100644
--- a/src/coreclr/jit/assertionprop.cpp
+++ b/src/coreclr/jit/assertionprop.cpp
@@ -4388,18 +4388,6 @@ GenTree* Compiler::optAssertionPropLocal_RelOp(ASSERT_VALARG_TP assertions, GenT
     return optAssertionProp_Update(op2, tree, stmt);
 }
 
-/*****************************************************************************
- *
- *  Given a tree ConditionalOp, check using standard morph function.
- */
-
-GenTree* Compiler::optAssertionProp_ConditionalOp(ASSERT_VALARG_TP assertions, GenTree* tree, Statement* stmt)
-{
-    assert(tree->OperIsConditional());
-    GenTree* newTree = fgMorphTree(tree);
-    return optAssertionProp_Update(newTree, tree, stmt);
-}
-
 //------------------------------------------------------------------------
 // optAssertionProp_Cast: Propagate assertion for a cast, possibly removing it.
 //
@@ -5086,9 +5074,6 @@ GenTree* Compiler::optAssertionProp(ASSERT_VALARG_TP assertions, GenTree* tree,
             }
             return nullptr;
 
-        case GT_SELECT:
-            return optAssertionProp_ConditionalOp(assertions, tree, stmt);
-
         default:
             return nullptr;
     }
@@ -6026,7 +6011,6 @@ Compiler::fgWalkResult Compiler::optVNConstantPropCurStmt(BasicBlock* block, Sta
         case GT_NEG:
         case GT_CAST:
         case GT_INTRINSIC:
-        case GT_SELECT:
             break;
 
         case GT_JTRUE:
diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h
index 104cf54ab220f..a26806c04bd68 100644
--- a/src/coreclr/jit/compiler.h
+++ b/src/coreclr/jit/compiler.h
@@ -6480,8 +6480,8 @@ class Compiler
     bool optInvertWhileLoop(BasicBlock* block);
 
     bool optIfConvert(BasicBlock* block);
-    bool optIfConvertSelect(GenTree* original_condition, GenTree* asg_node);
-    bool optIfConvertCCmp(GenTree* original_condition, GenTree* asg_node);
+    bool optIfConvertSelect(GenTree* originalCondition, GenTree* asgNode);
+    bool optIfConvertCCmp(GenTree* originalCondition, GenTree* asgNode);
 
 private:
     static bool optIterSmallOverflow(int iterAtExit, var_types incrType);
diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp
index 7c15869d101b6..c85badd919927 100644
--- a/src/coreclr/jit/gentree.cpp
+++ b/src/coreclr/jit/gentree.cpp
@@ -12895,12 +12895,12 @@ GenTree* Compiler::gtFoldExprConditional(GenTree* tree)
         JITDUMP("\nFolding conditional op with constant condition:\n");
         DISPTREE(tree);
 
-        assert(cond->TypeGet() == TYP_INT);
+        assert(cond->TypeIs(TYP_INT));
         assert((tree->gtFlags & GTF_SIDE_EFFECT & ~GTF_ASG) == 0);
         assert((tree->gtFlags & GTF_ORDER_SIDEEFF) == 0);
 
         GenTree* replacement = nullptr;
-        if (cond->AsIntConCommon()->IntegralValue() == 0)
+        if (cond->IsIntegralConst(0))
         {
             JITDUMP("Bashed to false path:\n");
             replacement = op2;
@@ -12908,8 +12908,7 @@ GenTree* Compiler::gtFoldExprConditional(GenTree* tree)
         else
         {
             // Condition should never be a constant other than 0 or 1
-            assert(cond->AsIntConCommon()->IntegralValue() == 1);
-
+            assert (cond->IsIntegralConst(1));
             JITDUMP("Bashed to true path:\n");
             replacement = op1;
         }
diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp
index 90441a7932aac..b0f41ce282f9d 100644
--- a/src/coreclr/jit/optimizer.cpp
+++ b/src/coreclr/jit/optimizer.cpp
@@ -4598,74 +4598,79 @@ bool Compiler::optIfConvert(BasicBlock* block)
 
     // Verify the test block ends with a conditional that we can manipulate.
     GenTree* last = block->lastStmt()->GetRootNode();
-    noway_assert(last->gtOper == GT_JTRUE);
+    noway_assert(last->OperIs(GT_JTRUE));
     if (!last->AsOp()->gtOp1->OperIsCmpCompare() || (last->gtFlags & GTF_SIDE_EFFECT) != 0 ||
         (last->AsOp()->gtOp1->gtFlags & GTF_COLON_COND) != 0)
     {
         return false;
     }
 
-    BasicBlock* middle_block = block;
-    GenTree*    asg_node     = nullptr;
-    Statement*  asg_stmt     = nullptr;
-    bool        found_select = false;
+    BasicBlock* middleBlock = block;
+    GenTree*    asgNode     = nullptr;
+    Statement*  asgStmt     = nullptr;
+    bool        foundSelect = false;
 
-    // Check the the block is followed by a block or chain of blocks that only contain NOPs and
+    // Check the block is followed by a block or chain of blocks that only contain NOPs and
     // a single ASG statement. The destination of the final block must point to the same as the
     // true path of the JTRUE block.
-    bool found_middle = false;
-    while (!found_middle)
+    bool foundMiddle = false;
+    while (!foundMiddle)
     {
-        middle_block = middle_block->bbNext;
-        noway_assert(middle_block != nullptr);
+        middleBlock = middleBlock->bbNext;
+        noway_assert(middleBlock != nullptr);
 
-        if (middle_block->bbNext == block->bbJumpDest)
+        if (middleBlock->bbNext == block->bbJumpDest)
         {
             // This is our final middle block.
-            found_middle = true;
+            foundMiddle = true;
         }
 
         // Make sure there is only one block which jumps to the middle block,
         // and the middle block is not the start of a TRY block or an exception handler.
         noway_assert(!fgCheapPredsValid);
-        if (middle_block->NumSucc() != 1 || middle_block->bbJumpKind != BBJ_NONE ||
-            middle_block->bbPreds->flNext != nullptr || middle_block->bbCatchTyp != BBCT_NONE ||
-            ((middle_block->bbFlags & (BBF_TRY_BEG | BBF_DONT_REMOVE)) != 0))
+        if (middleBlock->NumSucc() != 1 || middleBlock->bbJumpKind != BBJ_NONE ||
+            middleBlock->bbPreds->flNext != nullptr || middleBlock->bbCatchTyp != BBCT_NONE ||
+            bbIsTryBeg(middleBlock) || bbIsHandlerBeg(middleBlock) ||
+            (middleBlock->bbFlags & BBF_DONT_REMOVE) != 0)
         {
             return false;
         }
 
         // Can all the nodes within the middle block be made to conditionally execute?
-        for (Statement* const stmt : middle_block->Statements())
+        for (Statement* const stmt : middleBlock->Statements())
         {
             GenTree* tree = stmt->GetRootNode();
             switch (tree->gtOper)
             {
                 case GT_ASG:
                 {
-                    GenTree* op1 = tree->AsOp()->gtOp1;
-                    GenTree* op2 = tree->AsOp()->gtOp2;
+                    GenTree* op1 = tree->gtGetOp1();
+                    GenTree* op2 = tree->gtGetOp2();
 
                     // Only one per assignment per block can be conditionally executed.
                     // Ensure the destination of the assign is a local variable with integer type,
                     // and the nodes of the assign won't cause any additional side effects.
-                    if (asg_node != nullptr || op1->gtOper != GT_LCL_VAR || !varTypeIsIntegralOrI(op1->TypeGet()) ||
+                    if (asgNode != nullptr || !op1->OperIs(GT_LCL_VAR) || !varTypeIsIntegralOrI(op1) ||
                         (op1->gtFlags & GTF_SIDE_EFFECT) != 0 || (op2->gtFlags & GTF_SIDE_EFFECT) != 0)
                     {
                         return false;
                     }
-                    asg_node = tree;
-                    asg_stmt = stmt;
+                    asgNode = tree;
+                    asgStmt = stmt;
 
-                    if (op2->gtOper == GT_SELECT)
+                    if (op2->OperIs(GT_SELECT))
                     {
-                        found_select = true;
+                        foundSelect = true;
                     }
                     break;
                 }
 
                 // These do not need conditional execution.
                 case GT_NOP:
+                    if (tree->gtGetOp1() != nullptr)
+                    {
+                        return false;
+                    }
                     break;
 
                 // Cannot optimise this block.
@@ -4674,7 +4679,7 @@ bool Compiler::optIfConvert(BasicBlock* block)
             }
         }
     }
-    if (asg_node == nullptr)
+    if (asgNode == nullptr)
     {
         // The blocks checked didn't contain any ASG nodes.
         return false;
@@ -4683,19 +4688,19 @@ bool Compiler::optIfConvert(BasicBlock* block)
 #ifdef DEBUG
     if (verbose)
     {
-        JITDUMP("Attempting to conditionally execute " FMT_BB " from " FMT_BB "\n", middle_block->bbNum, block->bbNum);
-        for (BasicBlock* dump_block = block; dump_block != middle_block->bbNext; dump_block = dump_block->bbNext)
+        JITDUMP("Attempting to conditionally execute " FMT_BB " from " FMT_BB "\n", middleBlock->bbNum, block->bbNum);
+        for (BasicBlock* dumpBlock = block; dumpBlock != middleBlock->bbNext; dumpBlock = dumpBlock->bbNext)
         {
-            fgDumpBlock(dump_block);
+            fgDumpBlock(dumpBlock);
         }
         JITDUMP("\n");
     }
 #endif
 
-    if (found_select)
+    if (foundSelect)
     {
         // The assign is already conditional. Try adding another condition.
-        if (!optIfConvertCCmp(last->gtGetOp1(), asg_node))
+        if (!optIfConvertCCmp(last->gtGetOp1(), asgNode))
         {
             return false;
         }
@@ -4703,7 +4708,7 @@ bool Compiler::optIfConvert(BasicBlock* block)
     else
     {
         // Make the assign conditional.
-        if (!optIfConvertSelect(last->gtGetOp1(), asg_node))
+        if (!optIfConvertSelect(last->gtGetOp1(), asgNode))
         {
             return false;
         }
@@ -4713,20 +4718,20 @@ bool Compiler::optIfConvert(BasicBlock* block)
     last->ReplaceWith(gtNewNothingNode(), this);
     fgSetStmtSeq(block->lastStmt());
     gtSetEvalOrder(last);
-    fgSetStmtSeq(asg_stmt);
+    fgSetStmtSeq(asgStmt);
 
     // Update the flow.
     fgRemoveAllRefPreds(block->bbJumpDest, block);
     block->bbJumpKind = BBJ_NONE;
-    block->bbJumpDest = middle_block;
+    block->bbJumpDest = middleBlock;
 
 #ifdef DEBUG
     if (verbose)
     {
         JITDUMP("After if conversion\n");
-        for (BasicBlock* dump_block = block; dump_block != middle_block->bbNext; dump_block = dump_block->bbNext)
+        for (BasicBlock* dumpBlock = block; dumpBlock != middleBlock->bbNext; dumpBlock = dumpBlock->bbNext)
         {
-            fgDumpBlock(dump_block);
+            fgDumpBlock(dumpBlock);
         }
         JITDUMP("\n");
     }
@@ -4736,18 +4741,18 @@ bool Compiler::optIfConvert(BasicBlock* block)
 #endif
 }
 
-bool Compiler::optIfConvertSelect(GenTree* original_condition, GenTree* asg_node)
+bool Compiler::optIfConvertSelect(GenTree* originalCondition, GenTree* asgNode)
 {
-    assert(original_condition->OperIsCmpCompare());
-    assert(asg_node->gtOper == GT_ASG);
+    assert(originalCondition->OperIsCmpCompare());
+    assert(asgNode->OperIs(GT_ASG));
 
     // Duplicate the input of the assign.
     // This will be used as the false result of the select node.
-    GenTree* current_value = gtCloneExpr(asg_node->AsOp()->gtOp1);
-    current_value->gtFlags &= GTF_EMPTY;
+    GenTree* destination = gtCloneExpr(asgNode->AsOp()->gtOp1);
+    destination->gtFlags &= GTF_EMPTY;
 
     // Duplicate the condition and invert it
-    GenTree* cond = gtCloneExpr(original_condition);
+    GenTree* cond = gtCloneExpr(originalCondition);
     cond->gtFlags |= GTF_DONT_CSE;
     cond->gtFlags ^= GTF_RELOP_JMP_USED;
     cond->gtFlags ^= GTF_RELOP_NAN_UN;
@@ -4755,11 +4760,11 @@ bool Compiler::optIfConvertSelect(GenTree* original_condition, GenTree* asg_node
 
     // Create a select node.
     GenTreeConditional* select =
-        gtNewConditionalNode(GT_SELECT, cond, asg_node->AsOp()->gtOp2, current_value, asg_node->TypeGet());
+        gtNewConditionalNode(GT_SELECT, cond, asgNode->AsOp()->gtOp2, destination, asgNode->TypeGet());
 
     // Use the select as the input to the assignment.
-    asg_node->AsOp()->gtOp2 = select;
-    asg_node->AsOp()->gtFlags |= (select->gtFlags & GTF_ALL_EFFECT);
+    asgNode->AsOp()->gtOp2 = select;
+    asgNode->AsOp()->gtFlags |= (select->gtFlags & GTF_ALL_EFFECT);
 
     // Calculate costs.
     gtSetEvalOrder(select);
@@ -4767,13 +4772,13 @@ bool Compiler::optIfConvertSelect(GenTree* original_condition, GenTree* asg_node
     return true;
 }
 
-bool Compiler::optIfConvertCCmp(GenTree* original_condition, GenTree* asg_node)
+bool Compiler::optIfConvertCCmp(GenTree* originalCondition, GenTree* asgNode)
 {
-    assert(original_condition->OperIsCmpCompare());
-    assert(asg_node->gtOper == GT_ASG);
+    assert(originalCondition->OperIsCmpCompare());
+    assert(asgNode->OperIs(GT_ASG));
 
-    // For now, don't handle floats.
-    if (!varTypeIsIntegralOrI(original_condition->AsOp()->gtOp1->TypeGet()))
+    // TODO-CQ: Add float support.
+    if (!varTypeIsIntegralOrI(originalCondition->AsOp()->gtOp1->TypeGet()))
     {
         return false;
     }
@@ -4781,26 +4786,26 @@ bool Compiler::optIfConvertCCmp(GenTree* original_condition, GenTree* asg_node)
     // Note: Limiting the maximum number of chained compares may give a performance
     //       boost, at the cost of more emitted code.
 
-    GenTreeConditional* select_node      = asg_node->AsOp()->gtOp2->AsConditional();
-    GenTree*            select_node_cond = select_node->gtCond;
+    GenTreeConditional* selectNode      = asgNode->AsOp()->gtOp2->AsConditional();
+    GenTree*            selectNodeCond  = selectNode->gtCond;
 
     // Duplicate the condition and invert it
-    GenTree* new_cond = gtCloneExpr(original_condition);
-    new_cond->gtFlags |= GTF_DONT_CSE;
-    new_cond->gtFlags ^= GTF_RELOP_JMP_USED;
-    new_cond->gtFlags ^= GTF_RELOP_NAN_UN;
-    new_cond->gtOper = GenTree::ReverseRelop(new_cond->gtOper);
+    GenTree* newCond = gtCloneExpr(originalCondition);
+    newCond->gtFlags |= GTF_DONT_CSE;
+    newCond->gtFlags ^= GTF_RELOP_JMP_USED;
+    newCond->gtFlags ^= GTF_RELOP_NAN_UN;
+    newCond->gtOper = GenTree::ReverseRelop(newCond->gtOper);
 
     // Link together the two conditions.
-    GenTree* const link = gtNewOperNode(GT_AND, TYP_INT, select_node_cond, new_cond);
-    link->gtFlags |= (select_node_cond->gtFlags & GTF_ALL_EFFECT);
-    link->gtFlags |= (new_cond->gtFlags & GTF_ALL_EFFECT);
+    GenTree* const link = gtNewOperNode(GT_AND, TYP_INT, selectNodeCond, newCond);
+    link->gtFlags |= (selectNodeCond->gtFlags & GTF_ALL_EFFECT);
+    link->gtFlags |= (newCond->gtFlags & GTF_ALL_EFFECT);
 
     // Use the link as the conditional input to the select.
-    select_node->gtCond = link;
-    select_node->gtFlags |= (link->gtFlags & GTF_ALL_EFFECT);
-    select_node->gtFlags ^= GTF_RELOP_NAN_UN;
-    asg_node->AsOp()->gtFlags |= (select_node->gtFlags & GTF_ALL_EFFECT);
+    selectNode->gtCond = link;
+    selectNode->gtFlags |= (link->gtFlags & GTF_ALL_EFFECT);
+    selectNode->gtFlags ^= GTF_RELOP_NAN_UN;
+    asgNode->AsOp()->gtFlags |= (selectNode->gtFlags & GTF_ALL_EFFECT);
 
     // Calculate costs.
     gtSetEvalOrder(link);

From 039de48b8268bbe0497ea54708e0483aba24de10 Mon Sep 17 00:00:00 2001
From: Alan Hayward <alan.hayward@arm.com>
Date: Wed, 10 Aug 2022 18:11:35 +0100
Subject: [PATCH 03/38] Return a PhaseStatus

---
 src/coreclr/jit/compiler.h    |  2 +-
 src/coreclr/jit/optimizer.cpp | 10 +++++++---
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h
index a26806c04bd68..c80364a849d41 100644
--- a/src/coreclr/jit/compiler.h
+++ b/src/coreclr/jit/compiler.h
@@ -6095,7 +6095,7 @@ class Compiler
     void optEnsureUniqueHead(unsigned loopInd, weight_t ambientWeight);
     PhaseStatus optUnrollLoops(); // Unrolls loops (needs to have cost info)
     void        optRemoveRedundantZeroInits();
-    void        optIfConversion(); // If conversion
+    PhaseStatus optIfConversion(); // If conversion
 
 protected:
     // This enumeration describes what is killed by a call.
diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp
index b0f41ce282f9d..bf561eb522c4a 100644
--- a/src/coreclr/jit/optimizer.cpp
+++ b/src/coreclr/jit/optimizer.cpp
@@ -4762,7 +4762,7 @@ bool Compiler::optIfConvertSelect(GenTree* originalCondition, GenTree* asgNode)
     GenTreeConditional* select =
         gtNewConditionalNode(GT_SELECT, cond, asgNode->AsOp()->gtOp2, destination, asgNode->TypeGet());
 
-    // Use the select as the input to the assignment.
+    // Use the select as the source of the assignment.
     asgNode->AsOp()->gtOp2 = select;
     asgNode->AsOp()->gtFlags |= (select->gtFlags & GTF_ALL_EFFECT);
 
@@ -4819,15 +4819,19 @@ bool Compiler::optIfConvertCCmp(GenTree* originalCondition, GenTree* asgNode)
 // Returns:
 //   suitable phase status
 //
-void Compiler::optIfConversion()
+PhaseStatus Compiler::optIfConversion()
 {
+    bool madeChanges = false;
+
     // Reverse iterate through the blocks.
     BasicBlock* block = fgLastBB;
     while (block != nullptr)
     {
-        optIfConvert(block);
+        madeChanges |= optIfConvert(block);
         block = block->bbPrev;
     }
+
+    return madeChanges ? PhaseStatus::MODIFIED_EVERYTHING : PhaseStatus::MODIFIED_NOTHING;
 }
 
 /*****************************************************************************

From ddfad68c923d7f4a70fffc2beeb183dfb8e1f433 Mon Sep 17 00:00:00 2001
From: Alan Hayward <alan.hayward@arm.com>
Date: Thu, 11 Aug 2022 10:54:19 +0100
Subject: [PATCH 04/38] Fix formatting

---
 src/coreclr/jit/gentree.cpp   | 2 +-
 src/coreclr/jit/optimizer.cpp | 7 +++----
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp
index c85badd919927..f30bfd283ba9b 100644
--- a/src/coreclr/jit/gentree.cpp
+++ b/src/coreclr/jit/gentree.cpp
@@ -12908,7 +12908,7 @@ GenTree* Compiler::gtFoldExprConditional(GenTree* tree)
         else
         {
             // Condition should never be a constant other than 0 or 1
-            assert (cond->IsIntegralConst(1));
+            assert(cond->IsIntegralConst(1));
             JITDUMP("Bashed to true path:\n");
             replacement = op1;
         }
diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp
index bf561eb522c4a..6d7d0448eb49f 100644
--- a/src/coreclr/jit/optimizer.cpp
+++ b/src/coreclr/jit/optimizer.cpp
@@ -4630,8 +4630,7 @@ bool Compiler::optIfConvert(BasicBlock* block)
         noway_assert(!fgCheapPredsValid);
         if (middleBlock->NumSucc() != 1 || middleBlock->bbJumpKind != BBJ_NONE ||
             middleBlock->bbPreds->flNext != nullptr || middleBlock->bbCatchTyp != BBCT_NONE ||
-            bbIsTryBeg(middleBlock) || bbIsHandlerBeg(middleBlock) ||
-            (middleBlock->bbFlags & BBF_DONT_REMOVE) != 0)
+            bbIsTryBeg(middleBlock) || bbIsHandlerBeg(middleBlock) || (middleBlock->bbFlags & BBF_DONT_REMOVE) != 0)
         {
             return false;
         }
@@ -4786,8 +4785,8 @@ bool Compiler::optIfConvertCCmp(GenTree* originalCondition, GenTree* asgNode)
     // Note: Limiting the maximum number of chained compares may give a performance
     //       boost, at the cost of more emitted code.
 
-    GenTreeConditional* selectNode      = asgNode->AsOp()->gtOp2->AsConditional();
-    GenTree*            selectNodeCond  = selectNode->gtCond;
+    GenTreeConditional* selectNode     = asgNode->AsOp()->gtOp2->AsConditional();
+    GenTree*            selectNodeCond = selectNode->gtCond;
 
     // Duplicate the condition and invert it
     GenTree* newCond = gtCloneExpr(originalCondition);

From 41c3a9e35db8e51514bb08d4c3467eb40d8b6f70 Mon Sep 17 00:00:00 2001
From: Alan Hayward <alan.hayward@arm.com>
Date: Thu, 11 Aug 2022 10:56:06 +0100
Subject: [PATCH 05/38] Check for side effects on NOPs

---
 src/coreclr/jit/optimizer.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp
index 6d7d0448eb49f..ce981e58d99db 100644
--- a/src/coreclr/jit/optimizer.cpp
+++ b/src/coreclr/jit/optimizer.cpp
@@ -4666,7 +4666,7 @@ bool Compiler::optIfConvert(BasicBlock* block)
 
                 // These do not need conditional execution.
                 case GT_NOP:
-                    if (tree->gtGetOp1() != nullptr)
+                    if (tree->gtGetOp1() != nullptr || (tree->gtFlags & GTF_SIDE_EFFECT) != 0 )
                     {
                         return false;
                     }

From 4c93a4c320b8f170955c060a48850d32cf661c6b Mon Sep 17 00:00:00 2001
From: Alan Hayward <alan.hayward@arm.com>
Date: Thu, 11 Aug 2022 13:53:05 +0100
Subject: [PATCH 06/38] Add function block comments for the phase

---
 src/coreclr/jit/compiler.h    |   4 +-
 src/coreclr/jit/optimizer.cpp | 158 ++++++++++++++++++++++++++++++----
 2 files changed, 145 insertions(+), 17 deletions(-)

diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h
index c80364a849d41..1ec6a95b92d5a 100644
--- a/src/coreclr/jit/compiler.h
+++ b/src/coreclr/jit/compiler.h
@@ -6480,8 +6480,8 @@ class Compiler
     bool optInvertWhileLoop(BasicBlock* block);
 
     bool optIfConvert(BasicBlock* block);
-    bool optIfConvertSelect(GenTree* originalCondition, GenTree* asgNode);
-    bool optIfConvertCCmp(GenTree* originalCondition, GenTree* asgNode);
+    bool createConditionalAssignment(GenTree* asg, GenTree* condition);
+    bool extendConditionalAssignment(GenTree* asg, GenTree* originalCondition);
 
 private:
     static bool optIterSmallOverflow(int iterAtExit, var_types incrType);
diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp
index ce981e58d99db..5aa92bcc2f49d 100644
--- a/src/coreclr/jit/optimizer.cpp
+++ b/src/coreclr/jit/optimizer.cpp
@@ -4568,7 +4568,7 @@ PhaseStatus Compiler::optUnrollLoops()
 // optIfConvert
 //
 // Find blocks representing simple if statements represented by conditional jumps
-// over another block. Try to replace the jumps by use of conditional nodes.
+// over another block. Try to replace the jumps by use of SELECT nodes.
 //
 // Arguments:
 //   block -- block that may represent the conditional jump in an if statement.
@@ -4576,6 +4576,103 @@ PhaseStatus Compiler::optUnrollLoops()
 // Returns:
 //   true if any IR changes possibly made.
 //
+// Notes:
+//
+// 1: Example of simple if conversion:
+//
+// This is optimising a simple if statement. There is a single condition being
+// tested, and a single assignment inside the body. There must be no else
+// statement. For example:
+// if (x < 7) { a = 5; }
+//
+// This is represented in IR by two basic blocks. The first block ends with a
+// JTRUE statement which conditionally jumps to the second block. Both blocks
+// then jump to the same destination. The second block just contains a single
+// assign statement. (Note that the first block may contain additional statements
+// prior to the JTRUE statement.)
+//
+// For example:
+// ------------ BB03 [009..00D) -> BB05 (cond), preds={BB02} succs={BB04,BB05}
+// STMT00004
+//   *  JTRUE     void   $VN.Void
+//   \--*  GE        int    $102
+//      +--*  LCL_VAR   int    V02
+//      \--*  CNS_INT   int    7 $46
+//
+// ------------ BB04 [00D..010), preds={BB03} succs={BB05}
+// STMT00005
+//   *  ASG       int    $VN.Void
+// +--*  LCL_VAR   int    V00 arg0
+// \--*  CNS_INT   int    5 $47
+//
+// The JTRUE statement in the first block is replace with a SELECT node in the
+// assignment in the second block. If the result of the SELECTs op1 is true,
+// then op2 is passed to the assignment. Otherwise op3 is used. For example:
+//
+// ------------ BB03 [009..00D), preds={BB02} succs={BB04}
+// STMT00004
+//   *  NOP       void
+//
+// ------------ BB04 [00D..010), preds={BB03} succs={BB05}
+// STMT00005
+//   *  ASG       int    $VN.Void
+//   +--*  LCL_VAR   int    V00 arg0
+//   \--*  SELECT    int
+//      +--*  LT        int    $102
+//      |  +--*  LCL_VAR   int    V02
+//      |  \--*  CNS_INT   int    7 $46
+//      +--*  CNS_INT   int    5 $47
+//      \--*  LCL_VAR   int    V00
+//
+//
+// 2: Example of if conversion with AND chains:
+//
+// Consider the previous example, but with an additional condition. The two conditions
+// must be combined via an AND condition. For example:
+// if (y != 10 && x < 7) { a = 5; }
+//
+// The IR here will be the same as the previous example, except it will be preceeded
+// by a block containing the first condition:
+//
+// ------------ BB02 [004..009) -> BB05 (cond), preds={BB01} succs={BB03,BB05}
+// STMT00003
+//   *  JTRUE     void   $VN.Void
+//   \--*  EQ        int    $101
+//     +--*  LCL_VAR   int    V01
+//     \--*  CNS_INT   int    10 $45
+//
+// First the previous optimisation across blocks BB03 and BB04 will be applied.
+// Then the JTRUE in the addition condition will be removed and added onto the
+// condition in the select via an AND node:
+//
+// ------------ BB02 [004..009), preds={BB01} succs={BB03}
+// STMT00003
+//   *  NOP       void
+//
+// ------------ BB03 [009..00D), preds={BB02} succs={BB04}
+// STMT00004
+//   *  NOP       void
+//
+// ------------ BB04 [00D..010), preds={BB03} succs={BB05}
+// STMT00005
+//  *  ASG       int    $VN.Void
+//  +--*  LCL_VAR   int    V00 arg0
+//  \--*  SELECT    int
+//     +--*  AND       int
+//     |  +--*  LT        int    $102
+//     |  |  +--*  LCL_VAR   int    V02
+//     |  |  \--*  CNS_INT   int    7 $46
+//     |  \--*  NE        int    $101
+//     |     +--*  LCL_VAR   int    V01
+//     |     \--*  CNS_INT   int    10 $45
+//     +--*  CNS_INT   int    5 $47
+//     \--*  LCL_VAR   int    V00
+//
+// There is no limit on the number of conditions that could potentially be
+// combined this way. However this ordering means that every condition is
+// evalutated at execution time, which puts a practical limit on the number
+// of conditions to chain.
+//
 bool Compiler::optIfConvert(BasicBlock* block)
 {
 #ifndef TARGET_ARM64
@@ -4699,7 +4796,7 @@ bool Compiler::optIfConvert(BasicBlock* block)
     if (foundSelect)
     {
         // The assign is already conditional. Try adding another condition.
-        if (!optIfConvertCCmp(last->gtGetOp1(), asgNode))
+        if (!extendConditionalAssignment(asgNode, last->gtGetOp1()))
         {
             return false;
         }
@@ -4707,7 +4804,7 @@ bool Compiler::optIfConvert(BasicBlock* block)
     else
     {
         // Make the assign conditional.
-        if (!optIfConvertSelect(last->gtGetOp1(), asgNode))
+        if (!createConditionalAssignment(asgNode, last->gtGetOp1()))
         {
             return false;
         }
@@ -4740,18 +4837,34 @@ bool Compiler::optIfConvert(BasicBlock* block)
 #endif
 }
 
-bool Compiler::optIfConvertSelect(GenTree* originalCondition, GenTree* asgNode)
+//-----------------------------------------------------------------------------
+// createConditionalAssignment
+//
+// Helper function for optIfConvert. Given an assignment, make the assignment
+// conditionally execute based on the given condition. This is done by adding
+// a SELECT node as the source of the assignment. The exisiting value of the
+// assignment is used as the false path of the select
+// For example: (ASG Var Val) becomes (ASG Var (SELECT condition Val Var))
+//
+// Arguments:
+//   asg -- The assignment to conditionally execute.
+//   condition -- The assignment should only happen if this evaluates to true.
+//
+// Returns:
+//   true if any IR changes possibly made.
+//
+bool Compiler::createConditionalAssignment(GenTree* asg, GenTree* condition)
 {
-    assert(originalCondition->OperIsCmpCompare());
-    assert(asgNode->OperIs(GT_ASG));
+    assert(condition->OperIsCmpCompare());
+    assert(asg->OperIs(GT_ASG));
 
     // Duplicate the input of the assign.
     // This will be used as the false result of the select node.
-    GenTree* destination = gtCloneExpr(asgNode->AsOp()->gtOp1);
+    GenTree* destination = gtCloneExpr(asg->AsOp()->gtOp1);
     destination->gtFlags &= GTF_EMPTY;
 
     // Duplicate the condition and invert it
-    GenTree* cond = gtCloneExpr(originalCondition);
+    GenTree* cond = gtCloneExpr(condition);
     cond->gtFlags |= GTF_DONT_CSE;
     cond->gtFlags ^= GTF_RELOP_JMP_USED;
     cond->gtFlags ^= GTF_RELOP_NAN_UN;
@@ -4759,11 +4872,11 @@ bool Compiler::optIfConvertSelect(GenTree* originalCondition, GenTree* asgNode)
 
     // Create a select node.
     GenTreeConditional* select =
-        gtNewConditionalNode(GT_SELECT, cond, asgNode->AsOp()->gtOp2, destination, asgNode->TypeGet());
+        gtNewConditionalNode(GT_SELECT, cond, asg->AsOp()->gtOp2, destination, asg->TypeGet());
 
     // Use the select as the source of the assignment.
-    asgNode->AsOp()->gtOp2 = select;
-    asgNode->AsOp()->gtFlags |= (select->gtFlags & GTF_ALL_EFFECT);
+    asg->AsOp()->gtOp2 = select;
+    asg->AsOp()->gtFlags |= (select->gtFlags & GTF_ALL_EFFECT);
 
     // Calculate costs.
     gtSetEvalOrder(select);
@@ -4771,10 +4884,25 @@ bool Compiler::optIfConvertSelect(GenTree* originalCondition, GenTree* asgNode)
     return true;
 }
 
-bool Compiler::optIfConvertCCmp(GenTree* originalCondition, GenTree* asgNode)
+//-----------------------------------------------------------------------------
+// extendConditionalAssignment
+//
+// Helper function for optIfConvert. Given an assignment, which is already executed
+// conditionally via a SELECT node, add an additional condition.
+// For example: (ASG Var (SELECT oldcondition Val Var)) becomes
+// (ASG Var (SELECT (AND oldcondition newcondition) Val Var))
+//
+// Arguments:
+//   asg -- The conditional assignment to extend.
+//   condition -- The assignment should only happen if this evaluates to true.
+//
+// Returns:
+//   true if any IR changes possibly made.
+//
+bool Compiler::extendConditionalAssignment(GenTree* asg, GenTree* originalCondition)
 {
     assert(originalCondition->OperIsCmpCompare());
-    assert(asgNode->OperIs(GT_ASG));
+    assert(asg->OperIs(GT_ASG));
 
     // TODO-CQ: Add float support.
     if (!varTypeIsIntegralOrI(originalCondition->AsOp()->gtOp1->TypeGet()))
@@ -4785,7 +4913,7 @@ bool Compiler::optIfConvertCCmp(GenTree* originalCondition, GenTree* asgNode)
     // Note: Limiting the maximum number of chained compares may give a performance
     //       boost, at the cost of more emitted code.
 
-    GenTreeConditional* selectNode     = asgNode->AsOp()->gtOp2->AsConditional();
+    GenTreeConditional* selectNode     = asg->AsOp()->gtOp2->AsConditional();
     GenTree*            selectNodeCond = selectNode->gtCond;
 
     // Duplicate the condition and invert it
@@ -4804,7 +4932,7 @@ bool Compiler::optIfConvertCCmp(GenTree* originalCondition, GenTree* asgNode)
     selectNode->gtCond = link;
     selectNode->gtFlags |= (link->gtFlags & GTF_ALL_EFFECT);
     selectNode->gtFlags ^= GTF_RELOP_NAN_UN;
-    asgNode->AsOp()->gtFlags |= (selectNode->gtFlags & GTF_ALL_EFFECT);
+    asg->AsOp()->gtFlags |= (selectNode->gtFlags & GTF_ALL_EFFECT);
 
     // Calculate costs.
     gtSetEvalOrder(link);

From 891510dd879e0d168e6a190e30ed4d7fbb7166c4 Mon Sep 17 00:00:00 2001
From: Alan Hayward <alan.hayward@arm.com>
Date: Fri, 12 Aug 2022 09:36:53 +0100
Subject: [PATCH 07/38] Remove creation of AND chains from if conversion pass

---
 src/coreclr/jit/compiler.h    |   3 -
 src/coreclr/jit/optimizer.cpp | 208 +++++-----------------------------
 2 files changed, 29 insertions(+), 182 deletions(-)

diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h
index 1ec6a95b92d5a..267c75d0cda3f 100644
--- a/src/coreclr/jit/compiler.h
+++ b/src/coreclr/jit/compiler.h
@@ -6478,10 +6478,7 @@ class Compiler
     OptInvertCountTreeInfoType optInvertCountTreeInfo(GenTree* tree);
 
     bool optInvertWhileLoop(BasicBlock* block);
-
     bool optIfConvert(BasicBlock* block);
-    bool createConditionalAssignment(GenTree* asg, GenTree* condition);
-    bool extendConditionalAssignment(GenTree* asg, GenTree* originalCondition);
 
 private:
     static bool optIterSmallOverflow(int iterAtExit, var_types incrType);
diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp
index 5aa92bcc2f49d..546e50fee7ace 100644
--- a/src/coreclr/jit/optimizer.cpp
+++ b/src/coreclr/jit/optimizer.cpp
@@ -4578,7 +4578,7 @@ PhaseStatus Compiler::optUnrollLoops()
 //
 // Notes:
 //
-// 1: Example of simple if conversion:
+// Example of simple if conversion:
 //
 // This is optimising a simple if statement. There is a single condition being
 // tested, and a single assignment inside the body. There must be no else
@@ -4625,54 +4625,6 @@ PhaseStatus Compiler::optUnrollLoops()
 //      \--*  LCL_VAR   int    V00
 //
 //
-// 2: Example of if conversion with AND chains:
-//
-// Consider the previous example, but with an additional condition. The two conditions
-// must be combined via an AND condition. For example:
-// if (y != 10 && x < 7) { a = 5; }
-//
-// The IR here will be the same as the previous example, except it will be preceeded
-// by a block containing the first condition:
-//
-// ------------ BB02 [004..009) -> BB05 (cond), preds={BB01} succs={BB03,BB05}
-// STMT00003
-//   *  JTRUE     void   $VN.Void
-//   \--*  EQ        int    $101
-//     +--*  LCL_VAR   int    V01
-//     \--*  CNS_INT   int    10 $45
-//
-// First the previous optimisation across blocks BB03 and BB04 will be applied.
-// Then the JTRUE in the addition condition will be removed and added onto the
-// condition in the select via an AND node:
-//
-// ------------ BB02 [004..009), preds={BB01} succs={BB03}
-// STMT00003
-//   *  NOP       void
-//
-// ------------ BB03 [009..00D), preds={BB02} succs={BB04}
-// STMT00004
-//   *  NOP       void
-//
-// ------------ BB04 [00D..010), preds={BB03} succs={BB05}
-// STMT00005
-//  *  ASG       int    $VN.Void
-//  +--*  LCL_VAR   int    V00 arg0
-//  \--*  SELECT    int
-//     +--*  AND       int
-//     |  +--*  LT        int    $102
-//     |  |  +--*  LCL_VAR   int    V02
-//     |  |  \--*  CNS_INT   int    7 $46
-//     |  \--*  NE        int    $101
-//     |     +--*  LCL_VAR   int    V01
-//     |     \--*  CNS_INT   int    10 $45
-//     +--*  CNS_INT   int    5 $47
-//     \--*  LCL_VAR   int    V00
-//
-// There is no limit on the number of conditions that could potentially be
-// combined this way. However this ordering means that every condition is
-// evalutated at execution time, which puts a practical limit on the number
-// of conditions to chain.
-//
 bool Compiler::optIfConvert(BasicBlock* block)
 {
 #ifndef TARGET_ARM64
@@ -4696,8 +4648,8 @@ bool Compiler::optIfConvert(BasicBlock* block)
     // Verify the test block ends with a conditional that we can manipulate.
     GenTree* last = block->lastStmt()->GetRootNode();
     noway_assert(last->OperIs(GT_JTRUE));
-    if (!last->AsOp()->gtOp1->OperIsCmpCompare() || (last->gtFlags & GTF_SIDE_EFFECT) != 0 ||
-        (last->AsOp()->gtOp1->gtFlags & GTF_COLON_COND) != 0)
+    if (!last->gtGetOp1()->OperIsCmpCompare() || (last->gtFlags & GTF_SIDE_EFFECT) != 0 ||
+        (last->gtGetOp1()->gtFlags & GTF_COLON_COND) != 0)
     {
         return false;
     }
@@ -4705,7 +4657,6 @@ bool Compiler::optIfConvert(BasicBlock* block)
     BasicBlock* middleBlock = block;
     GenTree*    asgNode     = nullptr;
     Statement*  asgStmt     = nullptr;
-    bool        foundSelect = false;
 
     // Check the block is followed by a block or chain of blocks that only contain NOPs and
     // a single ASG statement. The destination of the final block must point to the same as the
@@ -4747,23 +4698,19 @@ bool Compiler::optIfConvert(BasicBlock* block)
                     // Ensure the destination of the assign is a local variable with integer type,
                     // and the nodes of the assign won't cause any additional side effects.
                     if (asgNode != nullptr || !op1->OperIs(GT_LCL_VAR) || !varTypeIsIntegralOrI(op1) ||
-                        (op1->gtFlags & GTF_SIDE_EFFECT) != 0 || (op2->gtFlags & GTF_SIDE_EFFECT) != 0)
+                        (op1->gtFlags & GTF_SIDE_EFFECT) != 0 || (op2->gtFlags & GTF_SIDE_EFFECT) != 0 ||
+                        op2->OperIs(GT_SELECT))
                     {
                         return false;
                     }
                     asgNode = tree;
                     asgStmt = stmt;
-
-                    if (op2->OperIs(GT_SELECT))
-                    {
-                        foundSelect = true;
-                    }
                     break;
                 }
 
                 // These do not need conditional execution.
                 case GT_NOP:
-                    if (tree->gtGetOp1() != nullptr || (tree->gtFlags & GTF_SIDE_EFFECT) != 0 )
+                    if (tree->gtGetOp1() != nullptr || (tree->gtFlags & GTF_SIDE_EFFECT) != 0)
                     {
                         return false;
                     }
@@ -4784,7 +4731,7 @@ bool Compiler::optIfConvert(BasicBlock* block)
 #ifdef DEBUG
     if (verbose)
     {
-        JITDUMP("Attempting to conditionally execute " FMT_BB " from " FMT_BB "\n", middleBlock->bbNum, block->bbNum);
+        JITDUMP("Conditionally executing " FMT_BB " from " FMT_BB "\n", middleBlock->bbNum, block->bbNum);
         for (BasicBlock* dumpBlock = block; dumpBlock != middleBlock->bbNext; dumpBlock = dumpBlock->bbNext)
         {
             fgDumpBlock(dumpBlock);
@@ -4793,22 +4740,28 @@ bool Compiler::optIfConvert(BasicBlock* block)
     }
 #endif
 
-    if (foundSelect)
-    {
-        // The assign is already conditional. Try adding another condition.
-        if (!extendConditionalAssignment(asgNode, last->gtGetOp1()))
-        {
-            return false;
-        }
-    }
-    else
-    {
-        // Make the assign conditional.
-        if (!createConditionalAssignment(asgNode, last->gtGetOp1()))
-        {
-            return false;
-        }
-    }
+    // Duplicate the input of the assign.
+    // This will be used as the false result of the select node.
+    GenTree* destination = gtCloneExpr(asgNode->AsOp()->gtOp1);
+    destination->gtFlags &= GTF_EMPTY;
+
+    // Duplicate the condition and invert it
+    GenTree* cond = gtCloneExpr(last->gtGetOp1());
+    cond->gtFlags |= GTF_DONT_CSE;
+    cond->gtFlags ^= GTF_RELOP_JMP_USED;
+    cond->gtFlags ^= GTF_RELOP_NAN_UN;
+    cond->gtOper = GenTree::ReverseRelop(cond->gtOper);
+
+    // Create a select node.
+    GenTreeConditional* select =
+        gtNewConditionalNode(GT_SELECT, cond, asgNode->gtGetOp2(), destination, asgNode->TypeGet());
+
+    // Use the select as the source of the assignment.
+    asgNode->AsOp()->gtOp2 = select;
+    asgNode->AsOp()->gtFlags |= (select->gtFlags & GTF_ALL_EFFECT);
+
+    // Calculate costs.
+    gtSetEvalOrder(select);
 
     // Remove the JTRUE statement.
     last->ReplaceWith(gtNewNothingNode(), this);
@@ -4837,109 +4790,6 @@ bool Compiler::optIfConvert(BasicBlock* block)
 #endif
 }
 
-//-----------------------------------------------------------------------------
-// createConditionalAssignment
-//
-// Helper function for optIfConvert. Given an assignment, make the assignment
-// conditionally execute based on the given condition. This is done by adding
-// a SELECT node as the source of the assignment. The exisiting value of the
-// assignment is used as the false path of the select
-// For example: (ASG Var Val) becomes (ASG Var (SELECT condition Val Var))
-//
-// Arguments:
-//   asg -- The assignment to conditionally execute.
-//   condition -- The assignment should only happen if this evaluates to true.
-//
-// Returns:
-//   true if any IR changes possibly made.
-//
-bool Compiler::createConditionalAssignment(GenTree* asg, GenTree* condition)
-{
-    assert(condition->OperIsCmpCompare());
-    assert(asg->OperIs(GT_ASG));
-
-    // Duplicate the input of the assign.
-    // This will be used as the false result of the select node.
-    GenTree* destination = gtCloneExpr(asg->AsOp()->gtOp1);
-    destination->gtFlags &= GTF_EMPTY;
-
-    // Duplicate the condition and invert it
-    GenTree* cond = gtCloneExpr(condition);
-    cond->gtFlags |= GTF_DONT_CSE;
-    cond->gtFlags ^= GTF_RELOP_JMP_USED;
-    cond->gtFlags ^= GTF_RELOP_NAN_UN;
-    cond->gtOper = GenTree::ReverseRelop(cond->gtOper);
-
-    // Create a select node.
-    GenTreeConditional* select =
-        gtNewConditionalNode(GT_SELECT, cond, asg->AsOp()->gtOp2, destination, asg->TypeGet());
-
-    // Use the select as the source of the assignment.
-    asg->AsOp()->gtOp2 = select;
-    asg->AsOp()->gtFlags |= (select->gtFlags & GTF_ALL_EFFECT);
-
-    // Calculate costs.
-    gtSetEvalOrder(select);
-
-    return true;
-}
-
-//-----------------------------------------------------------------------------
-// extendConditionalAssignment
-//
-// Helper function for optIfConvert. Given an assignment, which is already executed
-// conditionally via a SELECT node, add an additional condition.
-// For example: (ASG Var (SELECT oldcondition Val Var)) becomes
-// (ASG Var (SELECT (AND oldcondition newcondition) Val Var))
-//
-// Arguments:
-//   asg -- The conditional assignment to extend.
-//   condition -- The assignment should only happen if this evaluates to true.
-//
-// Returns:
-//   true if any IR changes possibly made.
-//
-bool Compiler::extendConditionalAssignment(GenTree* asg, GenTree* originalCondition)
-{
-    assert(originalCondition->OperIsCmpCompare());
-    assert(asg->OperIs(GT_ASG));
-
-    // TODO-CQ: Add float support.
-    if (!varTypeIsIntegralOrI(originalCondition->AsOp()->gtOp1->TypeGet()))
-    {
-        return false;
-    }
-
-    // Note: Limiting the maximum number of chained compares may give a performance
-    //       boost, at the cost of more emitted code.
-
-    GenTreeConditional* selectNode     = asg->AsOp()->gtOp2->AsConditional();
-    GenTree*            selectNodeCond = selectNode->gtCond;
-
-    // Duplicate the condition and invert it
-    GenTree* newCond = gtCloneExpr(originalCondition);
-    newCond->gtFlags |= GTF_DONT_CSE;
-    newCond->gtFlags ^= GTF_RELOP_JMP_USED;
-    newCond->gtFlags ^= GTF_RELOP_NAN_UN;
-    newCond->gtOper = GenTree::ReverseRelop(newCond->gtOper);
-
-    // Link together the two conditions.
-    GenTree* const link = gtNewOperNode(GT_AND, TYP_INT, selectNodeCond, newCond);
-    link->gtFlags |= (selectNodeCond->gtFlags & GTF_ALL_EFFECT);
-    link->gtFlags |= (newCond->gtFlags & GTF_ALL_EFFECT);
-
-    // Use the link as the conditional input to the select.
-    selectNode->gtCond = link;
-    selectNode->gtFlags |= (link->gtFlags & GTF_ALL_EFFECT);
-    selectNode->gtFlags ^= GTF_RELOP_NAN_UN;
-    asg->AsOp()->gtFlags |= (selectNode->gtFlags & GTF_ALL_EFFECT);
-
-    // Calculate costs.
-    gtSetEvalOrder(link);
-
-    return true;
-}
-
 //-----------------------------------------------------------------------------
 // optIfConversion: If conversion
 //

From c1281b561184199653a2a3d09be23effc2dc8518 Mon Sep 17 00:00:00 2001
From: Alan Hayward <alan.hayward@arm.com>
Date: Fri, 12 Aug 2022 12:16:54 +0100
Subject: [PATCH 08/38] Update middleBlock flow

---
 src/coreclr/jit/optimizer.cpp | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp
index 546e50fee7ace..5b705e691f064 100644
--- a/src/coreclr/jit/optimizer.cpp
+++ b/src/coreclr/jit/optimizer.cpp
@@ -4654,7 +4654,7 @@ bool Compiler::optIfConvert(BasicBlock* block)
         return false;
     }
 
-    BasicBlock* middleBlock = block;
+    BasicBlock* middleBlock = block->bbNext;
     GenTree*    asgNode     = nullptr;
     Statement*  asgStmt     = nullptr;
 
@@ -4664,8 +4664,8 @@ bool Compiler::optIfConvert(BasicBlock* block)
     bool foundMiddle = false;
     while (!foundMiddle)
     {
-        middleBlock = middleBlock->bbNext;
         noway_assert(middleBlock != nullptr);
+        BasicBlock* const middleNext = middleBlock->GetUniqueSucc();
 
         if (middleBlock->bbNext == block->bbJumpDest)
         {
@@ -4673,12 +4673,19 @@ bool Compiler::optIfConvert(BasicBlock* block)
             foundMiddle = true;
         }
 
-        // Make sure there is only one block which jumps to the middle block,
-        // and the middle block is not the start of a TRY block or an exception handler.
-        noway_assert(!fgCheapPredsValid);
-        if (middleBlock->NumSucc() != 1 || middleBlock->bbJumpKind != BBJ_NONE ||
-            middleBlock->bbPreds->flNext != nullptr || middleBlock->bbCatchTyp != BBCT_NONE ||
-            bbIsTryBeg(middleBlock) || bbIsHandlerBeg(middleBlock) || (middleBlock->bbFlags & BBF_DONT_REMOVE) != 0)
+        // Check that we have linear flow and are still in the same EH region
+        //
+        if (middleBlock->NumSucc() != 1 || middleBlock->bbJumpKind != BBJ_NONE)
+        {
+            return false;
+        }
+
+        if (middleBlock->GetUniquePred(this) == nullptr)
+        {
+            return false;
+        }
+
+        if (!BasicBlock::sameEHRegion(middleBlock, block))
         {
             return false;
         }
@@ -4721,6 +4728,8 @@ bool Compiler::optIfConvert(BasicBlock* block)
                     return false;
             }
         }
+
+        middleBlock = middleNext;
     }
     if (asgNode == nullptr)
     {

From b900344510b161a443d4366fbfe4597a27b81fab Mon Sep 17 00:00:00 2001
From: Alan Hayward <alan.hayward@arm.com>
Date: Fri, 12 Aug 2022 13:13:26 +0100
Subject: [PATCH 09/38] Check for order side effects

---
 src/coreclr/jit/optimizer.cpp | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp
index 5b705e691f064..2900714282035 100644
--- a/src/coreclr/jit/optimizer.cpp
+++ b/src/coreclr/jit/optimizer.cpp
@@ -4702,14 +4702,24 @@ bool Compiler::optIfConvert(BasicBlock* block)
                     GenTree* op2 = tree->gtGetOp2();
 
                     // Only one per assignment per block can be conditionally executed.
-                    // Ensure the destination of the assign is a local variable with integer type,
-                    // and the nodes of the assign won't cause any additional side effects.
-                    if (asgNode != nullptr || !op1->OperIs(GT_LCL_VAR) || !varTypeIsIntegralOrI(op1) ||
-                        (op1->gtFlags & GTF_SIDE_EFFECT) != 0 || (op2->gtFlags & GTF_SIDE_EFFECT) != 0 ||
-                        op2->OperIs(GT_SELECT))
+                    if (asgNode != nullptr || op2->OperIs(GT_SELECT))
                     {
                         return false;
                     }
+
+                    // Ensure the destination of the assign is a local variable with integer type.
+                    if (!op1->OperIs(GT_LCL_VAR) || !varTypeIsIntegralOrI(op1))
+                    {
+                        return false;
+                    }
+
+                    // Ensure the nodes of the assign won't cause any additional side effects.
+                    if ((op1->gtFlags & (GTF_SIDE_EFFECT | GTF_ORDER_SIDEEFF)) != 0 ||
+                        (op2->gtFlags & (GTF_SIDE_EFFECT | GTF_ORDER_SIDEEFF)) != 0)
+                    {
+                        return false;
+                    }
+
                     asgNode = tree;
                     asgStmt = stmt;
                     break;
@@ -4717,7 +4727,7 @@ bool Compiler::optIfConvert(BasicBlock* block)
 
                 // These do not need conditional execution.
                 case GT_NOP:
-                    if (tree->gtGetOp1() != nullptr || (tree->gtFlags & GTF_SIDE_EFFECT) != 0)
+                    if (tree->gtGetOp1() != nullptr || (tree->gtFlags & (GTF_SIDE_EFFECT | GTF_ORDER_SIDEEFF)) != 0)
                     {
                         return false;
                     }

From a3e89b90c9faa352aa0c505a54538695a259b47f Mon Sep 17 00:00:00 2001
From: Alan Hayward <alan.hayward@arm.com>
Date: Wed, 31 Aug 2022 15:06:47 +0100
Subject: [PATCH 10/38] Remove COLON_COND check

---
 src/coreclr/jit/optimizer.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp
index 2900714282035..9832c7858d46c 100644
--- a/src/coreclr/jit/optimizer.cpp
+++ b/src/coreclr/jit/optimizer.cpp
@@ -4648,8 +4648,7 @@ bool Compiler::optIfConvert(BasicBlock* block)
     // Verify the test block ends with a conditional that we can manipulate.
     GenTree* last = block->lastStmt()->GetRootNode();
     noway_assert(last->OperIs(GT_JTRUE));
-    if (!last->gtGetOp1()->OperIsCmpCompare() || (last->gtFlags & GTF_SIDE_EFFECT) != 0 ||
-        (last->gtGetOp1()->gtFlags & GTF_COLON_COND) != 0)
+    if (!last->gtGetOp1()->OperIsCmpCompare() || (last->gtFlags & GTF_SIDE_EFFECT) != 0)
     {
         return false;
     }

From 441a60cef5bd0585b302252c3cdf22e1483a3ac2 Mon Sep 17 00:00:00 2001
From: Alan Hayward <alan.hayward@arm.com>
Date: Wed, 31 Aug 2022 15:38:47 +0100
Subject: [PATCH 11/38] Remove flag toggling

---
 src/coreclr/jit/optimizer.cpp | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp
index 9832c7858d46c..382c255e7400d 100644
--- a/src/coreclr/jit/optimizer.cpp
+++ b/src/coreclr/jit/optimizer.cpp
@@ -4765,9 +4765,6 @@ bool Compiler::optIfConvert(BasicBlock* block)
 
     // Duplicate the condition and invert it
     GenTree* cond = gtCloneExpr(last->gtGetOp1());
-    cond->gtFlags |= GTF_DONT_CSE;
-    cond->gtFlags ^= GTF_RELOP_JMP_USED;
-    cond->gtFlags ^= GTF_RELOP_NAN_UN;
     cond->gtOper = GenTree::ReverseRelop(cond->gtOper);
 
     // Create a select node.

From 962c83bba709e37e33b00bd06c823a3e46f3e8e9 Mon Sep 17 00:00:00 2001
From: Alan Hayward <alan.hayward@arm.com>
Date: Wed, 31 Aug 2022 15:06:25 +0100
Subject: [PATCH 12/38] Move the conditional assignment to the JTRUE block

---
 src/coreclr/jit/optimizer.cpp | 92 +++++++++++++++++++++++------------
 1 file changed, 62 insertions(+), 30 deletions(-)

diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp
index 382c255e7400d..a69ac9742ea2a 100644
--- a/src/coreclr/jit/optimizer.cpp
+++ b/src/coreclr/jit/optimizer.cpp
@@ -4585,13 +4585,14 @@ PhaseStatus Compiler::optUnrollLoops()
 // statement. For example:
 // if (x < 7) { a = 5; }
 //
-// This is represented in IR by two basic blocks. The first block ends with a
-// JTRUE statement which conditionally jumps to the second block. Both blocks
-// then jump to the same destination. The second block just contains a single
-// assign statement. (Note that the first block may contain additional statements
-// prior to the JTRUE statement.)
+// This is represented in IR by two basic blocks. The first block (block) ends with
+// a JTRUE statement which conditionally jumps to the second block (middleBlock).
+// The second block just contains a single assign statement. Both blocks then jump
+// to the same destination (middleNext).  Note that the first block may contain
+// additional statements prior to the JTRUE statement.
 //
 // For example:
+//
 // ------------ BB03 [009..00D) -> BB05 (cond), preds={BB02} succs={BB04,BB05}
 // STMT00004
 //   *  JTRUE     void   $VN.Void
@@ -4605,15 +4606,20 @@ PhaseStatus Compiler::optUnrollLoops()
 // +--*  LCL_VAR   int    V00 arg0
 // \--*  CNS_INT   int    5 $47
 //
-// The JTRUE statement in the first block is replace with a SELECT node in the
-// assignment in the second block. If the result of the SELECTs op1 is true,
-// then op2 is passed to the assignment. Otherwise op3 is used. For example:
 //
-// ------------ BB03 [009..00D), preds={BB02} succs={BB04}
+// This is optimised by conditionally executing the store and removing the conditional
+// jumps. First the JTRUE is replaced with a NOP. The assignment is updated so that
+// the source of the store is a SELECT node with the condition set to the inverse of
+// the original JTRUE condition. If the condition passes the original assign happens,
+// otherwise the existing source value is used.
+//
+// In the example above, local var 0 is set to 5 if the LT returns true, otherwise
+// the existing value of local var 0 is used:
+//
+// ------------ BB03 [009..00D) -> BB05 (always), preds={BB02} succs={BB05}
 // STMT00004
 //   *  NOP       void
 //
-// ------------ BB04 [00D..010), preds={BB03} succs={BB05}
 // STMT00005
 //   *  ASG       int    $VN.Void
 //   +--*  LCL_VAR   int    V00 arg0
@@ -4624,6 +4630,7 @@ PhaseStatus Compiler::optUnrollLoops()
 //      +--*  CNS_INT   int    5 $47
 //      \--*  LCL_VAR   int    V00
 //
+// ------------ BB04 [00D..010), preds={} succs={BB05}
 //
 bool Compiler::optIfConvert(BasicBlock* block)
 {
@@ -4653,7 +4660,8 @@ bool Compiler::optIfConvert(BasicBlock* block)
         return false;
     }
 
-    BasicBlock* middleBlock = block->bbNext;
+    BasicBlock* middleBlock = nullptr;
+    BasicBlock* middleNext  = block->bbNext;
     GenTree*    asgNode     = nullptr;
     Statement*  asgStmt     = nullptr;
 
@@ -4663,21 +4671,23 @@ bool Compiler::optIfConvert(BasicBlock* block)
     bool foundMiddle = false;
     while (!foundMiddle)
     {
+        middleBlock = middleNext;
         noway_assert(middleBlock != nullptr);
-        BasicBlock* const middleNext = middleBlock->GetUniqueSucc();
 
-        if (middleBlock->bbNext == block->bbJumpDest)
+        // middleBlock should have a single successor.
+        middleNext = middleBlock->GetUniqueSucc();
+        if (middleNext == nullptr)
+        {
+            return false;
+        }
+
+        if (middleNext == block->bbJumpDest)
         {
             // This is our final middle block.
             foundMiddle = true;
         }
 
         // Check that we have linear flow and are still in the same EH region
-        //
-        if (middleBlock->NumSucc() != 1 || middleBlock->bbJumpKind != BBJ_NONE)
-        {
-            return false;
-        }
 
         if (middleBlock->GetUniquePred(this) == nullptr)
         {
@@ -4737,8 +4747,6 @@ bool Compiler::optIfConvert(BasicBlock* block)
                     return false;
             }
         }
-
-        middleBlock = middleNext;
     }
     if (asgNode == nullptr)
     {
@@ -4749,7 +4757,7 @@ bool Compiler::optIfConvert(BasicBlock* block)
 #ifdef DEBUG
     if (verbose)
     {
-        JITDUMP("Conditionally executing " FMT_BB " from " FMT_BB "\n", middleBlock->bbNum, block->bbNum);
+        JITDUMP("Conditionally executing " FMT_BB " inside " FMT_BB "\n", middleBlock->bbNum, block->bbNum);
         for (BasicBlock* dumpBlock = block; dumpBlock != middleBlock->bbNext; dumpBlock = dumpBlock->bbNext)
         {
             fgDumpBlock(dumpBlock);
@@ -4770,24 +4778,48 @@ bool Compiler::optIfConvert(BasicBlock* block)
     // Create a select node.
     GenTreeConditional* select =
         gtNewConditionalNode(GT_SELECT, cond, asgNode->gtGetOp2(), destination, asgNode->TypeGet());
+    gtSetEvalOrder(select);
 
     // Use the select as the source of the assignment.
     asgNode->AsOp()->gtOp2 = select;
     asgNode->AsOp()->gtFlags |= (select->gtFlags & GTF_ALL_EFFECT);
-
-    // Calculate costs.
-    gtSetEvalOrder(select);
+    gtSetEvalOrder(asgNode);
+    fgSetStmtSeq(asgStmt);
 
     // Remove the JTRUE statement.
     last->ReplaceWith(gtNewNothingNode(), this);
-    fgSetStmtSeq(block->lastStmt());
     gtSetEvalOrder(last);
-    fgSetStmtSeq(asgStmt);
+    fgSetStmtSeq(block->lastStmt());
+
+    // Move the Asg to the end of the original block
+    Statement* stmtList1 = block->firstStmt();
+    Statement* stmtList2 = middleBlock->firstStmt();
+    Statement* stmtLast1 = block->lastStmt();
+    Statement* stmtLast2 = middleBlock->lastStmt();
+    stmtLast1->SetNextStmt(stmtList2);
+    stmtList2->SetPrevStmt(stmtLast1);
+    stmtList1->SetPrevStmt(stmtLast2);
+    middleBlock->bbStmtList = nullptr;
+
+    // Fix up the SSA of assignment destination.
+    if (asgNode->AsOp()->gtOp1->IsLocal())
+    {
+        GenTreeLclVarCommon* lclVar = asgNode->AsOp()->gtOp1->AsLclVarCommon();
+        unsigned             lclNum = lclVar->GetLclNum();
+        unsigned             ssaNum = lclVar->GetSsaNum();
+
+        if (ssaNum != SsaConfig::RESERVED_SSA_NUM)
+        {
+            LclSsaVarDsc* ssaDef = lvaTable[lclNum].GetPerSsaData(ssaNum);
+            noway_assert(ssaDef->GetBlock() == middleBlock);
+            ssaDef->SetBlock(block);
+        }
+    }
+
+    // Update the flow from the original block.
+    fgRemoveAllRefPreds(block->bbNext, block);
+    block->bbJumpKind = BBJ_ALWAYS;
 
-    // Update the flow.
-    fgRemoveAllRefPreds(block->bbJumpDest, block);
-    block->bbJumpKind = BBJ_NONE;
-    block->bbJumpDest = middleBlock;
 
 #ifdef DEBUG
     if (verbose)

From 90bff398552dc2164b837299a03f51c27008adc3 Mon Sep 17 00:00:00 2001
From: Alan Hayward <alan.hayward@arm.com>
Date: Fri, 2 Sep 2022 16:13:52 +0100
Subject: [PATCH 13/38] Fix formatting

---
 src/coreclr/jit/optimizer.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp
index a69ac9742ea2a..15f16471873d9 100644
--- a/src/coreclr/jit/optimizer.cpp
+++ b/src/coreclr/jit/optimizer.cpp
@@ -4773,7 +4773,7 @@ bool Compiler::optIfConvert(BasicBlock* block)
 
     // Duplicate the condition and invert it
     GenTree* cond = gtCloneExpr(last->gtGetOp1());
-    cond->gtOper = GenTree::ReverseRelop(cond->gtOper);
+    cond->gtOper  = GenTree::ReverseRelop(cond->gtOper);
 
     // Create a select node.
     GenTreeConditional* select =
@@ -4820,7 +4820,6 @@ bool Compiler::optIfConvert(BasicBlock* block)
     fgRemoveAllRefPreds(block->bbNext, block);
     block->bbJumpKind = BBJ_ALWAYS;
 
-
 #ifdef DEBUG
     if (verbose)
     {

From 36cf21d28a405196236081fde5a86753595b7f32 Mon Sep 17 00:00:00 2001
From: Alan Hayward <alan.hayward@arm.com>
Date: Wed, 7 Sep 2022 10:26:24 +0100
Subject: [PATCH 14/38] Allow conditions with side effects

---
 src/coreclr/jit/optimizer.cpp | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp
index 15f16471873d9..70fce3fffdb74 100644
--- a/src/coreclr/jit/optimizer.cpp
+++ b/src/coreclr/jit/optimizer.cpp
@@ -4652,10 +4652,11 @@ bool Compiler::optIfConvert(BasicBlock* block)
         return false;
     }
 
-    // Verify the test block ends with a conditional that we can manipulate.
+    // Verify the test block ends with a condition that we can manipulate.
     GenTree* last = block->lastStmt()->GetRootNode();
     noway_assert(last->OperIs(GT_JTRUE));
-    if (!last->gtGetOp1()->OperIsCmpCompare() || (last->gtFlags & GTF_SIDE_EFFECT) != 0)
+    GenTree* cond = last->gtGetOp1();
+    if (!cond->OperIsCmpCompare())
     {
         return false;
     }
@@ -4771,8 +4772,7 @@ bool Compiler::optIfConvert(BasicBlock* block)
     GenTree* destination = gtCloneExpr(asgNode->AsOp()->gtOp1);
     destination->gtFlags &= GTF_EMPTY;
 
-    // Duplicate the condition and invert it
-    GenTree* cond = gtCloneExpr(last->gtGetOp1());
+    // Invert the condition.
     cond->gtOper  = GenTree::ReverseRelop(cond->gtOper);
 
     // Create a select node.

From 127f9a65dbe8bb6a54c2c6d8e129b80069a8b42d Mon Sep 17 00:00:00 2001
From: Alan Hayward <alan.hayward@arm.com>
Date: Wed, 7 Sep 2022 15:38:34 +0100
Subject: [PATCH 15/38] Fix formatting

---
 src/coreclr/jit/optimizer.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp
index 70fce3fffdb74..06c8aff9779a6 100644
--- a/src/coreclr/jit/optimizer.cpp
+++ b/src/coreclr/jit/optimizer.cpp
@@ -4773,7 +4773,7 @@ bool Compiler::optIfConvert(BasicBlock* block)
     destination->gtFlags &= GTF_EMPTY;
 
     // Invert the condition.
-    cond->gtOper  = GenTree::ReverseRelop(cond->gtOper);
+    cond->gtOper = GenTree::ReverseRelop(cond->gtOper);
 
     // Create a select node.
     GenTreeConditional* select =

From d1f0862b8f94cef69fa261c1c1fd45fdfa75db4b Mon Sep 17 00:00:00 2001
From: Alan Hayward <alan.hayward@arm.com>
Date: Thu, 15 Sep 2022 12:08:07 +0100
Subject: [PATCH 16/38] Correct all moved SSA statements

---
 src/coreclr/jit/optimizer.cpp | 67 ++++++++++++++++++++++++-----------
 1 file changed, 47 insertions(+), 20 deletions(-)

diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp
index 06c8aff9779a6..36a5538b548a6 100644
--- a/src/coreclr/jit/optimizer.cpp
+++ b/src/coreclr/jit/optimizer.cpp
@@ -4767,18 +4767,41 @@ bool Compiler::optIfConvert(BasicBlock* block)
     }
 #endif
 
-    // Duplicate the input of the assign.
+    // Duplicate the destination of the assign.
     // This will be used as the false result of the select node.
-    GenTree* destination = gtCloneExpr(asgNode->AsOp()->gtOp1);
-    destination->gtFlags &= GTF_EMPTY;
+    assert(asgNode->AsOp()->gtOp1->IsLocal());
+    GenTreeLclVarCommon* destination = asgNode->AsOp()->gtOp1->AsLclVarCommon();
+    GenTree*             falseInput  = gtCloneExpr(destination);
+    falseInput->gtFlags &= GTF_EMPTY;
+
+    // Create a new SSA entry for the false result.
+    {
+        // Get the SSA num of the destination.
+        unsigned lclNum            = destination->GetLclNum();
+        unsigned destinationSsaNum = destination->GetSsaNum();
+        assert(destinationSsaNum != SsaConfig::RESERVED_SSA_NUM);
+        LclSsaVarDsc* destinationSsaDef = lvaGetDesc(lclNum)->GetPerSsaData(destinationSsaNum);
+
+        // Create a new SSA num.
+        unsigned newSsaNum = lvaGetDesc(lclNum)->lvPerSsaData.AllocSsaNum(getAllocator(CMK_SSA));
+        assert(newSsaNum != SsaConfig::RESERVED_SSA_NUM);
+        LclSsaVarDsc* newSsaDef = lvaGetDesc(lclNum)->GetPerSsaData(newSsaNum);
+
+        // Copy across the SSA data.
+        newSsaDef->SetBlock(destinationSsaDef->GetBlock());
+        newSsaDef->SetAssignment(destinationSsaDef->GetAssignment());
+        newSsaDef->m_vnPair = destinationSsaDef->m_vnPair;
+        falseInput->AsLclVarCommon()->SetSsaNum(newSsaNum);
+
+        fgValueNumberTree(falseInput);
+    }
 
     // Invert the condition.
     cond->gtOper = GenTree::ReverseRelop(cond->gtOper);
 
     // Create a select node.
     GenTreeConditional* select =
-        gtNewConditionalNode(GT_SELECT, cond, asgNode->gtGetOp2(), destination, asgNode->TypeGet());
-    gtSetEvalOrder(select);
+        gtNewConditionalNode(GT_SELECT, cond, asgNode->gtGetOp2(), falseInput, asgNode->TypeGet());
 
     // Use the select as the source of the assignment.
     asgNode->AsOp()->gtOp2 = select;
@@ -4791,6 +4814,25 @@ bool Compiler::optIfConvert(BasicBlock* block)
     gtSetEvalOrder(last);
     fgSetStmtSeq(block->lastStmt());
 
+    // Before moving anything, fix up any SSAs in the middle block
+    for (Statement* const stmt : middleBlock->Statements())
+    {
+        for (GenTree* const node : stmt->TreeList())
+        {
+            if (node->IsLocal())
+            {
+                GenTreeLclVarCommon* lclVar = node->AsLclVarCommon();
+                unsigned             lclNum = lclVar->GetLclNum();
+                unsigned             ssaNum = lclVar->GetSsaNum();
+                if (ssaNum != SsaConfig::RESERVED_SSA_NUM)
+                {
+                    LclSsaVarDsc* ssaDef = lvaGetDesc(lclNum)->GetPerSsaData(ssaNum);
+                    ssaDef->SetBlock(block);
+                }
+            }
+        }
+    }
+
     // Move the Asg to the end of the original block
     Statement* stmtList1 = block->firstStmt();
     Statement* stmtList2 = middleBlock->firstStmt();
@@ -4801,21 +4843,6 @@ bool Compiler::optIfConvert(BasicBlock* block)
     stmtList1->SetPrevStmt(stmtLast2);
     middleBlock->bbStmtList = nullptr;
 
-    // Fix up the SSA of assignment destination.
-    if (asgNode->AsOp()->gtOp1->IsLocal())
-    {
-        GenTreeLclVarCommon* lclVar = asgNode->AsOp()->gtOp1->AsLclVarCommon();
-        unsigned             lclNum = lclVar->GetLclNum();
-        unsigned             ssaNum = lclVar->GetSsaNum();
-
-        if (ssaNum != SsaConfig::RESERVED_SSA_NUM)
-        {
-            LclSsaVarDsc* ssaDef = lvaTable[lclNum].GetPerSsaData(ssaNum);
-            noway_assert(ssaDef->GetBlock() == middleBlock);
-            ssaDef->SetBlock(block);
-        }
-    }
-
     // Update the flow from the original block.
     fgRemoveAllRefPreds(block->bbNext, block);
     block->bbJumpKind = BBJ_ALWAYS;

From 852a5afdd66327d05b8abab13190da00ff72097d Mon Sep 17 00:00:00 2001
From: Alan Hayward <alan.hayward@arm.com>
Date: Thu, 15 Sep 2022 12:11:54 +0100
Subject: [PATCH 17/38] Add size costing check

---
 src/coreclr/jit/gentree.cpp   |  3 +++
 src/coreclr/jit/optimizer.cpp | 21 ++++++++++++++++++---
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp
index f30bfd283ba9b..de22fc09ba4fb 100644
--- a/src/coreclr/jit/gentree.cpp
+++ b/src/coreclr/jit/gentree.cpp
@@ -5804,6 +5804,9 @@ unsigned Compiler::gtSetEvalOrder(GenTree* tree)
             level = max(level, lvl2);
             costEx += tree->AsConditional()->gtOp2->GetCostEx();
             costSz += tree->AsConditional()->gtOp2->GetCostSz();
+
+            costEx += 1;
+            costSz += 1;
             break;
 
         default:
diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp
index 36a5538b548a6..8458248b6b59f 100644
--- a/src/coreclr/jit/optimizer.cpp
+++ b/src/coreclr/jit/optimizer.cpp
@@ -4755,18 +4755,33 @@ bool Compiler::optIfConvert(BasicBlock* block)
         return false;
     }
 
+    // Calculate size costs.
+    // First reclaculate existing costs to make sure they are up to date.
+    gtSetEvalOrder(last);
+    gtSetEvalOrder(asgNode);
+    // If the condition is against 0 then JTRUE will have no size cost as it'll be merged into the compare.
+    int currentCostSz =
+        asgNode->GetCostSz() + (cond->gtGetOp2()->IsIntegralConst(0) ? cond->GetCostSz() : last->GetCostSz());
+    int postOptCostSz =
+        asgNode->GetCostSz() + 1 /* GT_SELECT */ + cond->GetCostSz() + asgNode->AsOp()->gtOp1->GetCostSz();
+
 #ifdef DEBUG
     if (verbose)
     {
-        JITDUMP("Conditionally executing " FMT_BB " inside " FMT_BB "\n", middleBlock->bbNum, block->bbNum);
+        JITDUMP("\nConditionally executing " FMT_BB " inside " FMT_BB "\n", middleBlock->bbNum, block->bbNum);
         for (BasicBlock* dumpBlock = block; dumpBlock != middleBlock->bbNext; dumpBlock = dumpBlock->bbNext)
         {
             fgDumpBlock(dumpBlock);
         }
-        JITDUMP("\n");
     }
 #endif
 
+    if (postOptCostSz > currentCostSz)
+    {
+        JITDUMP("Aborting If Conversion: optimisation too expensive (%d>%d)\n", postOptCostSz, currentCostSz);
+        return false;
+    }
+
     // Duplicate the destination of the assign.
     // This will be used as the false result of the select node.
     assert(asgNode->AsOp()->gtOp1->IsLocal());
@@ -4850,7 +4865,7 @@ bool Compiler::optIfConvert(BasicBlock* block)
 #ifdef DEBUG
     if (verbose)
     {
-        JITDUMP("After if conversion\n");
+        JITDUMP("\nAfter if conversion\n");
         for (BasicBlock* dumpBlock = block; dumpBlock != middleBlock->bbNext; dumpBlock = dumpBlock->bbNext)
         {
             fgDumpBlock(dumpBlock);

From 6690566f2ab1c8ec171c0544dac04c2e254b78ef Mon Sep 17 00:00:00 2001
From: Alan Hayward <alan.hayward@arm.com>
Date: Fri, 16 Sep 2022 12:38:13 +0100
Subject: [PATCH 18/38] Only move middle block ssa defs

---
 src/coreclr/jit/optimizer.cpp | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp
index 8458248b6b59f..12f58823d6dac 100644
--- a/src/coreclr/jit/optimizer.cpp
+++ b/src/coreclr/jit/optimizer.cpp
@@ -4773,6 +4773,7 @@ bool Compiler::optIfConvert(BasicBlock* block)
         {
             fgDumpBlock(dumpBlock);
         }
+        JITDUMP("\n");
     }
 #endif
 
@@ -4842,7 +4843,11 @@ bool Compiler::optIfConvert(BasicBlock* block)
                 if (ssaNum != SsaConfig::RESERVED_SSA_NUM)
                 {
                     LclSsaVarDsc* ssaDef = lvaGetDesc(lclNum)->GetPerSsaData(ssaNum);
-                    ssaDef->SetBlock(block);
+                    if (ssaDef->GetBlock() == middleBlock)
+                    {
+                        JITDUMP("SSA def %d for V%02u moved from " FMT_BB " to " FMT_BB ".\n", ssaNum, lclNum, ssaDef->GetBlock()->bbNum, block->bbNum);
+                        ssaDef->SetBlock(block);
+                    }
                 }
             }
         }

From 58372e2f5ad840a8dc02209417ff4e323e2f49b0 Mon Sep 17 00:00:00 2001
From: Alan Hayward <alan.hayward@arm.com>
Date: Fri, 16 Sep 2022 15:11:05 +0100
Subject: [PATCH 19/38] Fix formatting

---
 src/coreclr/jit/optimizer.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp
index 12f58823d6dac..1c335778fdb74 100644
--- a/src/coreclr/jit/optimizer.cpp
+++ b/src/coreclr/jit/optimizer.cpp
@@ -4845,7 +4845,8 @@ bool Compiler::optIfConvert(BasicBlock* block)
                     LclSsaVarDsc* ssaDef = lvaGetDesc(lclNum)->GetPerSsaData(ssaNum);
                     if (ssaDef->GetBlock() == middleBlock)
                     {
-                        JITDUMP("SSA def %d for V%02u moved from " FMT_BB " to " FMT_BB ".\n", ssaNum, lclNum, ssaDef->GetBlock()->bbNum, block->bbNum);
+                        JITDUMP("SSA def %d for V%02u moved from " FMT_BB " to " FMT_BB ".\n", ssaNum, lclNum,
+                                ssaDef->GetBlock()->bbNum, block->bbNum);
                         ssaDef->SetBlock(block);
                     }
                 }

From 10a8f1f797c7c46b4c1a131db26cc0a17ed8f2f9 Mon Sep 17 00:00:00 2001
From: Alan Hayward <alan.hayward@arm.com>
Date: Tue, 20 Sep 2022 12:59:52 +0100
Subject: [PATCH 20/38] Fewer SSA assumptions

---
 src/coreclr/jit/compiler.h    |  3 ++
 src/coreclr/jit/optimizer.cpp |  9 +++--
 src/coreclr/jit/valuenum.cpp  | 66 ++++++++++++++++++++++-------------
 3 files changed, 48 insertions(+), 30 deletions(-)

diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h
index 267c75d0cda3f..ae5c8c10aca48 100644
--- a/src/coreclr/jit/compiler.h
+++ b/src/coreclr/jit/compiler.h
@@ -4875,6 +4875,9 @@ class Compiler
     // Does value-numbering for a block assignment.
     void fgValueNumberBlockAssignment(GenTree* tree);
 
+    // Does value-numbering for a variable definition that has SSA.
+    void fgValueNumberSsaVarDef(GenTreeLclVarCommon* lcl);
+
     // Does value-numbering for a cast tree.
     void fgValueNumberCastTree(GenTree* tree);
 
diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp
index 1c335778fdb74..459f637157d4c 100644
--- a/src/coreclr/jit/optimizer.cpp
+++ b/src/coreclr/jit/optimizer.cpp
@@ -4791,11 +4791,10 @@ bool Compiler::optIfConvert(BasicBlock* block)
     falseInput->gtFlags &= GTF_EMPTY;
 
     // Create a new SSA entry for the false result.
+    if (destination->HasSsaName())
     {
-        // Get the SSA num of the destination.
-        unsigned lclNum            = destination->GetLclNum();
-        unsigned destinationSsaNum = destination->GetSsaNum();
-        assert(destinationSsaNum != SsaConfig::RESERVED_SSA_NUM);
+        unsigned      lclNum            = destination->GetLclNum();
+        unsigned      destinationSsaNum = destination->GetSsaNum();
         LclSsaVarDsc* destinationSsaDef = lvaGetDesc(lclNum)->GetPerSsaData(destinationSsaNum);
 
         // Create a new SSA num.
@@ -4809,7 +4808,7 @@ bool Compiler::optIfConvert(BasicBlock* block)
         newSsaDef->m_vnPair = destinationSsaDef->m_vnPair;
         falseInput->AsLclVarCommon()->SetSsaNum(newSsaNum);
 
-        fgValueNumberTree(falseInput);
+        fgValueNumberSsaVarDef(falseInput->AsLclVarCommon());
     }
 
     // Invert the condition.
diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp
index aed856ff6c172..4b63839d930b7 100644
--- a/src/coreclr/jit/valuenum.cpp
+++ b/src/coreclr/jit/valuenum.cpp
@@ -8390,6 +8390,46 @@ void Compiler::fgValueNumberBlockAssignment(GenTree* tree)
     tree->gtVNPair         = vnStore->VNPWithExc(vnStore->VNPForVoid(), vnpExcSet);
 }
 
+//------------------------------------------------------------------------
+// fgValueNumberSsaVarDef: Perform value numbering for a variable definition that has SSA
+//
+// Arguments:
+//    lcl - the variable definition.
+//
+void Compiler::fgValueNumberSsaVarDef(GenTreeLclVarCommon* lcl)
+{
+    unsigned   lclNum = lcl->GetLclNum();
+    LclVarDsc* varDsc = lvaGetDesc(lclNum);
+
+    assert((lcl->gtFlags & GTF_VAR_DEF) == 0);
+    assert(lcl->HasSsaName());
+
+    // We expect all uses of promoted structs to be replaced with uses of their fields.
+    assert(lvaInSsa(lclNum) && !varDsc->CanBeReplacedWithItsField(this));
+
+    ValueNumPair wholeLclVarVNP = varDsc->GetPerSsaData(lcl->GetSsaNum())->m_vnPair;
+    assert(wholeLclVarVNP.BothDefined());
+
+    // Account for type mismatches.
+    if (genActualType(varDsc) != genActualType(lcl))
+    {
+        if (genTypeSize(varDsc) != genTypeSize(lcl))
+        {
+            assert((varDsc->TypeGet() == TYP_LONG) && lcl->TypeIs(TYP_INT));
+            lcl->gtVNPair = vnStore->VNPairForCast(wholeLclVarVNP, lcl->TypeGet(), varDsc->TypeGet());
+        }
+        else
+        {
+            assert((varDsc->TypeGet() == TYP_I_IMPL) && lcl->TypeIs(TYP_BYREF));
+            lcl->gtVNPair = wholeLclVarVNP;
+        }
+    }
+    else
+    {
+        lcl->gtVNPair = wholeLclVarVNP;
+    }
+}
+
 void Compiler::fgValueNumberTree(GenTree* tree)
 {
     genTreeOps oper = tree->OperGet();
@@ -8426,31 +8466,7 @@ void Compiler::fgValueNumberTree(GenTree* tree)
                 {
                     if (lcl->HasSsaName())
                     {
-                        // We expect all uses of promoted structs to be replaced with uses of their fields.
-                        assert(lvaInSsa(lclNum) && !varDsc->CanBeReplacedWithItsField(this));
-
-                        ValueNumPair wholeLclVarVNP = varDsc->GetPerSsaData(lcl->GetSsaNum())->m_vnPair;
-                        assert(wholeLclVarVNP.BothDefined());
-
-                        // Account for type mismatches.
-                        if (genActualType(varDsc) != genActualType(lcl))
-                        {
-                            if (genTypeSize(varDsc) != genTypeSize(lcl))
-                            {
-                                assert((varDsc->TypeGet() == TYP_LONG) && lcl->TypeIs(TYP_INT));
-                                lcl->gtVNPair =
-                                    vnStore->VNPairForCast(wholeLclVarVNP, lcl->TypeGet(), varDsc->TypeGet());
-                            }
-                            else
-                            {
-                                assert((varDsc->TypeGet() == TYP_I_IMPL) && lcl->TypeIs(TYP_BYREF));
-                                lcl->gtVNPair = wholeLclVarVNP;
-                            }
-                        }
-                        else
-                        {
-                            lcl->gtVNPair = wholeLclVarVNP;
-                        }
+                        fgValueNumberSsaVarDef(lcl);
                     }
                     else if (varDsc->IsAddressExposed())
                     {

From 97b1af12615228fb8f96fe5bfe3cd3c85fa08f56 Mon Sep 17 00:00:00 2001
From: Alan Hayward <alan.hayward@arm.com>
Date: Wed, 21 Sep 2022 11:55:50 +0100
Subject: [PATCH 21/38] Use implicit func for value numbering

---
 src/coreclr/jit/valuenum.cpp | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp
index 4b63839d930b7..d681363ed08bc 100644
--- a/src/coreclr/jit/valuenum.cpp
+++ b/src/coreclr/jit/valuenum.cpp
@@ -9095,16 +9095,26 @@ void Compiler::fgValueNumberTree(GenTree* tree)
             {
                 GenTreeConditional* const conditional = tree->AsConditional();
 
-                ValueNumPair vnpExcSet = ValueNumStore::VNPForEmptyExcSet();
+                ValueNumPair condvnp;
+                ValueNumPair condXvnp;
+                vnStore->VNPUnpackExc(conditional->gtCond->gtVNPair, &condvnp, &condXvnp);
 
-                // Collect the exception sets from our operands
-                vnpExcSet = vnStore->VNPUnionExcSet(conditional->gtCond->gtVNPair, vnpExcSet);
-                vnpExcSet = vnStore->VNPUnionExcSet(conditional->gtOp1->gtVNPair, vnpExcSet);
-                vnpExcSet = vnStore->VNPUnionExcSet(conditional->gtOp2->gtVNPair, vnpExcSet);
+                ValueNumPair op1vnp;
+                ValueNumPair op1Xvnp;
+                vnStore->VNPUnpackExc(conditional->gtOp1->gtVNPair, &op1vnp, &op1Xvnp);
 
-                // The normal value is a new unique VN.
-                ValueNumPair normalPair;
-                normalPair.SetBoth(vnStore->VNForExpr(compCurBB, tree->TypeGet()));
+                ValueNumPair op2vnp;
+                ValueNumPair op2Xvnp;
+                vnStore->VNPUnpackExc(conditional->gtOp2->gtVNPair, &op2vnp, &op2Xvnp);
+
+                // Collect the exception sets.
+                ValueNumPair vnpExcSet = vnStore->VNPExcSetUnion(condXvnp, op1Xvnp);
+                vnpExcSet              = vnStore->VNPExcSetUnion(vnpExcSet, op2Xvnp);
+
+                // Get the normal value using the VN func.
+                VNFunc vnf = GetVNFuncForNode(tree);
+                assert(ValueNumStore::VNFuncIsLegal(vnf));
+                ValueNumPair normalPair = vnStore->VNPairForFunc(tree->TypeGet(), vnf, condvnp, op1vnp, op2vnp);
 
                 // Attach the combined exception set
                 tree->gtVNPair = vnStore->VNPWithExc(normalPair, vnpExcSet);

From 3f608514236514b2a08b4c69f09728330347b3b8 Mon Sep 17 00:00:00 2001
From: Alan Hayward <alan.hayward@arm.com>
Date: Thu, 6 Oct 2022 13:51:03 +0100
Subject: [PATCH 22/38] Update header for gtFoldExprConditional

---
 src/coreclr/jit/gentree.cpp | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp
index de22fc09ba4fb..0306f8e6200cb 100644
--- a/src/coreclr/jit/gentree.cpp
+++ b/src/coreclr/jit/gentree.cpp
@@ -12873,15 +12873,22 @@ GenTree* Compiler::gtFoldExprCompare(GenTree* tree)
     return cons;
 }
 
-/*****************************************************************************
- *
- * Some conditionals can be folded:
- *   SELECT TRUE  X Y  ->  X
- *   SELECT FALSE X Y  ->  Y
- *   SELECT COND X X   ->  X
- *
- */
-
+//------------------------------------------------------------------------
+// gtFoldExprConditional: see if a conditional is foldable
+//
+// Arguments:
+//    tree - condition to examine
+//
+// Returns:
+//    The original call if no folding happened.
+//    An alternative tree if folding happens.
+//
+// Notes:
+//    Supporting foldings are:
+//      SELECT TRUE  X Y  ->  X
+//      SELECT FALSE X Y  ->  Y
+//      SELECT COND  X X  ->  X
+//
 GenTree* Compiler::gtFoldExprConditional(GenTree* tree)
 {
     GenTree* cond = tree->AsConditional()->gtCond;

From 0f054fed10a8e9fd83f09cc167f2757818d8edeb Mon Sep 17 00:00:00 2001
From: Alan Hayward <alan.hayward@arm.com>
Date: Thu, 6 Oct 2022 16:09:59 +0100
Subject: [PATCH 23/38] Cost based on speed

---
 src/coreclr/jit/compiler.h    |  1 +
 src/coreclr/jit/optimizer.cpp | 17 +++++------------
 2 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h
index ae5c8c10aca48..7a58747426783 100644
--- a/src/coreclr/jit/compiler.h
+++ b/src/coreclr/jit/compiler.h
@@ -9479,6 +9479,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
         STRESS_MODE(CHK_REIMPORT)                                                               \
         STRESS_MODE(FLATFP)                                                                     \
         STRESS_MODE(GENERIC_CHECK)                                                              \
+        STRESS_MODE(IF_CONVERSION)                                                              \
         STRESS_MODE(COUNT)
 
     enum                compStressArea
diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp
index 459f637157d4c..25f24d96abc45 100644
--- a/src/coreclr/jit/optimizer.cpp
+++ b/src/coreclr/jit/optimizer.cpp
@@ -4755,16 +4755,6 @@ bool Compiler::optIfConvert(BasicBlock* block)
         return false;
     }
 
-    // Calculate size costs.
-    // First reclaculate existing costs to make sure they are up to date.
-    gtSetEvalOrder(last);
-    gtSetEvalOrder(asgNode);
-    // If the condition is against 0 then JTRUE will have no size cost as it'll be merged into the compare.
-    int currentCostSz =
-        asgNode->GetCostSz() + (cond->gtGetOp2()->IsIntegralConst(0) ? cond->GetCostSz() : last->GetCostSz());
-    int postOptCostSz =
-        asgNode->GetCostSz() + 1 /* GT_SELECT */ + cond->GetCostSz() + asgNode->AsOp()->gtOp1->GetCostSz();
-
 #ifdef DEBUG
     if (verbose)
     {
@@ -4777,9 +4767,12 @@ bool Compiler::optIfConvert(BasicBlock* block)
     }
 #endif
 
-    if (postOptCostSz > currentCostSz)
+    // Using SELECT nodes means that full assignment is always evaluated.
+    // Put a limit on the original source and destination of the assignment.
+    int costing = asgNode->gtGetOp1()->GetCostEx() + asgNode->gtGetOp2()->GetCostEx() - 2;
+    if (costing > 0 && !compStressCompile(STRESS_IF_CONVERSION, 0))
     {
-        JITDUMP("Aborting If Conversion: optimisation too expensive (%d>%d)\n", postOptCostSz, currentCostSz);
+        JITDUMP("Aborting If Conversion: optimisation too expensive (+%d)\n", costing);
         return false;
     }
 

From 393fd39bc44a2484d8e10bd3ddec88ea57e4d15e Mon Sep 17 00:00:00 2001
From: Alan Hayward <alan.hayward@arm.com>
Date: Fri, 7 Oct 2022 09:19:38 +0100
Subject: [PATCH 24/38] Add Stress mode for inner loops

---
 src/coreclr/jit/compiler.h    | 3 ++-
 src/coreclr/jit/optimizer.cpp | 4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h
index 54fb284c7d987..55f8d1a7e4512 100644
--- a/src/coreclr/jit/compiler.h
+++ b/src/coreclr/jit/compiler.h
@@ -9525,7 +9525,8 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
         STRESS_MODE(CHK_REIMPORT)                                                               \
         STRESS_MODE(FLATFP)                                                                     \
         STRESS_MODE(GENERIC_CHECK)                                                              \
-        STRESS_MODE(IF_CONVERSION)                                                              \
+        STRESS_MODE(IF_CONVERSION_COST)                                                         \
+        STRESS_MODE(IF_CONVERSION_INNER_LOOPS)                                                  \
         STRESS_MODE(COUNT)
 
     enum                compStressArea
diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp
index d4dca3a7ca4ef..54ebec9b5be44 100644
--- a/src/coreclr/jit/optimizer.cpp
+++ b/src/coreclr/jit/optimizer.cpp
@@ -4647,7 +4647,7 @@ bool Compiler::optIfConvert(BasicBlock* block)
     // Don't optimise the block if it is inside a loop
     // When inside a loop, branches are quicker than selects.
     // Detect via the block weight as that will be high when inside a loop.
-    if (block->getBBWeight(this) > BB_UNITY_WEIGHT)
+    if ((block->getBBWeight(this) > BB_UNITY_WEIGHT) && !compStressCompile(STRESS_IF_CONVERSION_INNER_LOOPS, 0))
     {
         return false;
     }
@@ -4776,7 +4776,7 @@ bool Compiler::optIfConvert(BasicBlock* block)
     // Using SELECT nodes means that full assignment is always evaluated.
     // Put a limit on the original source and destination of the assignment.
     int costing = asgNode->gtGetOp1()->GetCostEx() + asgNode->gtGetOp2()->GetCostEx() - 2;
-    if (costing > 0 && !compStressCompile(STRESS_IF_CONVERSION, 0))
+    if (costing > 0 && !compStressCompile(STRESS_IF_CONVERSION_COST, 0))
     {
         JITDUMP("Aborting If Conversion: optimisation too expensive (+%d)\n", costing);
         return false;

From 09d62fe7f4c1b6c7949ed0c04d1439db02e049fe Mon Sep 17 00:00:00 2001
From: Alan Hayward <alan.hayward@arm.com>
Date: Fri, 7 Oct 2022 14:11:31 +0100
Subject: [PATCH 25/38] Rework costings

---
 src/coreclr/jit/optimizer.cpp | 15 ++++++++++-----
 src/coreclr/jit/valuenum.cpp  |  2 +-
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp
index 54ebec9b5be44..b910e14bb5bb7 100644
--- a/src/coreclr/jit/optimizer.cpp
+++ b/src/coreclr/jit/optimizer.cpp
@@ -4647,7 +4647,7 @@ bool Compiler::optIfConvert(BasicBlock* block)
     // Don't optimise the block if it is inside a loop
     // When inside a loop, branches are quicker than selects.
     // Detect via the block weight as that will be high when inside a loop.
-    if ((block->getBBWeight(this) > BB_UNITY_WEIGHT) && !compStressCompile(STRESS_IF_CONVERSION_INNER_LOOPS, 0))
+    if ((block->getBBWeight(this) > BB_UNITY_WEIGHT) && !compStressCompile(STRESS_IF_CONVERSION_INNER_LOOPS, 25))
     {
         return false;
     }
@@ -4775,11 +4775,16 @@ bool Compiler::optIfConvert(BasicBlock* block)
 
     // Using SELECT nodes means that full assignment is always evaluated.
     // Put a limit on the original source and destination of the assignment.
-    int costing = asgNode->gtGetOp1()->GetCostEx() + asgNode->gtGetOp2()->GetCostEx() - 2;
-    if (costing > 0 && !compStressCompile(STRESS_IF_CONVERSION_COST, 0))
+    if (!compStressCompile(STRESS_IF_CONVERSION_COST, 25))
     {
-        JITDUMP("Aborting If Conversion: optimisation too expensive (+%d)\n", costing);
-        return false;
+        int cost = asgNode->gtGetOp2()->GetCostEx() + (gtIsLikelyRegVar(asgNode->gtGetOp1()) ? 0 : 2);
+
+        // Cost to allow for "x = cond ? a + b : x".
+        if (cost > 7)
+        {
+            JITDUMP("Skipping if-conversion that will evaluate RHS unconditionally at cost %d", cost);
+            return false;
+        }
     }
 
     // Duplicate the destination of the assign.
diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp
index 4ae5ca2360a0a..5f20c59c16a0b 100644
--- a/src/coreclr/jit/valuenum.cpp
+++ b/src/coreclr/jit/valuenum.cpp
@@ -8491,7 +8491,7 @@ void Compiler::fgValueNumberSsaVarDef(GenTreeLclVarCommon* lcl)
         else
         {
             assert(((varDsc->TypeGet() == TYP_I_IMPL) && lcl->TypeIs(TYP_BYREF)) ||
-                    ((varDsc->TypeGet() == TYP_BYREF) && lcl->TypeIs(TYP_I_IMPL)));
+                   ((varDsc->TypeGet() == TYP_BYREF) && lcl->TypeIs(TYP_I_IMPL)));
             lcl->gtVNPair = wholeLclVarVNP;
         }
     }

From 73007b25f1cd42a683ca2e665ea93371d70b9225 Mon Sep 17 00:00:00 2001
From: Alan Hayward <alan.hayward@arm.com>
Date: Mon, 10 Oct 2022 11:10:20 +0100
Subject: [PATCH 26/38] Check for invalid VNs

---
 src/coreclr/jit/optimizer.cpp | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp
index b910e14bb5bb7..f8c99969c4a7c 100644
--- a/src/coreclr/jit/optimizer.cpp
+++ b/src/coreclr/jit/optimizer.cpp
@@ -4812,7 +4812,10 @@ bool Compiler::optIfConvert(BasicBlock* block)
         newSsaDef->m_vnPair = destinationSsaDef->m_vnPair;
         falseInput->AsLclVarCommon()->SetSsaNum(newSsaNum);
 
-        fgValueNumberSsaVarDef(falseInput->AsLclVarCommon());
+        if (newSsaDef->m_vnPair.BothDefined())
+        {
+            fgValueNumberSsaVarDef(falseInput->AsLclVarCommon());
+        }
     }
 
     // Invert the condition.

From 61af92d1b9c87afcc5bbba2f6ec04804128a37d4 Mon Sep 17 00:00:00 2001
From: Alan Hayward <alan.hayward@arm.com>
Date: Mon, 10 Oct 2022 14:15:57 +0100
Subject: [PATCH 27/38] Ignore compares against zero

---
 src/coreclr/jit/optimizer.cpp | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp
index f8c99969c4a7c..4d8f4de97e5ed 100644
--- a/src/coreclr/jit/optimizer.cpp
+++ b/src/coreclr/jit/optimizer.cpp
@@ -4667,6 +4667,12 @@ bool Compiler::optIfConvert(BasicBlock* block)
         return false;
     }
 
+    // Skip compares against zero as they will be optimized via cbz etc.
+    if (cond->gtGetOp2()->IsIntegralConst(0) || cond->gtGetOp1()->IsIntegralConst(0))
+    {
+        return false;
+    }
+
     BasicBlock* middleBlock = nullptr;
     BasicBlock* middleNext  = block->bbNext;
     GenTree*    asgNode     = nullptr;

From 4fdabea68427a2d5ccf74359213c8c8ac4f6cd29 Mon Sep 17 00:00:00 2001
From: Alan Hayward <alan.hayward@arm.com>
Date: Wed, 12 Oct 2022 12:52:31 +0100
Subject: [PATCH 28/38] Ensure float compares are contained

---
 src/coreclr/jit/lowerarmarch.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/coreclr/jit/lowerarmarch.cpp b/src/coreclr/jit/lowerarmarch.cpp
index 206ca70794269..db5711aed74c3 100644
--- a/src/coreclr/jit/lowerarmarch.cpp
+++ b/src/coreclr/jit/lowerarmarch.cpp
@@ -2327,8 +2327,7 @@ bool Lowering::ContainCheckCompareChain(GenTree* child, GenTree* parent, GenTree
             child->SetContained();
             return true;
         }
-        else if (child->OperIsCmpCompare() && varTypeIsIntegral(child->gtGetOp1()) &&
-                 varTypeIsIntegral(child->gtGetOp2()))
+        else if (child->OperIsCmpCompare())
         {
             child->AsOp()->SetContained();
 

From be60d3036bca3d3b8e559f4c204d9ac631b2d173 Mon Sep 17 00:00:00 2001
From: Alan Hayward <alan.hayward@arm.com>
Date: Wed, 12 Oct 2022 15:26:42 +0100
Subject: [PATCH 29/38] Allow if conversion of test compares

---
 src/coreclr/jit/codegenarm64.cpp  |  2 +-
 src/coreclr/jit/codegenlinear.cpp |  2 +-
 src/coreclr/jit/gentree.cpp       |  4 ++--
 src/coreclr/jit/gentree.h         | 14 +-------------
 src/coreclr/jit/lowerarmarch.cpp  | 10 +++++-----
 src/coreclr/jit/lsrabuild.cpp     |  2 +-
 src/coreclr/jit/optimizer.cpp     |  8 +-------
 7 files changed, 12 insertions(+), 30 deletions(-)

diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp
index 7737953d21240..110caa6d062b4 100644
--- a/src/coreclr/jit/codegenarm64.cpp
+++ b/src/coreclr/jit/codegenarm64.cpp
@@ -4676,7 +4676,7 @@ void CodeGen::genCodeForContainedCompareChain(GenTree* tree, bool* inChain, GenC
     }
     else
     {
-        assert(tree->OperIsCmpCompare());
+        assert(tree->OperIsCompare());
 
         // Generate the compare, putting the result in the flags register.
         if (!*inChain)
diff --git a/src/coreclr/jit/codegenlinear.cpp b/src/coreclr/jit/codegenlinear.cpp
index da8cae58d0c28..b467a5da7e8bb 100644
--- a/src/coreclr/jit/codegenlinear.cpp
+++ b/src/coreclr/jit/codegenlinear.cpp
@@ -1629,7 +1629,7 @@ void CodeGen::genConsumeRegs(GenTree* tree)
             assert(cast->isContained());
             genConsumeAddress(cast->CastOp());
         }
-        else if (tree->OperIsCmpCompare() || tree->OperIs(GT_AND))
+        else if (tree->OperIsCompare() || tree->OperIs(GT_AND))
         {
             // Compares and ANDs may be contained in a conditional chain.
             genConsumeRegs(tree->gtGetOp1());
diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp
index dd55b320e36f8..9b6bc447a4be1 100644
--- a/src/coreclr/jit/gentree.cpp
+++ b/src/coreclr/jit/gentree.cpp
@@ -12945,7 +12945,7 @@ GenTree* Compiler::gtFoldExprConditional(GenTree* tree)
         JITDUMP("\n");
 
         // If we bashed to a compare, try to fold that.
-        if (replacement->OperIsCmpCompare())
+        if (replacement->OperIsCompare())
         {
             return gtFoldExprCompare(replacement);
         }
@@ -12953,7 +12953,7 @@ GenTree* Compiler::gtFoldExprConditional(GenTree* tree)
         return replacement;
     }
 
-    assert(cond->OperIsCmpCompare());
+    assert(cond->OperIsCompare());
 
     if (((tree->gtFlags & GTF_SIDE_EFFECT) != 0) || !GenTree::Compare(op1, op2, true))
     {
diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h
index 6b139b40b07cf..10e7659982a86 100644
--- a/src/coreclr/jit/gentree.h
+++ b/src/coreclr/jit/gentree.h
@@ -913,7 +913,7 @@ struct GenTree
     // Node and its child in isolation form a contained compare chain.
     bool isContainedCompareChainSegment(GenTree* child) const
     {
-        return (OperIs(GT_AND) && child->isContained() && (child->OperIs(GT_AND) || child->OperIsCmpCompare()));
+        return (OperIs(GT_AND) && child->isContained() && (child->OperIs(GT_AND) || child->OperIsCompare()));
     }
 
     bool isContainedFltOrDblImmed() const
@@ -1354,18 +1354,6 @@ struct GenTree
         return OperIsCompare(OperGet());
     }
 
-    // Oper is a compare that generates a cmp instruction (as opposed to a test instruction).
-    static bool OperIsCmpCompare(genTreeOps gtOper)
-    {
-        static_assert_no_msg(AreContiguous(GT_EQ, GT_NE, GT_LT, GT_LE, GT_GE, GT_GT));
-        return (GT_EQ <= gtOper) && (gtOper <= GT_GT);
-    }
-
-    bool OperIsCmpCompare() const
-    {
-        return OperIsCmpCompare(OperGet());
-    }
-
     static bool OperIsConditional(genTreeOps gtOper)
     {
         return (GT_SELECT == gtOper);
diff --git a/src/coreclr/jit/lowerarmarch.cpp b/src/coreclr/jit/lowerarmarch.cpp
index db5711aed74c3..a572fe146bad8 100644
--- a/src/coreclr/jit/lowerarmarch.cpp
+++ b/src/coreclr/jit/lowerarmarch.cpp
@@ -2266,7 +2266,7 @@ bool Lowering::IsValidCompareChain(GenTree* child, GenTree* parent)
         return IsValidCompareChain(child->AsOp()->gtGetOp2(), child) &&
                IsValidCompareChain(child->AsOp()->gtGetOp1(), child);
     }
-    else if (child->OperIsCmpCompare() && varTypeIsIntegral(child->gtGetOp1()) && varTypeIsIntegral(child->gtGetOp2()))
+    else if (child->OperIsCompare() && varTypeIsIntegral(child->gtGetOp1()) && varTypeIsIntegral(child->gtGetOp2()))
     {
         // Can the child compare be contained.
         return IsSafeToContainMem(parent, child);
@@ -2327,7 +2327,7 @@ bool Lowering::ContainCheckCompareChain(GenTree* child, GenTree* parent, GenTree
             child->SetContained();
             return true;
         }
-        else if (child->OperIsCmpCompare())
+        else if (child->OperIsCompare())
         {
             child->AsOp()->SetContained();
 
@@ -2373,7 +2373,7 @@ void Lowering::ContainCheckCompareChainForAnd(GenTree* tree)
                 if (startOfChain != nullptr)
                 {
                     // The earliest node in the chain will be generated as a standard compare.
-                    assert(startOfChain->OperIsCmpCompare());
+                    assert(startOfChain->OperIsCompare());
                     startOfChain->AsOp()->gtGetOp1()->ClearContained();
                     startOfChain->AsOp()->gtGetOp2()->ClearContained();
                     ContainCheckCompare(startOfChain->AsOp());
@@ -2394,7 +2394,7 @@ void Lowering::ContainCheckCompareChainForAnd(GenTree* tree)
 //
 void Lowering::ContainCheckConditionalCompare(GenTreeOp* cmp)
 {
-    assert(cmp->OperIsCmpCompare());
+    assert(cmp->OperIsCompare());
     GenTree* op2 = cmp->gtOp2;
 
     if (op2->IsCnsIntOrI() && !op2->AsIntCon()->ImmedValNeedsReloc(comp))
@@ -2428,7 +2428,7 @@ void Lowering::ContainCheckSelect(GenTreeConditional* node)
     if (startOfChain != nullptr)
     {
         // The earliest node in the chain will be generated as a standard compare.
-        assert(startOfChain->OperIsCmpCompare());
+        assert(startOfChain->OperIsCompare());
         startOfChain->AsOp()->gtGetOp1()->ClearContained();
         startOfChain->AsOp()->gtGetOp2()->ClearContained();
         ContainCheckCompare(startOfChain->AsOp());
diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp
index 370bfc393f23e..39e63c65beb3f 100644
--- a/src/coreclr/jit/lsrabuild.cpp
+++ b/src/coreclr/jit/lsrabuild.cpp
@@ -3151,7 +3151,7 @@ int LinearScan::BuildOperandUses(GenTree* node, regMaskTP candidates)
     }
 #endif // FEATURE_HW_INTRINSICS
 #ifdef TARGET_ARM64
-    if (node->OperIs(GT_MUL) || node->OperIsCmpCompare() || node->OperIs(GT_AND))
+    if (node->OperIs(GT_MUL) || node->OperIsCompare() || node->OperIs(GT_AND))
     {
         // MUL can be contained for madd or msub on arm64.
         // Compare and AND may be contained due to If Conversion.
diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp
index 4d8f4de97e5ed..34703be6db028 100644
--- a/src/coreclr/jit/optimizer.cpp
+++ b/src/coreclr/jit/optimizer.cpp
@@ -4662,13 +4662,7 @@ bool Compiler::optIfConvert(BasicBlock* block)
     GenTree* last = block->lastStmt()->GetRootNode();
     noway_assert(last->OperIs(GT_JTRUE));
     GenTree* cond = last->gtGetOp1();
-    if (!cond->OperIsCmpCompare())
-    {
-        return false;
-    }
-
-    // Skip compares against zero as they will be optimized via cbz etc.
-    if (cond->gtGetOp2()->IsIntegralConst(0) || cond->gtGetOp1()->IsIntegralConst(0))
+    if (!cond->OperIsCompare())
     {
         return false;
     }

From 806e0618858afdc20b99f43f588a8c117e5734b1 Mon Sep 17 00:00:00 2001
From: Alan Hayward <alan.hayward@arm.com>
Date: Thu, 13 Oct 2022 14:09:32 +0100
Subject: [PATCH 30/38] Do not contain test compares within compare chains

---
 src/coreclr/jit/codegenarm64.cpp |  3 +++
 src/coreclr/jit/gentree.h        | 12 ++++++++++++
 src/coreclr/jit/lowerarmarch.cpp |  5 +++--
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp
index 110caa6d062b4..cc6316ef08e27 100644
--- a/src/coreclr/jit/codegenarm64.cpp
+++ b/src/coreclr/jit/codegenarm64.cpp
@@ -4613,6 +4613,9 @@ void CodeGen::genCodeForConditionalCompare(GenTreeOp* tree, GenCondition prevCon
     // Should only be called on contained nodes.
     assert(targetReg == REG_NA);
 
+    // Should not be called for test conditionals (Arm64 does not have a ctst).
+    assert(tree->OperIsCmpCompare());
+
     // For the ccmp flags, invert the condition of the compare.
     insCflags cflags = InsCflagsForCcmp(GenCondition::FromRelop(tree));
 
diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h
index 10e7659982a86..b14204e45d957 100644
--- a/src/coreclr/jit/gentree.h
+++ b/src/coreclr/jit/gentree.h
@@ -1354,6 +1354,18 @@ struct GenTree
         return OperIsCompare(OperGet());
     }
 
+    // Oper is a compare that generates a cmp instruction (as opposed to a test instruction).
+    static bool OperIsCmpCompare(genTreeOps gtOper)
+    {
+        static_assert_no_msg(AreContiguous(GT_EQ, GT_NE, GT_LT, GT_LE, GT_GE, GT_GT));
+        return (GT_EQ <= gtOper) && (gtOper <= GT_GT);
+    }
+
+    bool OperIsCmpCompare() const
+    {
+        return OperIsCmpCompare(OperGet());
+    }
+
     static bool OperIsConditional(genTreeOps gtOper)
     {
         return (GT_SELECT == gtOper);
diff --git a/src/coreclr/jit/lowerarmarch.cpp b/src/coreclr/jit/lowerarmarch.cpp
index a572fe146bad8..4352a26b67635 100644
--- a/src/coreclr/jit/lowerarmarch.cpp
+++ b/src/coreclr/jit/lowerarmarch.cpp
@@ -2327,7 +2327,7 @@ bool Lowering::ContainCheckCompareChain(GenTree* child, GenTree* parent, GenTree
             child->SetContained();
             return true;
         }
-        else if (child->OperIsCompare())
+        else if (child->OperIsCmpCompare())
         {
             child->AsOp()->SetContained();
 
@@ -2340,6 +2340,7 @@ bool Lowering::ContainCheckCompareChain(GenTree* child, GenTree* parent, GenTree
         }
     }
 
+
     return false;
 }
 
@@ -2394,7 +2395,7 @@ void Lowering::ContainCheckCompareChainForAnd(GenTree* tree)
 //
 void Lowering::ContainCheckConditionalCompare(GenTreeOp* cmp)
 {
-    assert(cmp->OperIsCompare());
+    assert(cmp->OperIsCmpCompare());
     GenTree* op2 = cmp->gtOp2;
 
     if (op2->IsCnsIntOrI() && !op2->AsIntCon()->ImmedValNeedsReloc(comp))

From a508e857b2dd6d96f35b1df6655cf0e54598a1d0 Mon Sep 17 00:00:00 2001
From: Alan Hayward <alan.hayward@arm.com>
Date: Thu, 13 Oct 2022 14:16:13 +0100
Subject: [PATCH 31/38] Add float versions of the JIT/opt/Compares tests

---
 .../JIT/opt/Compares/compareAnd2Chains.cs     | 98 ++++++++++++++++++-
 .../JIT/opt/Compares/compareAnd3Chains.cs     | 96 ++++++++++++++++++
 .../JIT/opt/Compares/compareAndTestChains.cs  | 96 ++++++++++++++++++
 3 files changed, 289 insertions(+), 1 deletion(-)

diff --git a/src/tests/JIT/opt/Compares/compareAnd2Chains.cs b/src/tests/JIT/opt/Compares/compareAnd2Chains.cs
index 0fd39d96cbcfa..44ce6c88f6004 100644
--- a/src/tests/JIT/opt/Compares/compareAnd2Chains.cs
+++ b/src/tests/JIT/opt/Compares/compareAnd2Chains.cs
@@ -31,6 +31,12 @@ public class ComparisonTestAnd2Chains
     [MethodImpl(MethodImplOptions.NoInlining)]
     public static bool Eq_ulong_2(ulong a1, ulong a2) => a1 == 10 & a2 == 11;
 
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static bool Eq_float_2(float a1, float a2) => a1 == 10.5f & a2 == 11.5f;
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static bool Eq_double_2(double a1, double a2) => a1 == 10.5 & a2 == 11.5;
+
 
     [MethodImpl(MethodImplOptions.NoInlining)]
     public static bool Ne_byte_2(byte a1, byte a2) => a1 != 5 & a2 != 5;
@@ -53,6 +59,12 @@ public class ComparisonTestAnd2Chains
     [MethodImpl(MethodImplOptions.NoInlining)]
     public static bool Ne_ulong_2(ulong a1, ulong a2) => a1 != 5 & a2 != 5;
 
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static bool Ne_float_2(float a1, float a2) => a1 != 5.5f & a2 != 5.5f;
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static bool Ne_double_2(double a1, double a2) => a1 != 5.5 & a2 != 5.5;
+
 
     [MethodImpl(MethodImplOptions.NoInlining)]
     public static bool Lt_byte_2(byte a1, byte a2) => a1 < 5 & a2 < 5;
@@ -75,6 +87,12 @@ public class ComparisonTestAnd2Chains
     [MethodImpl(MethodImplOptions.NoInlining)]
     public static bool Lt_ulong_2(ulong a1, ulong a2) => a1 < 5 & a2 < 5;
 
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static bool Lt_float_2(float a1, float a2) => a1 < 5.5f & a2 < 5.5f;
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static bool Lt_double_2(double a1, double a2) => a1 < 5.5 & a2 < 5.5;
+
 
     [MethodImpl(MethodImplOptions.NoInlining)]
     public static bool Le_byte_2(byte a1, byte a2) => a1 <= 5 & a2 <= 5;
@@ -97,6 +115,12 @@ public class ComparisonTestAnd2Chains
     [MethodImpl(MethodImplOptions.NoInlining)]
     public static bool Le_ulong_2(ulong a1, ulong a2) => a1 <= 5 & a2 <= 5;
 
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static bool Le_float_2(float a1, float a2) => a1 <= 5.5f & a2 <= 5.5f;
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static bool Le_double_2(double a1, double a2) => a1 <= 5.5 & a2 <= 5.5;
+
 
     [MethodImpl(MethodImplOptions.NoInlining)]
     public static bool Gt_byte_2(byte a1, byte a2) => a1 > 5 & a2 > 5;
@@ -119,6 +143,12 @@ public class ComparisonTestAnd2Chains
     [MethodImpl(MethodImplOptions.NoInlining)]
     public static bool Gt_ulong_2(ulong a1, ulong a2) => a1 > 5 & a2 > 5;
 
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static bool Gt_float_2(float a1, float a2) => a1 > 5.5f & a2 > 5.5f;
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static bool Gt_double_2(double a1, double a2) => a1 > 5.5 & a2 > 5.5;
+
 
     [MethodImpl(MethodImplOptions.NoInlining)]
     public static bool Ge_byte_2(byte a1, byte a2) => a1 >= 5 & a2 >= 5;
@@ -141,6 +171,12 @@ public class ComparisonTestAnd2Chains
     [MethodImpl(MethodImplOptions.NoInlining)]
     public static bool Ge_ulong_2(ulong a1, ulong a2) => a1 >= 5 & a2 >= 5;
 
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static bool Ge_float_2(float a1, float a2) => a1 >= 5.5f & a2 >= 5.5f;
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static bool Ge_double_2(double a1, double a2) => a1 >= 5.5 & a2 >= 5.5;
+
 
     [MethodImpl(MethodImplOptions.NoInlining)]
     public static int Main()
@@ -180,6 +216,16 @@ public static int Main()
             Console.WriteLine("ComparisonTestAnd2Chains:Eq_ulong_2(10, 11) failed");
             return 101;
         }
+        if (!Eq_float_2(10.5f, 11.5f))
+        {
+            Console.WriteLine("ComparisonTestAnd2Chains:Eq_float_2(10.5, 11.5) failed");
+            return 101;
+        }
+        if (!Eq_double_2(10.5, 11.5))
+        {
+            Console.WriteLine("ComparisonTestAnd2Chains:Eq_ulong_2(10.5, 11.5) failed");
+            return 101;
+        }
 
         if (!Ne_byte_2(10, 11))
         {
@@ -216,6 +262,16 @@ public static int Main()
             Console.WriteLine("ComparisonTestAnd2Chains:Ne_ulong_2(10, 11) failed");
             return 101;
         }
+        if (!Ne_float_2(10.5f, 11.5f))
+        {
+            Console.WriteLine("ComparisonTestAnd2Chains:Ne_float_2(10.5, 11.5) failed");
+            return 101;
+        }
+        if (!Ne_double_2(10.5, 11.5))
+        {
+            Console.WriteLine("ComparisonTestAnd2Chains:Ne_double_2(10.5, 11.5) failed");
+            return 101;
+        }
 
         if (!Lt_byte_2(3, 4))
         {
@@ -252,6 +308,16 @@ public static int Main()
             Console.WriteLine("ComparisonTestAnd2Chains:Lt_ulong_2(3, 4) failed");
             return 101;
         }
+        if (!Lt_float_2(3f, 4f))
+        {
+            Console.WriteLine("ComparisonTestAnd2Chains:Lt_float_2(3.5, 4.5) failed");
+            return 101;
+        }
+        if (!Lt_double_2(3, 4))
+        {
+            Console.WriteLine("ComparisonTestAnd2Chains:Lt_double_2(3.5, 4.5) failed");
+            return 101;
+        }
 
         if (!Le_byte_2(3, 4))
         {
@@ -288,6 +354,16 @@ public static int Main()
             Console.WriteLine("ComparisonTestAnd2Chains:Le_ulong_2(3, 4) failed");
             return 101;
         }
+        if (!Le_float_2(3f, 4f))
+        {
+            Console.WriteLine("ComparisonTestAnd2Chains:Le_float_2(3.5, 4.5) failed");
+            return 101;
+        }
+        if (!Le_double_2(3, 4))
+        {
+            Console.WriteLine("ComparisonTestAnd2Chains:Le_double_2(3.5, 4.5) failed");
+            return 101;
+        }
 
         if (!Gt_byte_2(10, 11))
         {
@@ -324,6 +400,16 @@ public static int Main()
             Console.WriteLine("ComparisonTestAnd2Chains:Gt_ulong_2(10, 11) failed");
             return 101;
         }
+        if (!Gt_float_2(10.5f, 11.5f))
+        {
+            Console.WriteLine("ComparisonTestAnd2Chains:Gt_float_2(10.5, 11.5) failed");
+            return 101;
+        }
+        if (!Gt_double_2(10.5, 11.5))
+        {
+            Console.WriteLine("ComparisonTestAnd2Chains:Gt_ulong_2(10.5, 11.5) failed");
+            return 101;
+        }
 
         if (!Ge_byte_2(10, 11))
         {
@@ -357,7 +443,17 @@ public static int Main()
         }
         if (!Ge_ulong_2(10, 11))
         {
-            Console.WriteLine("ComparisonTestAnd2Chains:Le_ulong_2(10, 11) failed");
+            Console.WriteLine("ComparisonTestAnd2Chains:Ge_ulong_2(10, 11) failed");
+            return 101;
+        }
+        if (!Ge_float_2(10.5f, 11.5f))
+        {
+            Console.WriteLine("ComparisonTestAnd2Chains:Ge_float_2(10.5, 11.5) failed");
+            return 101;
+        }
+        if (!Ge_double_2(10.5, 11.5))
+        {
+            Console.WriteLine("ComparisonTestAnd2Chains:Ge_double_2(10.5, 11.5) failed");
             return 101;
         }
 
diff --git a/src/tests/JIT/opt/Compares/compareAnd3Chains.cs b/src/tests/JIT/opt/Compares/compareAnd3Chains.cs
index c609cdebc2d01..2f6dcfa84f6e9 100644
--- a/src/tests/JIT/opt/Compares/compareAnd3Chains.cs
+++ b/src/tests/JIT/opt/Compares/compareAnd3Chains.cs
@@ -31,6 +31,12 @@ public class ComparisonTestAnd3Chains
     [MethodImpl(MethodImplOptions.NoInlining)]
     public static bool Eq_ulong_3(ulong a1, ulong a2, ulong a3) => a1 == 10 & a2 == 11 & a3 == 12;
 
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static bool Eq_float_3(float a1, float a2, float a3) => a1 == 10.5f & a2 == 11.5f & a3 == 12.5f;
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static bool Eq_double_3(double a1, double a2, double a3) => a1 == 10.5 & a2 == 11.5 & a3 == 12.5;
+
 
     [MethodImpl(MethodImplOptions.NoInlining)]
     public static bool Ne_byte_3(byte a1, byte a2, byte a3) => a1 != 5 & a2 != 5 & a3 != 5;
@@ -53,6 +59,12 @@ public class ComparisonTestAnd3Chains
     [MethodImpl(MethodImplOptions.NoInlining)]
     public static bool Ne_ulong_3(ulong a1, ulong a2, ulong a3) => a1 != 5 & a2 != 5 & a3 != 5;
 
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static bool Ne_float_3(float a1, float a2, float a3) => a1 != 5.5f & a2 != 5.5f & a3 != 5.5f;
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static bool Ne_double_3(double a1, double a2, double a3) => a1 != 5.5 & a2 != 5.5 & a3 != 5.5;
+
 
     [MethodImpl(MethodImplOptions.NoInlining)]
     public static bool Lt_byte_3(byte a1, byte a2, byte a3) => a1 < 5 & a2 < 5 & a3 < 5;
@@ -75,6 +87,12 @@ public class ComparisonTestAnd3Chains
     [MethodImpl(MethodImplOptions.NoInlining)]
     public static bool Lt_ulong_3(ulong a1, ulong a2, ulong a3) => a1 < 5 & a2 < 5 & a3 < 5;
 
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static bool Lt_float_3(float a1, float a2, float a3) => a1 < 5.5f & a2 < 5.5f & a3 < 5.5f;
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static bool Lt_double_3(double a1, double a2, double a3) => a1 < 5.5 & a2 < 5.5 & a3 < 5.5;
+
 
     [MethodImpl(MethodImplOptions.NoInlining)]
     public static bool Le_byte_3(byte a1, byte a2, byte a3) => a1 <= 5 & a2 <= 5 & a3 <= 5;
@@ -97,6 +115,12 @@ public class ComparisonTestAnd3Chains
     [MethodImpl(MethodImplOptions.NoInlining)]
     public static bool Le_ulong_3(ulong a1, ulong a2, ulong a3) => a1 <= 5 & a2 <= 5 & a3 <= 5;
 
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static bool Le_float_3(float a1, float a2, float a3) => a1 <= 5.5f & a2 <= 5.5f & a3 <= 5.5f;
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static bool Le_double_3(double a1, double a2, double a3) => a1 <= 5.5 & a2 <= 5.5 & a3 <= 5.5;
+
 
     [MethodImpl(MethodImplOptions.NoInlining)]
     public static bool Gt_byte_3(byte a1, byte a2, byte a3) => a1 > 5 & a2 > 5 & a3 > 5;
@@ -119,6 +143,12 @@ public class ComparisonTestAnd3Chains
     [MethodImpl(MethodImplOptions.NoInlining)]
     public static bool Gt_ulong_3(ulong a1, ulong a2, ulong a3) => a1 > 5 & a2 > 5 & a3 > 5;
 
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static bool Gt_float_3(float a1, float a2, float a3) => a1 > 5.5f & a2 > 5.5f & a3 > 5.5f;
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static bool Gt_double_3(double a1, double a2, double a3) => a1 > 5.5 & a2 > 5.5 & a3 > 5.5;
+
 
     [MethodImpl(MethodImplOptions.NoInlining)]
     public static bool Ge_byte_3(byte a1, byte a2, byte a3) => a1 >= 5 & a2 >= 5 & a3 >= 5;
@@ -141,6 +171,12 @@ public class ComparisonTestAnd3Chains
     [MethodImpl(MethodImplOptions.NoInlining)]
     public static bool Ge_ulong_3(ulong a1, ulong a2, ulong a3) => a1 >= 5 & a2 >= 5 & a3 >= 5;
 
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static bool Ge_float_3(float a1, float a2, float a3) => a1 >= 5.5f & a2 >= 5.5f & a3 >= 5.5f;
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static bool Ge_double_3(double a1, double a2, double a3) => a1 >= 5.5 & a2 >= 5.5 & a3 >= 5.5;
+
 
     [MethodImpl(MethodImplOptions.NoInlining)]
     public static int Main()
@@ -180,6 +216,16 @@ public static int Main()
             Console.WriteLine("ComparisonTestAnd2Chains:Eq_ulong_3(10, 11, 12) failed");
             return 101;
         }
+        if (!Eq_float_3(10.5f, 11.5f, 12.5f))
+        {
+            Console.WriteLine("ComparisonTestAnd2Chains:Eq_float_3(10.5, 11.5, 12.5) failed");
+            return 101;
+        }
+        if (!Eq_double_3(10.5, 11.5, 12.5))
+        {
+            Console.WriteLine("ComparisonTestAnd2Chains:Eq_double_3(10.5, 11.5, 12.5) failed");
+            return 101;
+        }
 
         if (!Ne_byte_3(10, 11, 12))
         {
@@ -216,6 +262,16 @@ public static int Main()
             Console.WriteLine("ComparisonTestAnd2Chains:Ne_ulong_3(10, 11, 12) failed");
             return 101;
         }
+        if (!Ne_float_3(10.5f, 11.5f, 12.5f))
+        {
+            Console.WriteLine("ComparisonTestAnd2Chains:Ne_float_3(10.5, 11.5, 12.5) failed");
+            return 101;
+        }
+        if (!Ne_double_3(10.5, 11.5, 12.5))
+        {
+            Console.WriteLine("ComparisonTestAnd2Chains:Ne_double_3(10.5, 11.5, 12.5) failed");
+            return 101;
+        }
 
         if (!Lt_byte_3(2, 3, 4))
         {
@@ -252,6 +308,16 @@ public static int Main()
             Console.WriteLine("ComparisonTestAnd2Chains:Lt_ulong_3(2, 3, 4) failed");
             return 101;
         }
+        if (!Lt_float_3(2.5f, 3.5f, 4.5f))
+        {
+            Console.WriteLine("ComparisonTestAnd2Chains:Lt_float_3(2.5, 3.5, 4.5) failed");
+            return 101;
+        }
+        if (!Lt_double_3(2.5, 3.5, 4.5))
+        {
+            Console.WriteLine("ComparisonTestAnd2Chains:Lt_double_3(2.5, 3.5, 4.5) failed");
+            return 101;
+        }
 
         if (!Le_byte_3(2, 3, 4))
         {
@@ -288,6 +354,16 @@ public static int Main()
             Console.WriteLine("ComparisonTestAnd2Chains:Le_ulong_3(2, 3, 4) failed");
             return 101;
         }
+        if (!Le_float_3(2.5f, 3.5f, 4.5f))
+        {
+            Console.WriteLine("ComparisonTestAnd2Chains:Le_float_3(2.5, 3.5, 4.5) failed");
+            return 101;
+        }
+        if (!Le_double_3(2.5, 3.5, 4.5))
+        {
+            Console.WriteLine("ComparisonTestAnd2Chains:Le_double_3(2.5, 3.5, 4.5) failed");
+            return 101;
+        }
 
         if (!Gt_byte_3(10, 11, 12))
         {
@@ -324,6 +400,16 @@ public static int Main()
             Console.WriteLine("ComparisonTestAnd2Chains:Gt_ulong_3(10, 11, 12) failed");
             return 101;
         }
+        if (!Gt_float_3(10.5f, 11.5f, 12.5f))
+        {
+            Console.WriteLine("ComparisonTestAnd2Chains:Gt_float_3(10.5, 11.5, 12.5) failed");
+            return 101;
+        }
+        if (!Gt_double_3(10.5, 11.5, 12.5))
+        {
+            Console.WriteLine("ComparisonTestAnd2Chains:Gt_double_3(10.5, 11.5, 12.5) failed");
+            return 101;
+        }
 
         if (!Ge_byte_3(10, 11, 12))
         {
@@ -360,6 +446,16 @@ public static int Main()
             Console.WriteLine("ComparisonTestAnd2Chains:Le_ulong_3(10, 11, 12) failed");
             return 101;
         }
+        if (!Ge_float_3(10.5f, 11.5f, 12.5f))
+        {
+            Console.WriteLine("ComparisonTestAnd2Chains:Ge_float_3(10.5, 11.5, 12.5) failed");
+            return 101;
+        }
+        if (!Ge_double_3(10.5, 11.5, 12.5))
+        {
+            Console.WriteLine("ComparisonTestAnd2Chains:Le_double_3(10.5, 11.5, 12.5) failed");
+            return 101;
+        }
 
         Console.WriteLine("PASSED");
         return 100;
diff --git a/src/tests/JIT/opt/Compares/compareAndTestChains.cs b/src/tests/JIT/opt/Compares/compareAndTestChains.cs
index c89e04f487ecf..575a9372509d3 100644
--- a/src/tests/JIT/opt/Compares/compareAndTestChains.cs
+++ b/src/tests/JIT/opt/Compares/compareAndTestChains.cs
@@ -31,6 +31,12 @@ public class ComparisonTestAndTestChains
     [MethodImpl(MethodImplOptions.NoInlining)]
     public static bool Eq_ulong_bool(ulong a1, bool a2) => (a1 == 10) & !a2;
 
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static bool Eq_float_bool(float a1, bool a2) => (a1 == 10.1f) & !a2;
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static bool Eq_double_bool(double a1, bool a2) => (a1 == 10.1) & !a2;
+
 
     [MethodImpl(MethodImplOptions.NoInlining)]
     public static bool Ne_byte_bool(byte a1, bool a2) => (a1 != 5) & !a2;
@@ -53,6 +59,12 @@ public class ComparisonTestAndTestChains
     [MethodImpl(MethodImplOptions.NoInlining)]
     public static bool Ne_ulong_bool(ulong a1, bool a2) => (a1 != 5) & !a2;
 
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static bool Ne_float_bool(float a1, bool a2) => (a1 != 5.1f) & !a2;
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static bool Ne_double_bool(double a1, bool a2) => (a1 != 5.1) & !a2;
+
 
     [MethodImpl(MethodImplOptions.NoInlining)]
     public static bool Lt_byte_bool(byte a1, bool a2) => (a1 < 5) & !a2;
@@ -75,6 +87,12 @@ public class ComparisonTestAndTestChains
     [MethodImpl(MethodImplOptions.NoInlining)]
     public static bool Lt_ulong_bool(ulong a1, bool a2) => (a1 < 5) & !a2;
 
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static bool Lt_float_bool(float a1, bool a2) => (a1 < 5.1f) & !a2;
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static bool Lt_double_bool(double a1, bool a2) => (a1 < 5.1) & !a2;
+
 
     [MethodImpl(MethodImplOptions.NoInlining)]
     public static bool Le_byte_bool(byte a1, bool a2) => (a1 <= 5) & !a2;
@@ -97,6 +115,12 @@ public class ComparisonTestAndTestChains
     [MethodImpl(MethodImplOptions.NoInlining)]
     public static bool Le_ulong_bool(ulong a1, bool a2) => (a1 <= 5) & !a2;
 
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static bool Le_float_bool(float a1, bool a2) => (a1 <= 5.1f) & !a2;
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static bool Le_double_bool(double a1, bool a2) => (a1 <= 5.1) & !a2;
+
 
     [MethodImpl(MethodImplOptions.NoInlining)]
     public static bool Gt_byte_bool(byte a1, bool a2) => (a1 > 5) & !a2;
@@ -119,6 +143,12 @@ public class ComparisonTestAndTestChains
     [MethodImpl(MethodImplOptions.NoInlining)]
     public static bool Gt_ulong_bool(ulong a1, bool a2) => (a1 > 5) & !a2;
 
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static bool Gt_float_bool(float a1, bool a2) => (a1 > 5.1f) & !a2;
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static bool Gt_double_bool(double a1, bool a2) => (a1 > 5.1) & !a2;
+
 
     [MethodImpl(MethodImplOptions.NoInlining)]
     public static bool Ge_byte_bool(byte a1, bool a2) => (a1 >= 5) & !a2;
@@ -141,6 +171,12 @@ public class ComparisonTestAndTestChains
     [MethodImpl(MethodImplOptions.NoInlining)]
     public static bool Ge_ulong_bool(ulong a1, bool a2) => (a1 >= 5) & !a2;
 
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static bool Ge_float_bool(float a1, bool a2) => (a1 >= 5.1f) & !a2;
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static bool Ge_double_bool(double a1, bool a2) => (a1 >= 5.1) & !a2;
+
 
     [MethodImpl(MethodImplOptions.NoInlining)]
     public static int Main()
@@ -180,6 +216,16 @@ public static int Main()
             Console.WriteLine("ComparisonTestAndTestChains:Eq_ulong_bool(10, false) failed");
             return 101;
         }
+        if (!Eq_float_bool(10.1f, false))
+        {
+            Console.WriteLine("ComparisonTestAndTestChains:Eq_float_bool(10.1, false) failed");
+            return 101;
+        }
+        if (!Eq_double_bool(10.1, false))
+        {
+            Console.WriteLine("ComparisonTestAndTestChains:Eq_double_bool(10.1, false) failed");
+            return 101;
+        }
 
         if (!Ne_byte_bool(10, false))
         {
@@ -216,6 +262,16 @@ public static int Main()
             Console.WriteLine("ComparisonTestAndTestChains:Ne_ulong_bool(10, false) failed");
             return 101;
         }
+        if (!Ne_float_bool(10.1f, false))
+        {
+            Console.WriteLine("ComparisonTestAndTestChains:Ne_float_bool(10.1, false) failed");
+            return 101;
+        }
+        if (!Ne_double_bool(10.1, false))
+        {
+            Console.WriteLine("ComparisonTestAndTestChains:Ne_double_bool(10.1, false) failed");
+            return 101;
+        }
 
         if (!Lt_byte_bool(3, false))
         {
@@ -252,6 +308,16 @@ public static int Main()
             Console.WriteLine("ComparisonTestAndTestChains:Lt_ulong_bool(3, false) failed");
             return 101;
         }
+        if (!Lt_float_bool(3.1f, false))
+        {
+            Console.WriteLine("ComparisonTestAndTestChains:Lt_float_bool(3.1, false) failed");
+            return 101;
+        }
+        if (!Lt_double_bool(3.1, false))
+        {
+            Console.WriteLine("ComparisonTestAndTestChains:Lt_double_bool(3.1, false) failed");
+            return 101;
+        }
 
         if (!Le_byte_bool(3, false))
         {
@@ -288,6 +354,16 @@ public static int Main()
             Console.WriteLine("ComparisonTestAndTestChains:Le_ulong_bool(3, false) failed");
             return 101;
         }
+        if (!Le_float_bool(3.1f, false))
+        {
+            Console.WriteLine("ComparisonTestAndTestChains:Le_float_bool(3.1, false) failed");
+            return 101;
+        }
+        if (!Le_double_bool(3.1, false))
+        {
+            Console.WriteLine("ComparisonTestAndTestChains:Le_double_bool(3.1, false) failed");
+            return 101;
+        }
 
         if (!Gt_byte_bool(10, false))
         {
@@ -324,6 +400,16 @@ public static int Main()
             Console.WriteLine("ComparisonTestAndTestChains:Gt_ulong_bool(10, false) failed");
             return 101;
         }
+        if (!Gt_float_bool(10.1f, false))
+        {
+            Console.WriteLine("ComparisonTestAndTestChains:Gt_float_bool(10.1, false) failed");
+            return 101;
+        }
+        if (!Gt_double_bool(10.1, false))
+        {
+            Console.WriteLine("ComparisonTestAndTestChains:Gt_double_bool(10.1, false) failed");
+            return 101;
+        }
 
         if (!Ge_byte_bool(10, false))
         {
@@ -360,6 +446,16 @@ public static int Main()
             Console.WriteLine("ComparisonTestAndTestChains:Le_ulong_bool(10, false) failed");
             return 101;
         }
+        if (!Ge_float_bool(10.1f, false))
+        {
+            Console.WriteLine("ComparisonTestAndTestChains:Ge_float_bool(10.1, false) failed");
+            return 101;
+        }
+        if (!Ge_double_bool(10.1, false))
+        {
+            Console.WriteLine("ComparisonTestAndTestChains:Ge_double_bool(10.1, false) failed");
+            return 101;
+        }
 
         Console.WriteLine("PASSED");
         return 100;

From ac2568fce9bb401d6791a72ced519fb4895f95f1 Mon Sep 17 00:00:00 2001
From: Alan Hayward <alan.hayward@arm.com>
Date: Thu, 13 Oct 2022 15:41:30 +0100
Subject: [PATCH 32/38] Fix formatting

---
 src/coreclr/jit/lowerarmarch.cpp | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/coreclr/jit/lowerarmarch.cpp b/src/coreclr/jit/lowerarmarch.cpp
index 4352a26b67635..aa3c360c4b6df 100644
--- a/src/coreclr/jit/lowerarmarch.cpp
+++ b/src/coreclr/jit/lowerarmarch.cpp
@@ -2340,7 +2340,6 @@ bool Lowering::ContainCheckCompareChain(GenTree* child, GenTree* parent, GenTree
         }
     }
 
-
     return false;
 }
 

From bad8097a173d581b08e5880e1814eb0d3a09b543 Mon Sep 17 00:00:00 2001
From: Alan Hayward <alan.hayward@arm.com>
Date: Fri, 14 Oct 2022 09:49:17 +0100
Subject: [PATCH 33/38] Compare chains use CmpCompares, selects use Compares

---
 src/coreclr/jit/codegenarm64.cpp  | 20 +++++++++++++-----
 src/coreclr/jit/codegenlinear.cpp |  3 ++-
 src/coreclr/jit/gentree.h         |  2 +-
 src/coreclr/jit/lowerarmarch.cpp  | 35 ++++++++++++++++++++-----------
 src/coreclr/jit/lsrabuild.cpp     |  3 ++-
 5 files changed, 43 insertions(+), 20 deletions(-)

diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp
index cc6316ef08e27..c6e2f98964067 100644
--- a/src/coreclr/jit/codegenarm64.cpp
+++ b/src/coreclr/jit/codegenarm64.cpp
@@ -4679,7 +4679,7 @@ void CodeGen::genCodeForContainedCompareChain(GenTree* tree, bool* inChain, GenC
     }
     else
     {
-        assert(tree->OperIsCompare());
+        assert(tree->OperIsCmpCompare());
 
         // Generate the compare, putting the result in the flags register.
         if (!*inChain)
@@ -4724,10 +4724,20 @@ void CodeGen::genCodeForSelect(GenTreeConditional* tree)
     if (opcond->isContained())
     {
         // Generate the contained condition.
-        bool chain = false;
-        JITDUMP("Generating compare chain:\n");
-        genCodeForContainedCompareChain(opcond, &chain, &prevCond);
-        assert(chain);
+        if (opcond->OperIsCompare())
+        {
+            genCodeForCompare(opcond->AsOp());
+            prevCond = GenCondition::FromRelop(opcond);
+        }
+        else
+        {
+            // Condition is a compare chain. Try to contain it.
+            assert(opcond->OperIs(GT_AND));
+            bool chain = false;
+            JITDUMP("Generating compare chain:\n");
+            genCodeForContainedCompareChain(opcond, &chain, &prevCond);
+            assert(chain);
+        }
     }
     else
     {
diff --git a/src/coreclr/jit/codegenlinear.cpp b/src/coreclr/jit/codegenlinear.cpp
index b467a5da7e8bb..e4e95939ce34e 100644
--- a/src/coreclr/jit/codegenlinear.cpp
+++ b/src/coreclr/jit/codegenlinear.cpp
@@ -1631,7 +1631,8 @@ void CodeGen::genConsumeRegs(GenTree* tree)
         }
         else if (tree->OperIsCompare() || tree->OperIs(GT_AND))
         {
-            // Compares and ANDs may be contained in a conditional chain.
+            // Compares can be contained by a SELECT.
+            // ANDs and Cmp Compares may be contained in a chain.
             genConsumeRegs(tree->gtGetOp1());
             genConsumeRegs(tree->gtGetOp2());
         }
diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h
index b14204e45d957..6b139b40b07cf 100644
--- a/src/coreclr/jit/gentree.h
+++ b/src/coreclr/jit/gentree.h
@@ -913,7 +913,7 @@ struct GenTree
     // Node and its child in isolation form a contained compare chain.
     bool isContainedCompareChainSegment(GenTree* child) const
     {
-        return (OperIs(GT_AND) && child->isContained() && (child->OperIs(GT_AND) || child->OperIsCompare()));
+        return (OperIs(GT_AND) && child->isContained() && (child->OperIs(GT_AND) || child->OperIsCmpCompare()));
     }
 
     bool isContainedFltOrDblImmed() const
diff --git a/src/coreclr/jit/lowerarmarch.cpp b/src/coreclr/jit/lowerarmarch.cpp
index aa3c360c4b6df..1401de10b4705 100644
--- a/src/coreclr/jit/lowerarmarch.cpp
+++ b/src/coreclr/jit/lowerarmarch.cpp
@@ -2266,7 +2266,7 @@ bool Lowering::IsValidCompareChain(GenTree* child, GenTree* parent)
         return IsValidCompareChain(child->AsOp()->gtGetOp2(), child) &&
                IsValidCompareChain(child->AsOp()->gtGetOp1(), child);
     }
-    else if (child->OperIsCompare() && varTypeIsIntegral(child->gtGetOp1()) && varTypeIsIntegral(child->gtGetOp2()))
+    else if (child->OperIsCmpCompare() && varTypeIsIntegral(child->gtGetOp1()) && varTypeIsIntegral(child->gtGetOp2()))
     {
         // Can the child compare be contained.
         return IsSafeToContainMem(parent, child);
@@ -2373,7 +2373,7 @@ void Lowering::ContainCheckCompareChainForAnd(GenTree* tree)
                 if (startOfChain != nullptr)
                 {
                     // The earliest node in the chain will be generated as a standard compare.
-                    assert(startOfChain->OperIsCompare());
+                    assert(startOfChain->OperIsCmpCompare());
                     startOfChain->AsOp()->gtGetOp1()->ClearContained();
                     startOfChain->AsOp()->gtGetOp2()->ClearContained();
                     ContainCheckCompare(startOfChain->AsOp());
@@ -2421,17 +2421,28 @@ void Lowering::ContainCheckSelect(GenTreeConditional* node)
         return;
     }
 
-    // Check if the compare does not need to be generated into a register.
-    GenTree* startOfChain = nullptr;
-    ContainCheckCompareChain(node->gtCond, node, &startOfChain);
-
-    if (startOfChain != nullptr)
+    if (node->gtCond->OperIsCompare())
+    {
+        // All compare node types (including TEST_) are containable.
+        if (IsSafeToContainMem(node, node->gtCond))
+        {
+            node->gtCond->AsOp()->SetContained();
+        }
+    }
+    else
     {
-        // The earliest node in the chain will be generated as a standard compare.
-        assert(startOfChain->OperIsCompare());
-        startOfChain->AsOp()->gtGetOp1()->ClearContained();
-        startOfChain->AsOp()->gtGetOp2()->ClearContained();
-        ContainCheckCompare(startOfChain->AsOp());
+        // Check for a compare chain and try to contain it.
+        GenTree* startOfChain = nullptr;
+        ContainCheckCompareChain(node->gtCond, node, &startOfChain);
+
+        if (startOfChain != nullptr)
+        {
+            // The earliest node in the chain will be generated as a standard compare.
+            assert(startOfChain->OperIsCmpCompare());
+            startOfChain->AsOp()->gtGetOp1()->ClearContained();
+            startOfChain->AsOp()->gtGetOp2()->ClearContained();
+            ContainCheckCompare(startOfChain->AsOp());
+        }
     }
 }
 
diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp
index 39e63c65beb3f..5a8c4b97b6460 100644
--- a/src/coreclr/jit/lsrabuild.cpp
+++ b/src/coreclr/jit/lsrabuild.cpp
@@ -3154,7 +3154,8 @@ int LinearScan::BuildOperandUses(GenTree* node, regMaskTP candidates)
     if (node->OperIs(GT_MUL) || node->OperIsCompare() || node->OperIs(GT_AND))
     {
         // MUL can be contained for madd or msub on arm64.
-        // Compare and AND may be contained due to If Conversion.
+        // Compares can be contained by a SELECT.
+        // ANDs and Cmp Compares may be contained in a chain.
         return BuildBinaryUses(node->AsOp(), candidates);
     }
     if (node->OperIs(GT_NEG, GT_CAST, GT_LSH, GT_RSH, GT_RSZ))

From 935716c1af42d6f971f3592a5d00f877668ef12e Mon Sep 17 00:00:00 2001
From: Alan Hayward <alan.hayward@arm.com>
Date: Thu, 20 Oct 2022 11:04:33 +0100
Subject: [PATCH 34/38] Fix flow checking for empty blocks

---
 src/coreclr/jit/optimizer.cpp | 47 +++++++++++++++++++----------------
 1 file changed, 26 insertions(+), 21 deletions(-)

diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp
index 9e3fd2ad2de9a..e6fdd38267576 100644
--- a/src/coreclr/jit/optimizer.cpp
+++ b/src/coreclr/jit/optimizer.cpp
@@ -4620,9 +4620,9 @@ PhaseStatus Compiler::optUnrollLoops()
 // if (x < 7) { a = 5; }
 //
 // This is represented in IR by two basic blocks. The first block (block) ends with
-// a JTRUE statement which conditionally jumps to the second block (middleBlock).
+// a JTRUE statement which conditionally jumps to the second block (asgBlock).
 // The second block just contains a single assign statement. Both blocks then jump
-// to the same destination (middleNext).  Note that the first block may contain
+// to the same destination (finalBlock).  Note that the first block may contain
 // additional statements prior to the JTRUE statement.
 //
 // For example:
@@ -4695,10 +4695,12 @@ bool Compiler::optIfConvert(BasicBlock* block)
         return false;
     }
 
-    BasicBlock* middleBlock = nullptr;
-    BasicBlock* middleNext  = block->bbNext;
-    GenTree*    asgNode     = nullptr;
-    Statement*  asgStmt     = nullptr;
+    // Block where the flows merge.
+    BasicBlock* finalBlock = block->bbNext;
+    // The node, statement and block of the assignment.
+    GenTree*    asgNode  = nullptr;
+    Statement*  asgStmt  = nullptr;
+    BasicBlock* asgBlock = nullptr;
 
     // Check the block is followed by a block or chain of blocks that only contain NOPs and
     // a single ASG statement. The destination of the final block must point to the same as the
@@ -4706,17 +4708,17 @@ bool Compiler::optIfConvert(BasicBlock* block)
     bool foundMiddle = false;
     while (!foundMiddle)
     {
-        middleBlock = middleNext;
+        BasicBlock* middleBlock = finalBlock;
         noway_assert(middleBlock != nullptr);
 
         // middleBlock should have a single successor.
-        middleNext = middleBlock->GetUniqueSucc();
-        if (middleNext == nullptr)
+        finalBlock = middleBlock->GetUniqueSucc();
+        if (finalBlock == nullptr)
         {
             return false;
         }
 
-        if (middleNext == block->bbJumpDest)
+        if (finalBlock == block->bbJumpDest)
         {
             // This is our final middle block.
             foundMiddle = true;
@@ -4764,8 +4766,9 @@ bool Compiler::optIfConvert(BasicBlock* block)
                         return false;
                     }
 
-                    asgNode = tree;
-                    asgStmt = stmt;
+                    asgNode  = tree;
+                    asgStmt  = stmt;
+                    asgBlock = middleBlock;
                     break;
                 }
 
@@ -4792,8 +4795,9 @@ bool Compiler::optIfConvert(BasicBlock* block)
 #ifdef DEBUG
     if (verbose)
     {
-        JITDUMP("\nConditionally executing " FMT_BB " inside " FMT_BB "\n", middleBlock->bbNum, block->bbNum);
-        for (BasicBlock* dumpBlock = block; dumpBlock != middleBlock->bbNext; dumpBlock = dumpBlock->bbNext)
+        JITDUMP("\nConditionally executing " FMT_BB " inside " FMT_BB "\n", asgBlock->bbNum, block->bbNum);
+        fgDumpBlock(block);
+        for (BasicBlock* dumpBlock = block->bbNext; dumpBlock != finalBlock; dumpBlock = dumpBlock->GetUniqueSucc())
         {
             fgDumpBlock(dumpBlock);
         }
@@ -4864,8 +4868,8 @@ bool Compiler::optIfConvert(BasicBlock* block)
     gtSetEvalOrder(last);
     fgSetStmtSeq(block->lastStmt());
 
-    // Before moving anything, fix up any SSAs in the middle block
-    for (Statement* const stmt : middleBlock->Statements())
+    // Before moving anything, fix up any SSAs in the asgBlock
+    for (Statement* const stmt : asgBlock->Statements())
     {
         for (GenTree* const node : stmt->TreeList())
         {
@@ -4877,7 +4881,7 @@ bool Compiler::optIfConvert(BasicBlock* block)
                 if (ssaNum != SsaConfig::RESERVED_SSA_NUM)
                 {
                     LclSsaVarDsc* ssaDef = lvaGetDesc(lclNum)->GetPerSsaData(ssaNum);
-                    if (ssaDef->GetBlock() == middleBlock)
+                    if (ssaDef->GetBlock() == asgBlock)
                     {
                         JITDUMP("SSA def %d for V%02u moved from " FMT_BB " to " FMT_BB ".\n", ssaNum, lclNum,
                                 ssaDef->GetBlock()->bbNum, block->bbNum);
@@ -4890,13 +4894,13 @@ bool Compiler::optIfConvert(BasicBlock* block)
 
     // Move the Asg to the end of the original block
     Statement* stmtList1 = block->firstStmt();
-    Statement* stmtList2 = middleBlock->firstStmt();
+    Statement* stmtList2 = asgBlock->firstStmt();
     Statement* stmtLast1 = block->lastStmt();
-    Statement* stmtLast2 = middleBlock->lastStmt();
+    Statement* stmtLast2 = asgBlock->lastStmt();
     stmtLast1->SetNextStmt(stmtList2);
     stmtList2->SetPrevStmt(stmtLast1);
     stmtList1->SetPrevStmt(stmtLast2);
-    middleBlock->bbStmtList = nullptr;
+    asgBlock->bbStmtList = nullptr;
 
     // Update the flow from the original block.
     fgRemoveAllRefPreds(block->bbNext, block);
@@ -4906,7 +4910,8 @@ bool Compiler::optIfConvert(BasicBlock* block)
     if (verbose)
     {
         JITDUMP("\nAfter if conversion\n");
-        for (BasicBlock* dumpBlock = block; dumpBlock != middleBlock->bbNext; dumpBlock = dumpBlock->bbNext)
+        fgDumpBlock(block);
+        for (BasicBlock* dumpBlock = block->bbNext; dumpBlock != finalBlock; dumpBlock = dumpBlock->GetUniqueSucc())
         {
             fgDumpBlock(dumpBlock);
         }

From f2f3a0286658c5e2a15e7726aee7b0ffe01a4cd4 Mon Sep 17 00:00:00 2001
From: Jakob Botsch Nielsen <jakob.botsch.nielsen@gmail.com>
Date: Wed, 26 Oct 2022 21:39:34 +0200
Subject: [PATCH 35/38] Fix to contexts setting JitStdOutFile

---
 src/coreclr/scripts/superpmi.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/coreclr/scripts/superpmi.py b/src/coreclr/scripts/superpmi.py
index c25849d9beaf2..2f3e1d68718c1 100644
--- a/src/coreclr/scripts/superpmi.py
+++ b/src/coreclr/scripts/superpmi.py
@@ -1703,7 +1703,7 @@ async def create_replay_artifacts(print_prefix, context_index, self, mch_file, e
                         with ChangeDir(self.coreclr_args.core_root):
 
                             async def create_one_artifact(jit_path: str, location: str, flags) -> str:
-                                command = [self.superpmi_path] + flags + [jit_path, mch_file]
+                                command = [self.superpmi_path] + flags + ["-jitoption", "force", "JitStdOutFile=" + item_path] + [jit_path, mch_file]
                                 item_path = os.path.join(location, "{}{}".format(context_index, extension))
                                 modified_env = env.copy()
                                 modified_env['DOTNET_JitStdOutFile'] = item_path

From 2e183bd0010ffed30e54621bfc6433673cbd4abc Mon Sep 17 00:00:00 2001
From: Jakob Botsch Nielsen <jakob.botsch.nielsen@gmail.com>
Date: Wed, 26 Oct 2022 21:59:42 +0200
Subject: [PATCH 36/38] Temporary fix for SPMI problem

---
 src/coreclr/scripts/superpmi.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/coreclr/scripts/superpmi.py b/src/coreclr/scripts/superpmi.py
index 2f3e1d68718c1..4dae2aacceaa5 100644
--- a/src/coreclr/scripts/superpmi.py
+++ b/src/coreclr/scripts/superpmi.py
@@ -1703,8 +1703,8 @@ async def create_replay_artifacts(print_prefix, context_index, self, mch_file, e
                         with ChangeDir(self.coreclr_args.core_root):
 
                             async def create_one_artifact(jit_path: str, location: str, flags) -> str:
-                                command = [self.superpmi_path] + flags + ["-jitoption", "force", "JitStdOutFile=" + item_path] + [jit_path, mch_file]
                                 item_path = os.path.join(location, "{}{}".format(context_index, extension))
+                                command = [self.superpmi_path] + flags + ["-jitoption", "force", "JitStdOutFile=" + item_path] + [jit_path, mch_file]
                                 modified_env = env.copy()
                                 modified_env['DOTNET_JitStdOutFile'] = item_path
                                 logging.debug("%sGenerating %s", print_prefix, item_path)

From b2772ff94a86ff61d8223bdcd5b52a832b07953a Mon Sep 17 00:00:00 2001
From: Alan Hayward <alan.hayward@arm.com>
Date: Fri, 28 Oct 2022 16:44:23 +0100
Subject: [PATCH 37/38] Fix attr and reg producing in select generation

---
 src/coreclr/jit/codegenarm64.cpp | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp
index c6e2f98964067..4032cc944957a 100644
--- a/src/coreclr/jit/codegenarm64.cpp
+++ b/src/coreclr/jit/codegenarm64.cpp
@@ -4714,7 +4714,7 @@ void CodeGen::genCodeForSelect(GenTreeConditional* tree)
     GenTree*  op2     = tree->gtOp2;
     var_types op1Type = genActualType(op1->TypeGet());
     var_types op2Type = genActualType(op2->TypeGet());
-    emitAttr  cmpSize = EA_ATTR(genTypeSize(op1Type));
+    emitAttr  attr    = emitActualTypeSize(tree->TypeGet());
 
     assert(!op1->isUsedFromMemory());
     assert(genTypeSize(op1Type) == genTypeSize(op2Type));
@@ -4751,8 +4751,9 @@ void CodeGen::genCodeForSelect(GenTreeConditional* tree)
     regNumber               srcReg2   = genConsumeReg(op2);
     const GenConditionDesc& prevDesc  = GenConditionDesc::Get(prevCond);
 
-    emit->emitIns_R_R_R_COND(INS_csel, cmpSize, targetReg, srcReg1, srcReg2, JumpKindToInsCond(prevDesc.jumpKind1));
+    emit->emitIns_R_R_R_COND(INS_csel, attr, targetReg, srcReg1, srcReg2, JumpKindToInsCond(prevDesc.jumpKind1));
     regSet.verifyRegUsed(targetReg);
+    genProduceReg(tree);
 }
 
 //------------------------------------------------------------------------

From 4281fc0e78650fbd6528eb1007fc93e76ab2d055 Mon Sep 17 00:00:00 2001
From: Jakob Botsch Nielsen <jakob.botsch.nielsen@gmail.com>
Date: Fri, 28 Oct 2022 17:52:11 +0200
Subject: [PATCH 38/38] Revert SPMI changes

---
 src/coreclr/scripts/superpmi.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/coreclr/scripts/superpmi.py b/src/coreclr/scripts/superpmi.py
index 4dae2aacceaa5..c25849d9beaf2 100644
--- a/src/coreclr/scripts/superpmi.py
+++ b/src/coreclr/scripts/superpmi.py
@@ -1703,8 +1703,8 @@ async def create_replay_artifacts(print_prefix, context_index, self, mch_file, e
                         with ChangeDir(self.coreclr_args.core_root):
 
                             async def create_one_artifact(jit_path: str, location: str, flags) -> str:
+                                command = [self.superpmi_path] + flags + [jit_path, mch_file]
                                 item_path = os.path.join(location, "{}{}".format(context_index, extension))
-                                command = [self.superpmi_path] + flags + ["-jitoption", "force", "JitStdOutFile=" + item_path] + [jit_path, mch_file]
                                 modified_env = env.copy()
                                 modified_env['DOTNET_JitStdOutFile'] = item_path
                                 logging.debug("%sGenerating %s", print_prefix, item_path)