JIT: Allow liveness to remove some nodes with contained side effects

src/coreclr/jit/compiler.h

@@ -5598,6 +5598,8 @@ class Compiler
 
     bool fgTryRemoveNonLocal(GenTree* node, LIR::Range* blockRange);
 
+    bool fgCanUncontainOrRemoveOperands(GenTree* node);
+
     bool fgTryRemoveDeadStoreLIR(GenTree* store, GenTreeLclVarCommon* lclNode, BasicBlock* block);
 
     bool fgRemoveDeadStore(GenTree**        pTree,

src/coreclr/jit/compiler.hpp

@@ -4503,8 +4503,15 @@ inline bool Compiler::PreciseRefCountsRequired()
     return opts.OptimizationEnabled();
 }
 
+#define RETURN_IF_ABORT(expr)                                                                                          \
+    do                                                                                                                 \
+    {                                                                                                                  \
+        if (expr == VisitResult::Abort)                                                                                \
+            return VisitResult::Abort;                                                                                 \
+    } while (0)
+
 template <typename TVisitor>
-void GenTree::VisitOperands(TVisitor visitor)
+GenTree::VisitResult GenTree::VisitOperands(TVisitor visitor)
 {
     switch (OperGet())
     {
@@ -4548,15 +4555,15 @@ void GenTree::VisitOperands(TVisitor visitor)
         case GT_NOP:
         case GT_SWIFT_ERROR:
         case GT_GCPOLL:
-            return;
+            return VisitResult::Continue;
 
         // Unary operators with an optional operand
         case GT_FIELD_ADDR:
         case GT_RETURN:
         case GT_RETFILT:
             if (this->AsUnOp()->gtOp1 == nullptr)
             {
-                return;
+                return VisitResult::Continue;
             }
             FALLTHROUGH;
 
@@ -4592,73 +4599,50 @@ void GenTree::VisitOperands(TVisitor visitor)
         case GT_KEEPALIVE:
         case GT_INC_SATURATE:
         case GT_RETURN_SUSPEND:
-            visitor(this->AsUnOp()->gtOp1);
-            return;
+            return visitor(this->AsUnOp()->gtOp1);
 
 // Variadic nodes
 #if defined(FEATURE_HW_INTRINSICS)
         case GT_HWINTRINSIC:
             for (GenTree* operand : this->AsMultiOp()->Operands())
             {
-                if (visitor(operand) == VisitResult::Abort)
-                {
-                    break;
-                }
+                RETURN_IF_ABORT(visitor(operand));
             }
-            return;
+            return VisitResult::Continue;
 #endif // defined(FEATURE_HW_INTRINSICS)
 
         // Special nodes
         case GT_PHI:
             for (GenTreePhi::Use& use : AsPhi()->Uses())
             {
-                if (visitor(use.GetNode()) == VisitResult::Abort)
-                {
-                    break;
-                }
+                RETURN_IF_ABORT(visitor(use.GetNode()));
             }
-            return;
+            return VisitResult::Continue;
 
         case GT_FIELD_LIST:
             for (GenTreeFieldList::Use& field : AsFieldList()->Uses())
             {
-                if (visitor(field.GetNode()) == VisitResult::Abort)
-                {
-                    break;
-                }
+                RETURN_IF_ABORT(visitor(field.GetNode()));
             }
-            return;
+            return VisitResult::Continue;
 
         case GT_CMPXCHG:
         {
             GenTreeCmpXchg* const cmpXchg = this->AsCmpXchg();
-            if (visitor(cmpXchg->Addr()) == VisitResult::Abort)
-            {
-                return;
-            }
-            if (visitor(cmpXchg->Data()) == VisitResult::Abort)
-            {
-                return;
-            }
-            visitor(cmpXchg->Comparand());
-            return;
+            RETURN_IF_ABORT(visitor(cmpXchg->Addr()));
+            RETURN_IF_ABORT(visitor(cmpXchg->Data()));
+            return visitor(cmpXchg->Comparand());
         }
 
         case GT_ARR_ELEM:
         {
             GenTreeArrElem* const arrElem = this->AsArrElem();
-            if (visitor(arrElem->gtArrObj) == VisitResult::Abort)
-            {
-                return;
-            }
+            RETURN_IF_ABORT(visitor(arrElem->gtArrObj));
             for (unsigned i = 0; i < arrElem->gtArrRank; i++)
             {
-                if (visitor(arrElem->gtArrInds[i]) == VisitResult::Abort)
-                {
-                    return;
-                }
+                RETURN_IF_ABORT(visitor(arrElem->gtArrInds[i]));
             }
-            return;
+            return VisitResult::Continue;
         }
 
         case GT_CALL:
@@ -4667,79 +4651,55 @@ void GenTree::VisitOperands(TVisitor visitor)
 
             for (CallArg& arg : call->gtArgs.EarlyArgs())
             {
-                if (visitor(arg.GetEarlyNode()) == VisitResult::Abort)
-                {
-                    return;
-                }
+                RETURN_IF_ABORT(visitor(arg.GetEarlyNode()));
             }
 
             for (CallArg& arg : call->gtArgs.LateArgs())
             {
-                if (visitor(arg.GetLateNode()) == VisitResult::Abort)
-                {
-                    return;
-                }
+                RETURN_IF_ABORT(visitor(arg.GetLateNode()));
             }
 
             if (call->gtCallType == CT_INDIRECT)
             {
-                if (!call->IsVirtualStub() && (call->gtCallCookie != nullptr) &&
-                    (visitor(call->gtCallCookie) == VisitResult::Abort))
+                if (!call->IsVirtualStub() && (call->gtCallCookie != nullptr))
                 {
-                    return;
+                    RETURN_IF_ABORT(visitor(call->gtCallCookie));
                 }
-                if ((call->gtCallAddr != nullptr) && (visitor(call->gtCallAddr) == VisitResult::Abort))
+                if (call->gtCallAddr != nullptr)
                 {
-                    return;
+                    RETURN_IF_ABORT(visitor(call->gtCallAddr));
                 }
             }
-            if ((call->gtControlExpr != nullptr))
+            if (call->gtControlExpr != nullptr)
             {
-                visitor(call->gtControlExpr);
+                return visitor(call->gtControlExpr);
             }
-            return;
+            return VisitResult::Continue;
         }
 
         case GT_SELECT:
         {
             GenTreeConditional* const cond = this->AsConditional();
-            if (visitor(cond->gtCond) == VisitResult::Abort)
-            {
-                return;
-            }
-            if (visitor(cond->gtOp1) == VisitResult::Abort)
-            {
-                return;
-            }
-            visitor(cond->gtOp2);
-            return;
+            RETURN_IF_ABORT(visitor(cond->gtCond));
+            RETURN_IF_ABORT(visitor(cond->gtOp1));
+            return visitor(cond->gtOp2);
         }
 
         // Binary nodes
         default:
             assert(this->OperIsBinary());
-            VisitBinOpOperands<TVisitor>(visitor);
-            return;
-    }
-}
-
-template <typename TVisitor>
-void GenTree::VisitBinOpOperands(TVisitor visitor)
-{
-    assert(this->OperIsBinary());
-
-    GenTreeOp* const op = this->AsOp();
-
-    GenTree* const op1 = op->gtOp1;
-    if ((op1 != nullptr) && (visitor(op1) == VisitResult::Abort))
-    {
-        return;
-    }
+            GenTree* op1 = gtGetOp1();
+            if (op1 != nullptr)
+            {
+                RETURN_IF_ABORT(visitor(op1));
+            }
 
-    GenTree* const op2 = op->gtOp2;
-    if (op2 != nullptr)
-    {
-        visitor(op2);
+            GenTree* op2 = gtGetOp2();
+            if (op2 != nullptr)
+            {
+                return visitor(op2);
+            }
+            return VisitResult::Continue;
     }
 }
 

src/coreclr/jit/gentree.cpp

@@ -7213,15 +7213,16 @@ bool GenTree::NodeOrContainedOperandsMayThrow(Compiler* comp)
         return true;
     }
 
-    // Check all contained children
-    for (GenTree* operand : Operands())
-    {
+    auto visit = [=](GenTree* operand) {
         if (operand->isContained() && operand->NodeOrContainedOperandsMayThrow(comp))
         {
-            return true;
+            return GenTree::VisitResult::Abort;
         }
-    }
-    return false;
+
+        return GenTree::VisitResult::Continue;
+    };
+
+    return VisitOperands(visit) == GenTree::VisitResult::Abort;
 }
 
 //------------------------------------------------------------------------------

src/coreclr/jit/gentree.h

@@ -2359,11 +2359,7 @@ struct GenTree
     // Note that this function does not respect `GTF_REVERSE_OPS`. This is always safe in LIR, but may be dangerous
     // in HIR if for some reason you need to visit operands in the order in which they will execute.
     template <typename TVisitor>
-    void VisitOperands(TVisitor visitor);
-
-private:
-    template <typename TVisitor>
-    void VisitBinOpOperands(TVisitor visitor);
+    VisitResult VisitOperands(TVisitor visitor);
 
 public:
     bool Precedes(GenTree* other);

src/coreclr/jit/liveness.cpp

@@ -1624,7 +1624,7 @@ bool Compiler::fgTryRemoveNonLocal(GenTree* node, LIR::Range* blockRange)
         // (as opposed to side effects of their children).
         // This default case should never include calls or stores.
         assert(!node->OperRequiresAsgFlag() && !node->OperIs(GT_CALL));
-        if (!node->gtSetFlags() && !node->NodeOrContainedOperandsMayThrow(this))
+        if (!node->gtSetFlags() && !node->OperMayThrow(this) && fgCanUncontainOrRemoveOperands(node))
         {
             JITDUMP("Removing dead node:\n");
             DISPNODE(node);
@@ -1683,6 +1683,38 @@ bool Compiler::fgTryRemoveDeadStoreLIR(GenTree* store, GenTreeLclVarCommon* lclN
     return true;
 }
 
+//---------------------------------------------------------------------
+// fgCanUncontainOrRemoveOperands - Check if the operands of a node that is
+// slated for removal can be either uncontained or deleted entirely.
+//
+// Arguments:
+//   node - The node whose operands are to be checked
+//
+// Return Value:
+//   Whether the operands can be uncontained or removed.
+//
+// Remarks:
+//   Only embedded mask ops do not support standalone codegen. All other
+//   nodes can be uncontained.
+//
+bool Compiler::fgCanUncontainOrRemoveOperands(GenTree* node)
+{
+#ifdef FEATURE_HW_INTRINSICS
+    auto visit = [=](GenTree* op) {
+        if (!op->isContained() || !op->IsEmbMaskOp() || !op->NodeOrContainedOperandsMayThrow(this))
+        {
+            return GenTree::VisitResult::Continue;
+        }
+
+        return GenTree::VisitResult::Abort;
+    };
+
+    return node->VisitOperands(visit) != GenTree::VisitResult::Abort;
+#else
+    return true;
+#endif
+}
+
 //---------------------------------------------------------------------
 // fgRemoveDeadStore - remove a store to a local which has no exposed uses.
 //

src/tests/JIT/Regression/JitBlue/Runtime_116814/Runtime_116814.cs

@@ -0,0 +1,46 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+// Generated by Fuzzlyn v3.2 on 2025-06-15 17:16:07
+// Run on X86 Windows
+// Seed: 11396928574809418613-vectort,vector128,vector256,x86aes,x86avx,x86avx2,x86avx512bw,x86avx512bwvl,x86avx512cd,x86avx512cdvl,x86avx512dq,x86avx512dqvl,x86avx512f,x86avx512fvl,x86bmi1,x86bmi2,x86fma,x86lzcnt,x86pclmulqdq,x86popcnt,x86sse,x86sse2,x86sse3,x86sse41,x86sse42,x86ssse3,x86x86base
+// Reduced from 316.8 KiB to 0.6 KiB in 00:08:07
+// Hits JIT assert for Release:
+// Assertion failed '!node->IsUnusedValue() || !node->OperIs(GT_FIELD_LIST, GT_INIT_VAL)' in 'C1:M1(byref):byte:this' during 'Global local var liveness' (IL size 51; hash 0x17217cb6; FullOpts)
+// 
+//     File: D:\a\_work\1\s\src\coreclr\jit\lir.cpp Line: 1636
+// 
+using System.Runtime.CompilerServices;
+using Xunit;
+
+public class Runtime_116814
+{
+    static C0 s_3;
+    static byte s_11;
+
+    [Fact]
+    public static void TestEntryPoint()
+    {
+        s_3 = new C0();
+        (new C1 { F3 = s_3 }).M1(ref s_11);
+    }
+
+    class C0
+    {
+        public short F3;
+        public long F4;
+    }
+
+    class C1
+    {
+        public byte F0;
+        public C0 F3;
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        public byte M1(ref byte arg0)
+        {
+            sbyte var0 = (sbyte)((this.F3.F4 * s_3.F3) << -1);
+            System.Console.WriteLine(var0);
+            return this.F0;
+        }
+    }
+}

src/tests/JIT/Regression/JitBlue/Runtime_116814/Runtime_116814.csproj

@@ -0,0 +1,8 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+</Project>