JIT: remove simple lowering

src/coreclr/jit/compiler.cpp

@@ -5088,13 +5088,6 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
 
     fgNodeThreading = NodeThreading::LIR;
 
-    // Here we do "simple lowering".  When the RyuJIT backend works for all
-    // platforms, this will be part of the more general lowering phase.  For now, though, we do a separate
-    // pass of "final lowering."  We must do this before (final) liveness analysis, because this creates
-    // range check throw blocks, in which the liveness must be correct.
-    //
-    DoPhase(this, PHASE_SIMPLE_LOWERING, &Compiler::fgSimpleLowering);
-
     // Enable this to gather statistical data such as
     // call and register argument info, flowgraph and loop info, etc.
     compJitStats();

src/coreclr/jit/compiler.h

@@ -4890,10 +4890,6 @@ class Compiler
     void fgExpandQmarkStmt(BasicBlock* block, Statement* stmt);
     void fgExpandQmarkNodes();
 
-    // Do "simple lowering."  This functionality is (conceptually) part of "general"
-    // lowering that is distributed between fgMorph and the lowering phase of LSRA.
-    PhaseStatus fgSimpleLowering();
-
     bool fgSimpleLowerCastOfSmpOp(LIR::Range& range, GenTreeCast* cast);
 
 #if FEATURE_LOOP_ALIGN

src/coreclr/jit/compphases.h

@@ -102,7 +102,6 @@ CompPhaseNameMacro(PHASE_INSERT_GC_POLLS,            "Insert GC Polls",
 CompPhaseNameMacro(PHASE_CREATE_THROW_HELPERS,       "Create throw helper blocks",     false, -1, true)
 CompPhaseNameMacro(PHASE_DETERMINE_FIRST_COLD_BLOCK, "Determine first cold block",     false, -1, true)
 CompPhaseNameMacro(PHASE_RATIONALIZE,                "Rationalize IR",                 false, -1, false)
-CompPhaseNameMacro(PHASE_SIMPLE_LOWERING,            "Do 'simple' lowering",           false, -1, false)
 
 CompPhaseNameMacro(PHASE_LCLVARLIVENESS,             "Local var liveness",             true,  -1, false)
 CompPhaseNameMacro(PHASE_LCLVARLIVENESS_INIT,        "Local var liveness init",        false, PHASE_LCLVARLIVENESS, false)

src/coreclr/jit/flowgraph.cpp

@@ -2781,120 +2781,6 @@ PhaseStatus Compiler::fgFindOperOrder()
     return PhaseStatus::MODIFIED_EVERYTHING;
 }
 
-//------------------------------------------------------------------------
-// fgSimpleLowering: do full walk of all IR, lowering selected operations
-// and computing lvaOutgoingArgSpaceSize.
-//
-// Returns:
-//    Suitable phase status
-//
-// Notes:
-//    Lowers GT_ARR_LENGTH, GT_MDARR_LENGTH, GT_MDARR_LOWER_BOUND
-//
-PhaseStatus Compiler::fgSimpleLowering()
-{
-    bool madeChanges = false;
-
-#if FEATURE_FIXED_OUT_ARGS
-    unsigned outgoingArgSpaceSize = 0;
-#endif // FEATURE_FIXED_OUT_ARGS
-
-    for (BasicBlock* const block : Blocks())
-    {
-        // Walk the statement trees in this basic block.
-        compCurBB = block; // Used in fgRngChkTarget.
-
-        LIR::Range& range = LIR::AsRange(block);
-        for (GenTree* tree : range)
-        {
-            switch (tree->OperGet())
-            {
-                case GT_ARR_LENGTH:
-                case GT_MDARR_LENGTH:
-                case GT_MDARR_LOWER_BOUND:
-                {
-                    GenTree* arr       = tree->AsArrCommon()->ArrRef();
-                    int      lenOffset = 0;
-
-                    switch (tree->OperGet())
-                    {
-                        case GT_ARR_LENGTH:
-                        {
-                            lenOffset = tree->AsArrLen()->ArrLenOffset();
-                            noway_assert(lenOffset == OFFSETOF__CORINFO_Array__length ||
-                                         lenOffset == OFFSETOF__CORINFO_String__stringLen);
-                            break;
-                        }
-
-                        case GT_MDARR_LENGTH:
-                            lenOffset = (int)eeGetMDArrayLengthOffset(tree->AsMDArr()->Rank(), tree->AsMDArr()->Dim());
-                            break;
-
-                        case GT_MDARR_LOWER_BOUND:
-                            lenOffset =
-                                (int)eeGetMDArrayLowerBoundOffset(tree->AsMDArr()->Rank(), tree->AsMDArr()->Dim());
-                            break;
-
-                        default:
-                            unreached();
-                    }
-
-                    // Create the expression `*(array_addr + lenOffset)`
-
-                    GenTree* addr;
-
-                    noway_assert(arr->gtNext == tree);
-
-                    JITDUMP("Lower %s:\n", GenTree::OpName(tree->OperGet()));
-                    DISPRANGE(LIR::ReadOnlyRange(arr, tree));
-
-                    if ((arr->gtOper == GT_CNS_INT) && (arr->AsIntCon()->gtIconVal == 0))
-                    {
-                        // If the array is NULL, then we should get a NULL reference
-                        // exception when computing its length.  We need to maintain
-                        // an invariant where there is no sum of two constants node, so
-                        // let's simply return an indirection of NULL.
-
-                        addr = arr;
-                    }
-                    else
-                    {
-                        GenTree* con = gtNewIconNode(lenOffset, TYP_I_IMPL);
-                        addr         = gtNewOperNode(GT_ADD, TYP_BYREF, arr, con);
-                        range.InsertAfter(arr, con, addr);
-                    }
-
-                    // Change to a GT_IND.
-                    tree->ChangeOper(GT_IND);
-                    tree->AsIndir()->Addr() = addr;
-
-                    JITDUMP("After Lower %s:\n", GenTree::OpName(tree->OperGet()));
-                    DISPRANGE(LIR::ReadOnlyRange(arr, tree));
-                    madeChanges = true;
-                    break;
-                }
-
-                case GT_CAST:
-                {
-                    if (tree->AsCast()->CastOp()->OperIsSimple() && fgSimpleLowerCastOfSmpOp(range, tree->AsCast()))
-                    {
-                        madeChanges = true;
-                    }
-                    break;
-                }
-
-                default:
-                {
-                    // No other operators need processing.
-                    break;
-                }
-            } // switch on oper
-        }     // foreach tree
-    }         // foreach BB
-
-    return madeChanges ? PhaseStatus::MODIFIED_EVERYTHING : PhaseStatus::MODIFIED_NOTHING;
-}
-
 //------------------------------------------------------------------------
 // fgSimpleLowerCastOfSmpOp: Optimization to remove CAST nodes from operands of some simple ops that are safe to do so
 // since the upper bits do not affect the lower bits, and result of the simple op is zero/sign-extended via a CAST.
@@ -2909,7 +2795,7 @@ PhaseStatus Compiler::fgSimpleLowering()
 //      problems with NOLs (normalized-on-load locals) and how they are handled in VN.
 //      Simple put, you cannot remove a CAST from CAST(LCL_VAR{nol}) in HIR.
 //
-//      Because the optimization happens after rationalization, turning into LIR, it is safe to remove the CAST.
+//      Because the optimization happens during rationalization, turning into LIR, it is safe to remove the CAST.
 //
 bool Compiler::fgSimpleLowerCastOfSmpOp(LIR::Range& range, GenTreeCast* cast)
 {

src/coreclr/jit/lower.cpp

@@ -542,13 +542,6 @@ GenTree* Lowering::LowerNode(GenTree* node)
             break;
 #endif // defined(TARGET_XARCH) || defined(TARGET_ARM64) || defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64)
 
-        case GT_ARR_ELEM: // Lowered by fgMorphArrayOps()
-        case GT_MDARR_LENGTH:
-        case GT_MDARR_LOWER_BOUND:
-            // Lowered by fgSimpleLowering()
-            unreached();
-            break;
-
         case GT_ROL:
         case GT_ROR:
             LowerRotate(node);
@@ -688,13 +681,87 @@ GenTree* Lowering::LowerNode(GenTree* node)
             break;
 #endif // FEATURE_HW_INTRINSICS && TARGET_XARCH
 
+        case GT_ARR_LENGTH:
+        case GT_MDARR_LENGTH:
+        case GT_MDARR_LOWER_BOUND:
+            return LowerArrLength(node->AsArrCommon());
+            break;
+
         default:
             break;
     }
 
     return node->gtNext;
 }
 
+//------------------------------------------------------------------------
+// LowerArrLength: lower an array length
+//
+// Arguments:
+//    node - the array length node we are lowering.
+//
+// Returns:
+//    next node that needs to be lowered.
+//
+// Notes:
+//    If base array is nullptr, this effectively
+//    turns into a nullcheck.
+//
+GenTree* Lowering::LowerArrLength(GenTreeArrCommon* node)
+{
+    GenTree* const arr       = node->ArrRef();
+    int            lenOffset = 0;
+
+    switch (node->OperGet())
+    {
+        case GT_ARR_LENGTH:
+        {
+            lenOffset = node->AsArrLen()->ArrLenOffset();
+            noway_assert(lenOffset == OFFSETOF__CORINFO_Array__length ||
+                         lenOffset == OFFSETOF__CORINFO_String__stringLen);
+            break;
+        }
+
+        case GT_MDARR_LENGTH:
+            lenOffset = (int)comp->eeGetMDArrayLengthOffset(node->AsMDArr()->Rank(), node->AsMDArr()->Dim());
+            break;
+
+        case GT_MDARR_LOWER_BOUND:
+            lenOffset = (int)comp->eeGetMDArrayLowerBoundOffset(node->AsMDArr()->Rank(), node->AsMDArr()->Dim());
+            break;
+
+        default:
+            unreached();
+    }
+
+    // Create the expression `*(array_addr + lenOffset)`
+
+    GenTree* addr;
+    noway_assert(arr->gtNext == node);
+
+    if ((arr->gtOper == GT_CNS_INT) && (arr->AsIntCon()->gtIconVal == 0))
+    {
+        // If the array is NULL, then we should get a NULL reference
+        // exception when computing its length.  We need to maintain
+        // an invariant where there is no sum of two constants node, so
+        // let's simply return an indirection of NULL.
+
+        addr = arr;
+    }
+    else
+    {
+        GenTree* con = comp->gtNewIconNode(lenOffset, TYP_I_IMPL);
+        addr         = comp->gtNewOperNode(GT_ADD, TYP_BYREF, arr, con);
+        BlockRange().InsertAfter(arr, con, addr);
+    }
+
+    // Change to a GT_IND.
+    node->ChangeOper(GT_IND);
+    node->AsIndir()->Addr() = addr;
+
+    return arr->gtNext;
+}
+
 /**  -- Switch Lowering --
  * The main idea of switch lowering is to keep transparency of the register requirements of this node
  * downstream in LSRA.  Given that the switch instruction is inherently a control statement which in the JIT

src/coreclr/jit/lower.h

@@ -333,6 +333,8 @@ class Lowering final : public Phase
     void LowerLclHeap(GenTree* node);
     void ContainBlockStoreAddress(GenTreeBlk* blkNode, unsigned size, GenTree* addr, GenTree* addrParent);
     void LowerPutArgStkOrSplit(GenTreePutArgStk* putArgNode);
+    GenTree* LowerArrLength(GenTreeArrCommon* node);
+
 #ifdef TARGET_XARCH
     void LowerPutArgStk(GenTreePutArgStk* putArgStk);
     GenTree* TryLowerMulWithConstant(GenTreeOp* node);
@@ -385,7 +387,6 @@ class Lowering final : public Phase
     void LowerModPow2(GenTree* node);
     GenTree* LowerAddForPossibleContainment(GenTreeOp* node);
 #endif // !TARGET_XARCH && !TARGET_ARM64
-
     GenTree* InsertNewSimdCreateScalarUnsafeNode(var_types   type,
                                                  GenTree*    op1,
                                                  CorInfoType simdBaseJitType,

src/coreclr/jit/rationalize.cpp

@@ -321,6 +321,13 @@ Compiler::fgWalkResult Rationalizer::RewriteNode(GenTree** useEdge, Compiler::Ge
             assert(comp->IsTargetIntrinsic(node->AsIntrinsic()->gtIntrinsicName));
             break;
 
+        case GT_CAST:
+            if (node->AsCast()->CastOp()->OperIsSimple())
+            {
+                comp->fgSimpleLowerCastOfSmpOp(BlockRange(), node->AsCast());
+            }
+            break;
+
         default:
             // Check that we don't have nodes not allowed in HIR here.
             assert((node->DebugOperKind() & DBK_NOTHIR) == 0);