Fix missing side effect flags on HWI nodes (#67206)

* Set side-effect flags in constructor for HWIs

* Add test

* Experiment: NULL_CHECK LDOBJs

src/coreclr/jit/gentree.h

@@ -5598,17 +5598,7 @@ struct GenTreeHWIntrinsic : public GenTreeJitIntrinsic
                        bool                   isSimdAsHWIntrinsic)
         : GenTreeJitIntrinsic(GT_HWINTRINSIC, type, std::move(nodeBuilder), simdBaseJitType, simdSize)
     {
-        SetHWIntrinsicId(hwIntrinsicID);
-
-        if (OperIsMemoryStore())
-        {
-            gtFlags |= (GTF_GLOB_REF | GTF_ASG);
-        }
-
-        if (isSimdAsHWIntrinsic)
-        {
-            gtFlags |= GTF_SIMDASHW_OP;
-        }
+        Initialize(hwIntrinsicID, isSimdAsHWIntrinsic);
     }
 
     template <typename... Operands>
@@ -5621,17 +5611,7 @@ struct GenTreeHWIntrinsic : public GenTreeJitIntrinsic
                        Operands... operands)
         : GenTreeJitIntrinsic(GT_HWINTRINSIC, type, allocator, simdBaseJitType, simdSize, operands...)
     {
-        SetHWIntrinsicId(hwIntrinsicID);
-
-        if ((sizeof...(Operands) > 0) && OperIsMemoryStore())
-        {
-            gtFlags |= (GTF_GLOB_REF | GTF_ASG);
-        }
-
-        if (isSimdAsHWIntrinsic)
-        {
-            gtFlags |= GTF_SIMDASHW_OP;
-        }
+        Initialize(hwIntrinsicID, isSimdAsHWIntrinsic);
     }
 
 #if DEBUGGABLE_GENTREE
@@ -5650,6 +5630,7 @@ struct GenTreeHWIntrinsic : public GenTreeJitIntrinsic
     {
         return (gtFlags & GTF_SIMDASHW_OP) != 0;
     }
+
     unsigned GetResultOpNumForFMA(GenTree* use, GenTree* op1, GenTree* op2, GenTree* op3);
 
     NamedIntrinsic GetHWIntrinsicId() const;
@@ -5740,6 +5721,29 @@ struct GenTreeHWIntrinsic : public GenTreeJitIntrinsic
 
 private:
     void SetHWIntrinsicId(NamedIntrinsic intrinsicId);
+
+    void Initialize(NamedIntrinsic intrinsicId, bool isSimdAsHWIntrinsic)
+    {
+        SetHWIntrinsicId(intrinsicId);
+
+        bool isStore = OperIsMemoryStore();
+        bool isLoad  = OperIsMemoryLoad();
+
+        if (isStore || isLoad)
+        {
+            gtFlags |= (GTF_GLOB_REF | GTF_EXCEPT);
+
+            if (isStore)
+            {
+                gtFlags |= GTF_ASG;
+            }
+        }
+
+        if (isSimdAsHWIntrinsic)
+        {
+            gtFlags |= GTF_SIMDASHW_OP;
+        }
+    }
 };
 #endif // FEATURE_HW_INTRINSICS
 

src/coreclr/jit/hwintrinsic.cpp

@@ -764,6 +764,7 @@ GenTree* Compiler::impHWIntrinsic(NamedIntrinsic        intrinsic,
     int                    numArgs         = sig->numArgs;
     var_types              retType         = JITtype2varType(sig->retType);
     CorInfoType            simdBaseJitType = CORINFO_TYPE_UNDEF;
+    GenTree*               retNode         = nullptr;
 
     if (retType == TYP_STRUCT)
     {
@@ -1005,11 +1006,10 @@ GenTree* Compiler::impHWIntrinsic(NamedIntrinsic        intrinsic,
             return nullptr;
         }
 
-        GenTree*            op1     = nullptr;
-        GenTree*            op2     = nullptr;
-        GenTree*            op3     = nullptr;
-        GenTree*            op4     = nullptr;
-        GenTreeHWIntrinsic* retNode = nullptr;
+        GenTree* op1 = nullptr;
+        GenTree* op2 = nullptr;
+        GenTree* op3 = nullptr;
+        GenTree* op4 = nullptr;
 
         switch (numArgs)
         {
@@ -1052,9 +1052,10 @@ GenTree* Compiler::impHWIntrinsic(NamedIntrinsic        intrinsic,
 
                         CorInfoType auxiliaryType = CORINFO_TYPE_UNDEF;
 
-                        if (!varTypeIsSIMD(op1->TypeGet()))
+                        if (!varTypeIsSIMD(op1))
                         {
                             auxiliaryType = CORINFO_TYPE_PTR;
+                            retNode->gtFlags |= (GTF_EXCEPT | GTF_GLOB_REF);
                         }
 
                         retNode->AsHWIntrinsic()->SetAuxiliaryJitType(auxiliaryType);
@@ -1179,31 +1180,23 @@ GenTree* Compiler::impHWIntrinsic(NamedIntrinsic        intrinsic,
                 retNode = gtNewSimdHWIntrinsicNode(retType, op1, op2, op3, op4, intrinsic, simdBaseJitType, simdSize);
                 break;
 #endif
-
             default:
-                return nullptr;
+                break;
         }
+    }
+    else
+    {
+        retNode = impSpecialIntrinsic(intrinsic, clsHnd, method, sig, simdBaseJitType, retType, simdSize);
+    }
 
-        const bool isMemoryStore = retNode->OperIsMemoryStore();
-
-        if (isMemoryStore || retNode->OperIsMemoryLoad())
-        {
-            if (isMemoryStore)
-            {
-                // A MemoryStore operation is an assignment
-                retNode->gtFlags |= GTF_ASG;
-            }
-
-            // This operation contains an implicit indirection
-            //   it could point into the global heap or
-            //   it could throw a null reference exception.
-            //
-            retNode->gtFlags |= (GTF_GLOB_REF | GTF_EXCEPT);
-        }
-        return retNode;
+    if ((retNode != nullptr) && retNode->OperIs(GT_HWINTRINSIC))
+    {
+        assert(!retNode->OperMayThrow(this) || ((retNode->gtFlags & GTF_EXCEPT) != 0));
+        assert(!retNode->OperRequiresAsgFlag() || ((retNode->gtFlags & GTF_ASG) != 0));
+        assert(!retNode->OperIsImplicitIndir() || ((retNode->gtFlags & GTF_GLOB_REF) != 0));
     }
 
-    return impSpecialIntrinsic(intrinsic, clsHnd, method, sig, simdBaseJitType, retType, simdSize);
+    return retNode;
 }
 
 #endif // FEATURE_HW_INTRINSICS

src/mono/mono/mini/interp/interp.c

@@ -5291,7 +5291,9 @@ MINT_IN_CASE(MINT_BRTRUE_I8_SP) ZEROP_SP(gint64, !=); MINT_IN_BREAK;
 		}
 		MINT_IN_CASE(MINT_LDOBJ_VT) {
 			guint16 size = ip [3];
-			memcpy (locals + ip [1], LOCAL_VAR (ip [2], gpointer), size);
+			gpointer srcAddr = LOCAL_VAR (ip [2], gpointer);
+			NULL_CHECK (srcAddr);
+			memcpy (locals + ip [1], srcAddr, size);
 			ip += 4;
 			MINT_IN_BREAK;
 		}

src/tests/JIT/HardwareIntrinsics/General/HwiOp/HwiSideEffects.cs

 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System;
+using System.Runtime.Intrinsics;
 using System.Runtime.Intrinsics.X86;
 using System.Runtime.CompilerServices;
 
@@ -15,6 +17,11 @@ public static int Main()
             return 101;
         }
 
+        if (!ProblemWithThrowingLoads(null))
+        {
+            return 102;
+        }
+
         return 100;
     }
 
@@ -34,4 +41,20 @@ private static uint ProblemWithInterferenceChecks(uint a)
 
         return x;
     }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    private static bool ProblemWithThrowingLoads(int* a)
+    {
+        bool result = false;
+        try
+        {
+            Vector128.Load(a);
+        }
+        catch (NullReferenceException)
+        {
+            result = true;
+        }
+
+        return result;
+    }
 }