JIT: Avoid boxing ArgumentNullException.ThrowIfNull arguments in Tier…

…-0 (#104815)


Co-authored-by: Jakob Botsch Nielsen <Jakob.botsch.nielsen@gmail.com>

src/coreclr/jit/compiler.h

@@ -4541,6 +4541,8 @@ class Compiler
 
     GenTree* impDuplicateWithProfiledArg(GenTreeCall* call, IL_OFFSET ilOffset);
 
+    GenTree* impThrowIfNull(GenTreeCall* call);
+
 #ifdef DEBUG
     var_types impImportJitTestLabelMark(int numArgs);
 #endif // DEBUG

src/coreclr/jit/gentree.h

@@ -2265,6 +2265,7 @@ struct GenTree
         return OperGet() == GT_CALL;
     }
     inline bool IsHelperCall();
+    inline bool IsHelperCall(Compiler* compiler, unsigned helper);
 
     bool gtOverflow() const;
     bool gtOverflowEx() const;
@@ -10500,7 +10501,12 @@ inline bool GenTree::IsCnsVec() const
 
 inline bool GenTree::IsHelperCall()
 {
-    return OperGet() == GT_CALL && AsCall()->IsHelperCall();
+    return IsCall() && AsCall()->IsHelperCall();
+}
+
+inline bool GenTree::IsHelperCall(Compiler* compiler, unsigned helper)
+{
+    return IsCall() && AsCall()->IsHelperCall(compiler, helper);
 }
 
 inline var_types GenTree::CastFromType()

src/coreclr/jit/importercalls.cpp

@@ -1330,6 +1330,10 @@ var_types Compiler::impImportCall(OPCODE                  opcode,
         }
         else
         {
+            if (call->IsCall() && call->AsCall()->IsSpecialIntrinsic(this, NI_System_ArgumentNullException_ThrowIfNull))
+            {
+                call = impThrowIfNull(call->AsCall());
+            }
             impAppendTree(call, CHECK_SPILL_ALL, impCurStmtDI);
         }
     }
@@ -1528,6 +1532,97 @@ var_types Compiler::impImportCall(OPCODE                  opcode,
 #pragma warning(pop)
 #endif
 
+//------------------------------------------------------------------------
+// impThrowIfNull: Remove redundandant boxing from ArgumentNullException_ThrowIfNull
+//    it is done for Tier0 where we can't remove it without inlining otherwise.
+//
+// Arguments:
+//    call -- call representing ArgumentNullException_ThrowIfNull
+//
+// Return Value:
+//    Optimized tree (or the original call tree if we can't optimize it).
+//
+GenTree* Compiler::impThrowIfNull(GenTreeCall* call)
+{
+    // We have two overloads:
+    //
+    //  void ThrowIfNull(object argument, string paramName = null)
+    //  void ThrowIfNull(object argument, ExceptionArgument paramName)
+    //
+    assert(call->IsSpecialIntrinsic(this, NI_System_ArgumentNullException_ThrowIfNull));
+    assert(call->gtArgs.CountUserArgs() == 2);
+    assert(call->TypeIs(TYP_VOID));
+
+    if (opts.compDbgCode || opts.jitFlags->IsSet(JitFlags::JIT_FLAG_MIN_OPT))
+    {
+        // Don't fold it for debug code or forced MinOpts
+        return call;
+    }
+
+    GenTree* value     = call->gtArgs.GetUserArgByIndex(0)->GetNode();
+    GenTree* valueName = call->gtArgs.GetUserArgByIndex(1)->GetNode();
+
+    // Case 1: value-type (non-nullable):
+    //
+    //  ArgumentNullException_ThrowIfNull(GT_BOX(value), valueName)
+    //    ->
+    //  NOP (with side-effects if any)
+    //
+    if (value->OperIs(GT_BOX))
+    {
+        // Now we need to spill the addr and argName arguments in the correct order
+        // to preserve possible side effects.
+        unsigned boxedValTmp     = lvaGrabTemp(true DEBUGARG("boxedVal spilled"));
+        unsigned boxedArgNameTmp = lvaGrabTemp(true DEBUGARG("boxedArg spilled"));
+        impStoreToTemp(boxedValTmp, value, CHECK_SPILL_ALL);
+        impStoreToTemp(boxedArgNameTmp, valueName, CHECK_SPILL_ALL);
+        gtTryRemoveBoxUpstreamEffects(value, BR_REMOVE_AND_NARROW);
+        return gtNewNothingNode();
+    }
+
+    // Case 2: nullable:
+    //
+    //  ArgumentNullException.ThrowIfNull(CORINFO_HELP_BOX_NULLABLE(classHandle, addr), valueName);
+    //    ->
+    //  addr->hasValue != 0 ? NOP : ArgumentNullException.ThrowIfNull(null, valueNameTmp)
+    //
+    if (opts.OptimizationEnabled() || !value->IsHelperCall(this, CORINFO_HELP_BOX_NULLABLE))
+    {
+        // We're not boxing - bail out.
+        // NOTE: when opts are enabled, we remove the box as is (with better CQ)
+        return call;
+    }
+
+    GenTree* boxHelperClsArg  = value->AsCall()->gtArgs.GetUserArgByIndex(0)->GetNode();
+    GenTree* boxHelperAddrArg = value->AsCall()->gtArgs.GetUserArgByIndex(1)->GetNode();
+
+    if ((boxHelperClsArg->gtFlags & GTF_SIDE_EFFECT) != 0)
+    {
+        // boxHelperClsArg is always just a class handle constant, so we don't bother spilling it.
+        return call;
+    }
+
+    // Now we need to spill the addr and argName arguments in the correct order
+    // to preserve possible side effects.
+    unsigned boxedValTmp     = lvaGrabTemp(true DEBUGARG("boxedVal spilled"));
+    unsigned boxedArgNameTmp = lvaGrabTemp(true DEBUGARG("boxedArg spilled"));
+    impStoreToTemp(boxedValTmp, boxHelperAddrArg, CHECK_SPILL_ALL);
+    impStoreToTemp(boxedArgNameTmp, valueName, CHECK_SPILL_ALL);
+
+    // Change arguments to 'ThrowIfNull(null, valueNameTmp)'
+    call->gtArgs.GetUserArgByIndex(0)->SetEarlyNode(gtNewNull());
+    call->gtArgs.GetUserArgByIndex(1)->SetEarlyNode(gtNewLclvNode(boxedArgNameTmp, valueName->TypeGet()));
+
+    // This is Tier0 specific, so we create a raw indir node to access Nullable<T>.hasValue field
+    // which is the first field of Nullable<T> struct and is of type 'bool'.
+    //
+    static_assert_no_msg(OFFSETOF__CORINFO_NullableOfT__hasValue == 0);
+    GenTree*   hasValueField = gtNewIndir(TYP_UBYTE, gtNewLclvNode(boxedValTmp, boxHelperAddrArg->TypeGet()));
+    GenTreeOp* cond          = gtNewOperNode(GT_NE, TYP_INT, hasValueField, gtNewIconNode(0));
+
+    return gtNewQmarkNode(TYP_VOID, cond, gtNewColonNode(TYP_VOID, gtNewNothingNode(), call));
+}
+
 //------------------------------------------------------------------------
 // impDuplicateWithProfiledArg: duplicates a call with a profiled argument, e.g.:
 //    Given `Buffer.Memmove(dst, src, len)` call,
@@ -3257,6 +3352,9 @@ GenTree* Compiler::impIntrinsic(CORINFO_CLASS_HANDLE    clsHnd,
             // to avoid some unnecessary boxing
             case NI_System_Enum_HasFlag:
 
+            // This one is made intrinsic specifically to avoid boxing in Tier0
+            case NI_System_ArgumentNullException_ThrowIfNull:
+
             // Most atomics are compiled to single instructions
             case NI_System_Threading_Interlocked_And:
             case NI_System_Threading_Interlocked_Or:
@@ -3787,6 +3885,10 @@ GenTree* Compiler::impIntrinsic(CORINFO_CLASS_HANDLE    clsHnd,
                 break;
             }
 
+            case NI_System_ArgumentNullException_ThrowIfNull:
+                isSpecial = true;
+                break;
+
             case NI_System_Enum_HasFlag:
             {
                 GenTree* thisOp  = impStackTop(1).val;
@@ -9826,6 +9928,13 @@ NamedIntrinsic Compiler::lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method)
                             result = NI_System_Activator_DefaultConstructorOf;
                         }
                     }
+                    else if (strcmp(className, "ArgumentNullException") == 0)
+                    {
+                        if (strcmp(methodName, "ThrowIfNull") == 0)
+                        {
+                            result = NI_System_ArgumentNullException_ThrowIfNull;
+                        }
+                    }
                     else if (strcmp(className, "Array") == 0)
                     {
                         if (strcmp(methodName, "Clone") == 0)

src/coreclr/jit/namedintrinsiclist.h

@@ -13,6 +13,8 @@ enum NamedIntrinsic : unsigned short
 {
     NI_Illegal = 0,
 
+    NI_System_ArgumentNullException_ThrowIfNull,
+
     NI_System_Enum_HasFlag,
 
     NI_System_BitConverter_DoubleToInt64Bits,

src/libraries/System.Private.CoreLib/src/System/ArgumentNullException.cs

@@ -51,6 +51,7 @@ protected ArgumentNullException(SerializationInfo info, StreamingContext context
         /// <summary>Throws an <see cref="ArgumentNullException"/> if <paramref name="argument"/> is null.</summary>
         /// <param name="argument">The reference type argument to validate as non-null.</param>
         /// <param name="paramName">The name of the parameter with which <paramref name="argument"/> corresponds.</param>
+        [Intrinsic] // Tier0 intrinsic to avoid redundant boxing in generics
         public static void ThrowIfNull([NotNull] object? argument, [CallerArgumentExpression(nameof(argument))] string? paramName = null)
         {
             if (argument is null)
@@ -59,6 +60,7 @@ public static void ThrowIfNull([NotNull] object? argument, [CallerArgumentExpres
             }
         }
 
+        [Intrinsic] // Tier0 intrinsic to avoid redundant boxing in generics
         internal static void ThrowIfNull([NotNull] object? argument, ExceptionArgument paramName)
         {
             if (argument is null)