Allow constant propagation of Vector.Zero. (#65028)

* Initial work

* Added a comma to display

* Cleanup

* Fixing build

* More cleanup

* Update comment

* Update comment

* Added CompareEqual Vector64/128 with Zero tests

* Do not contain op1 for now

* Wrong intrinsic id used

* Removing generated tests

* Removing generated tests

* Added CompareEqual tests

* Supporting containment for first operand

* Fix test build

* Passing correct register

* Check IsVectorZero before not allocing a register

* Update comment

* Fixing test

* Minor format change

* Fixed formatting

* Renamed test

* Adding AdvSimd_Arm64 tests:

* Adding support for rest of 'cmeq' and 'fcmeq' instructions

* Removing github csproj

* Minor test fix

* Fixed tests

* Fix print

* Minor format change

* Fixing test

* Initial commit for Vector.Create to Vector.Zero normalization

* Added some emitter tests

* Feedback

* Update emitarm64.cpp

* Feedback

* Handling variations of Vector.Create

* Use Operands iterator instead of edges

* Fix condition

* Simplify

* format

* Fixed IsFloatPositiveZero

* Uncomment

* Updated tests to include Vector64.Create/Vector128.Create for ARM64

* Making implementation of IsFloatPositiveZero explicit

* Update src/coreclr/jit/gentree.cpp
Co-authored-by: NSingleAccretion <62474226+SingleAccretion@users.noreply.github.com>

* Feedback

* Update comment

* Update comment

* Do not perform optimization when VN CSE phase

* use ResetHWIntrinsicId

* Assert !optValnumCSE_phase

* Simplify IsVectorZero

* Simplify IsVectorZero

* Simplify some uses of Vector*_get_Zero

* Added another test

* Fixed formatting

* Revert lowering removal

* Initial work for optimizations on VectorZero value numbering

* Allowing all Vector.Zero to be constant prop'ed. Added VNFuncSimdTypeInfo.

* Update gentree.h

* Quick rename

* Removed extra variable

* Added default case

* Format

* Fixed vnDumpSimdType to take into account CorInfoType

* Fixed gtNewSimdZeroNode to produce the right Vector*_get_Zero based on simdSize

* Formatting

* Feedback and a loop test

* Added another test. Formatting fixes

* Added GetSimdBaseJitPreciseType

* Feedback

* Minor fix

* Minor comment update

* Added another comment

* Added another comment

* Added another comment

* Update comment

* Formatting

* Feedback

* Fixing build

* Feedback

* Update assertionprop.cpp

* Formatting
Co-authored-by: NSingleAccretion <62474226+SingleAccretion@users.noreply.github.com>

src/coreclr/jit/assertionprop.cpp

@@ -2882,8 +2882,8 @@ GenTree* Compiler::optVNConstantPropOnTree(BasicBlock* block, GenTree* tree)
     ValueNumPair vnPair = tree->gtVNPair;
     ValueNum     vnCns  = vnStore->VNConservativeNormalValue(vnPair);
 
-    // Check if node evaluates to a constant.
-    if (!vnStore->IsVNConstant(vnCns))
+    // Check if node evaluates to a constant or Vector.Zero.
+    if (!vnStore->IsVNConstant(vnCns) && !vnStore->IsVNVectorZero(vnCns))
     {
         return nullptr;
     }
@@ -3042,6 +3042,24 @@ GenTree* Compiler::optVNConstantPropOnTree(BasicBlock* block, GenTree* tree)
         }
         break;
 
+#if FEATURE_HW_INTRINSICS
+        case TYP_SIMD8:
+        case TYP_SIMD12:
+        case TYP_SIMD16:
+        case TYP_SIMD32:
+        {
+            assert(vnStore->IsVNVectorZero(vnCns));
+            VNSimdTypeInfo vnInfo = vnStore->GetVectorZeroSimdTypeOfVN(vnCns);
+
+            assert(vnInfo.m_simdBaseJitType != CORINFO_TYPE_UNDEF);
+            assert(vnInfo.m_simdSize != 0);
+            assert(getSIMDTypeForSize(vnInfo.m_simdSize) == vnStore->TypeOfVN(vnCns));
+
+            conValTree = gtNewSimdZeroNode(tree->TypeGet(), vnInfo.m_simdBaseJitType, vnInfo.m_simdSize, true);
+        }
+        break;
+#endif
+
         case TYP_BYREF:
             // Do not support const byref optimization.
             break;
@@ -5449,7 +5467,8 @@ struct VNAssertionPropVisitorInfo
 //
 GenTree* Compiler::optExtractSideEffListFromConst(GenTree* tree)
 {
-    assert(vnStore->IsVNConstant(vnStore->VNConservativeNormalValue(tree->gtVNPair)));
+    assert(vnStore->IsVNConstant(vnStore->VNConservativeNormalValue(tree->gtVNPair)) ||
+           vnStore->IsVNVectorZero(vnStore->VNConservativeNormalValue(tree->gtVNPair)));
 
     GenTree* sideEffList = nullptr;
 

src/coreclr/jit/gentree.cpp

@@ -21518,7 +21518,7 @@ GenTree* Compiler::gtNewSimdZeroNode(var_types   type,
 #if defined(TARGET_XARCH)
     intrinsic = (simdSize == 32) ? NI_Vector256_get_Zero : NI_Vector128_get_Zero;
 #elif defined(TARGET_ARM64)
-    intrinsic     = (simdSize == 16) ? NI_Vector128_get_Zero : NI_Vector64_get_Zero;
+    intrinsic     = (simdSize > 8) ? NI_Vector128_get_Zero : NI_Vector64_get_Zero;
 #else
 #error Unsupported platform
 #endif // !TARGET_XARCH && !TARGET_ARM64

src/coreclr/jit/gentree.h

@@ -5323,6 +5323,34 @@ public:
         return (CorInfoType)gtSimdBaseJitType;
     }
 
+    CorInfoType GetNormalizedSimdBaseJitType() const
+    {
+        CorInfoType simdBaseJitType = GetSimdBaseJitType();
+        switch (simdBaseJitType)
+        {
+            case CORINFO_TYPE_NATIVEINT:
+            {
+#ifdef TARGET_64BIT
+                return CORINFO_TYPE_LONG;
+#else
+                return CORINFO_TYPE_INT;
+#endif
+            }
+
+            case CORINFO_TYPE_NATIVEUINT:
+            {
+#ifdef TARGET_64BIT
+                return CORINFO_TYPE_ULONG;
+#else
+                return CORINFO_TYPE_UINT;
+#endif
+            }
+
+            default:
+                return simdBaseJitType;
+        }
+    }
+
     void SetSimdBaseJitType(CorInfoType simdBaseJitType)
     {
         gtSimdBaseJitType = (unsigned char)simdBaseJitType;

src/coreclr/jit/liveness.cpp

@@ -2066,9 +2066,6 @@ void Compiler::fgComputeLifeLIR(VARSET_TP& life, BasicBlock* block, VARSET_VALAR
             case GT_PUTARG_STK:
             case GT_IL_OFFSET:
             case GT_KEEPALIVE:
-#ifdef FEATURE_HW_INTRINSICS
-            case GT_HWINTRINSIC:
-#endif // FEATURE_HW_INTRINSICS
                 // Never remove these nodes, as they are always side-effecting.
                 //
                 // NOTE: the only side-effect of some of these nodes (GT_CMP, GT_SUB_HI) is a write to the flags
@@ -2076,6 +2073,16 @@ void Compiler::fgComputeLifeLIR(VARSET_TP& life, BasicBlock* block, VARSET_VALAR
                 // Properly modeling this would allow these nodes to be removed.
                 break;
 
+#ifdef FEATURE_HW_INTRINSICS
+            case GT_HWINTRINSIC:
+                // Conservative: This only removes Vector.Zero nodes, but could be expanded.
+                if (node->IsVectorZero())
+                {
+                    fgTryRemoveNonLocal(node, &blockRange);
+                }
+                break;
+#endif // FEATURE_HW_INTRINSICS
+
             case GT_NOP:
             {
                 // NOTE: we need to keep some NOPs around because they are referenced by calls. See the dead store

src/coreclr/jit/valuenum.cpp

@@ -1888,7 +1888,7 @@ ValueNum ValueNumStore::VNZeroForType(var_types typ)
             // "fully zeroed" vectors, and here we may be loading one from memory, leaving upper
             // bits undefined. So using "SIMD_Init" is "the next best thing", so to speak, and
             // TYP_FLOAT is one of the more popular base types, so that's why we use it here.
-            return VNForFunc(typ, VNF_SIMD_Init, VNForFloatCon(0), VNForSimdType(genTypeSize(typ), TYP_FLOAT));
+            return VNForFunc(typ, VNF_SIMD_Init, VNForFloatCon(0), VNForSimdType(genTypeSize(typ), CORINFO_TYPE_FLOAT));
 #endif // FEATURE_SIMD
 
         // These should be unreached.
@@ -1935,9 +1935,9 @@ ValueNum ValueNumStore::VNOneForType(var_types typ)
 }
 
 #ifdef FEATURE_SIMD
-ValueNum ValueNumStore::VNForSimdType(unsigned simdSize, var_types simdBaseType)
+ValueNum ValueNumStore::VNForSimdType(unsigned simdSize, CorInfoType simdBaseJitType)
 {
-    ValueNum baseTypeVN = VNForIntCon(INT32(simdBaseType));
+    ValueNum baseTypeVN = VNForIntCon(INT32(simdBaseJitType));
     ValueNum sizeVN     = VNForIntCon(simdSize);
     ValueNum simdTypeVN = VNForFunc(TYP_REF, VNF_SimdType, sizeVN, baseTypeVN);
 
@@ -4595,6 +4595,114 @@ bool ValueNumStore::IsVNConstant(ValueNum vn)
     }
 }
 
+//------------------------------------------------------------------------
+// IsVNVectorZero: Checks if the value number is a Vector*_get_Zero.
+//
+// Arguments:
+//    vn - The value number.
+//
+// Return Value:
+//    true  - The value number is a Vector*_get_Zero.
+//    false - The value number is not a Vector*_get_Zero.
+bool ValueNumStore::IsVNVectorZero(ValueNum vn)
+{
+#ifdef FEATURE_SIMD
+    VNSimdTypeInfo vnInfo = GetVectorZeroSimdTypeOfVN(vn);
+    // Check the size to see if we got a valid SIMD type.
+    // '0' means it is not valid.
+    if (vnInfo.m_simdSize != 0)
+    {
+        return true;
+    }
+#endif
+    return false;
+}
+
+#ifdef FEATURE_SIMD
+//------------------------------------------------------------------------
+// GetSimdTypeOfVN: Returns the SIMD type information based on the given value number.
+//
+// Arguments:
+//    vn - The value number.
+//
+// Return Value:
+//    Returns VNSimdTypeInfo(0, CORINFO_TYPE_UNDEF) if the given value number has not been given a SIMD type.
+VNSimdTypeInfo ValueNumStore::GetSimdTypeOfVN(ValueNum vn)
+{
+    VNSimdTypeInfo vnInfo;
+
+    // The SIMD type is encoded as a function,
+    // even though it is not actually a function.
+    VNFuncApp simdType;
+    if (GetVNFunc(vn, &simdType) && simdType.m_func == VNF_SimdType)
+    {
+        assert(simdType.m_arity == 2);
+        vnInfo.m_simdSize        = GetConstantInt32(simdType.m_args[0]);
+        vnInfo.m_simdBaseJitType = (CorInfoType)GetConstantInt32(simdType.m_args[1]);
+        return vnInfo;
+    }
+
+    vnInfo.m_simdSize        = 0;
+    vnInfo.m_simdBaseJitType = CORINFO_TYPE_UNDEF;
+    return vnInfo;
+}
+
+//------------------------------------------------------------------------
+// GetVectorZeroSimdTypeOfVN: Returns the SIMD type information based on the given value number
+//                            if it's Vector*_get_Zero.
+//
+// Arguments:
+//    vn - The value number.
+//
+// Return Value:
+//    Returns VNSimdTypeInfo(0, CORINFO_TYPE_UNDEF) if the given value number has not been given a SIMD type
+//    for a Vector*_get_Zero value number.
+//
+// REVIEW: Vector*_get_Zero nodes in VN currently encode their SIMD type for
+//         conservative reasons. In the future, it might be possible not do this
+//         on most platforms since Vector*_get_Zero's base type does not matter.
+VNSimdTypeInfo ValueNumStore::GetVectorZeroSimdTypeOfVN(ValueNum vn)
+{
+#ifdef FEATURE_HW_INTRINSICS
+    // REVIEW: This will only return true if Vector*_get_Zero encodes
+    //         its base type as an argument. On XARCH there may be
+    //         scenarios where Vector*_get_Zero will not encode its base type;
+    //         therefore, returning false here.
+    // Vector*_get_Zero does not have any arguments,
+    // but its SIMD type is encoded as an argument.
+    VNFuncApp funcApp;
+    if (GetVNFunc(vn, &funcApp) && funcApp.m_arity == 1)
+    {
+        switch (funcApp.m_func)
+        {
+            case VNF_HWI_Vector128_get_Zero:
+#if defined(TARGET_XARCH)
+            case VNF_HWI_Vector256_get_Zero:
+#elif defined(TARGET_ARM64)
+            case VNF_HWI_Vector64_get_Zero:
+#endif
+            {
+                return GetSimdTypeOfVN(funcApp.m_args[0]);
+            }
+
+            default:
+            {
+                VNSimdTypeInfo vnInfo;
+                vnInfo.m_simdSize        = 0;
+                vnInfo.m_simdBaseJitType = CORINFO_TYPE_UNDEF;
+                return vnInfo;
+            }
+        }
+    }
+#endif
+
+    VNSimdTypeInfo vnInfo;
+    vnInfo.m_simdSize        = 0;
+    vnInfo.m_simdBaseJitType = CORINFO_TYPE_UNDEF;
+    return vnInfo;
+}
+#endif // FEATURE_SIMD
+
 bool ValueNumStore::IsVNInt32Constant(ValueNum vn)
 {
     if (!IsVNConstant(vn))
@@ -6178,10 +6286,10 @@ void ValueNumStore::vnDumpSimdType(Compiler* comp, VNFuncApp* simdType)
     assert(IsVNConstant(simdType->m_args[0]));
     assert(IsVNConstant(simdType->m_args[1]));
 
-    int       simdSize = ConstantValue<int>(simdType->m_args[0]);
-    var_types baseType = (var_types)ConstantValue<int>(simdType->m_args[1]);
+    int         simdSize    = ConstantValue<int>(simdType->m_args[0]);
+    CorInfoType baseJitType = (CorInfoType)ConstantValue<int>(simdType->m_args[1]);
 
-    printf("%s(simd%d, %s)", VNFuncName(simdType->m_func), simdSize, varTypeName(baseType));
+    printf("%s(simd%d, %s)", VNFuncName(simdType->m_func), simdSize, varTypeName(JitType2PreciseVarType(baseJitType)));
 }
 #endif // FEATURE_SIMD
 
@@ -9422,7 +9530,7 @@ void Compiler::fgValueNumberSimd(GenTreeSIMD* tree)
 
         if (encodeResultType)
         {
-            ValueNum simdTypeVN = vnStore->VNForSimdType(tree->GetSimdSize(), tree->GetSimdBaseType());
+            ValueNum simdTypeVN = vnStore->VNForSimdType(tree->GetSimdSize(), tree->GetNormalizedSimdBaseJitType());
             resvnp.SetBoth(simdTypeVN);
 
 #ifdef DEBUG
@@ -9537,7 +9645,7 @@ void Compiler::fgValueNumberHWIntrinsic(GenTreeHWIntrinsic* tree)
 
     if (encodeResultType)
     {
-        ValueNum simdTypeVN = vnStore->VNForSimdType(tree->GetSimdSize(), tree->GetSimdBaseType());
+        ValueNum simdTypeVN = vnStore->VNForSimdType(tree->GetSimdSize(), tree->GetNormalizedSimdBaseJitType());
         resvnp.SetBoth(simdTypeVN);
 
 #ifdef DEBUG

src/coreclr/jit/valuenum.h

@@ -184,6 +184,13 @@ struct VNFuncApp
     }
 };
 
+// An instance of this struct represents the decoded information of a SIMD type from a value number.
+struct VNSimdTypeInfo
+{
+    unsigned int m_simdSize;
+    CorInfoType  m_simdBaseJitType;
+};
+
 // We use a unique prefix character when printing value numbers in dumps:  i.e.  $1c0
 // This define is used with string concatenation to put this in printf format strings
 #define FMT_VN "$%x"
@@ -463,7 +470,7 @@ public:
 
 #ifdef FEATURE_SIMD
     // A helper function for constructing VNF_SimdType VNs.
-    ValueNum VNForSimdType(unsigned simdSize, var_types simdBaseType);
+    ValueNum VNForSimdType(unsigned simdSize, CorInfoType simdBaseJitType);
 #endif // FEATURE_SIMD
 
     // Create or return the existimg value number representing a singleton exception set
@@ -714,6 +721,14 @@ public:
     // Returns true iff the VN represents a (non-handle) constant.
     bool IsVNConstant(ValueNum vn);
 
+    bool IsVNVectorZero(ValueNum vn);
+
+#ifdef FEATURE_SIMD
+    VNSimdTypeInfo GetSimdTypeOfVN(ValueNum vn);
+
+    VNSimdTypeInfo GetVectorZeroSimdTypeOfVN(ValueNum vn);
+#endif
+
     // Returns true iff the VN represents an integer constant.
     bool IsVNInt32Constant(ValueNum vn);
 

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

@@ -149,6 +149,46 @@ static Vector128<float> AdvSimd_CompareEqual_Vector128_Single_CreateZeroZeroZero
         return AdvSimd.CompareEqual(left, asVar);
     }
 
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static Vector128<float> AdvSimd_CompareEqual_Vector128_Single_CreateZeroZeroZeroZero_AsVariableLoop(Vector128<float> left)
+    {
+        Vector128<float> result = default;
+        var asVar = Vector128.Create(0f, 0f, 0f, 0f);
+        for (var i = 0; i < 4; i++)
+        {
+            result = AdvSimd.CompareEqual(left, asVar);
+            result = AdvSimd.CompareEqual(left, asVar);
+            result = AdvSimd.CompareEqual(left, asVar);
+            result = AdvSimd.CompareEqual(left, asVar);
+            for (var j = 0; j < 4; j++)
+            {
+                result = AdvSimd.CompareEqual(left, asVar);
+                result = AdvSimd.CompareEqual(left, asVar);
+                result = AdvSimd.CompareEqual(left, asVar);
+                result = AdvSimd.CompareEqual(left, asVar);
+            }
+        }
+        return result;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static unsafe Vector128<long> AdvSimd_Arm64_CompareEqual_Vector128_Long_AsVariableLoop(Vector128<long> left)
+    {
+        Vector128<long> result = default;
+        Vector128<long> asVar = Vector128.Create((long)0);
+        Vector128<nint> asVar2 = Vector128.Create((nint)0);
+        Vector128<long> asVar3 = asVar2.AsInt64();
+        for (var i = 0; i < 4; i++)
+        {
+            result = AdvSimd.Arm64.CompareEqual(left, asVar);
+            for (var j = 0; j < 4; j++)
+            {
+                result = AdvSimd.Arm64.CompareEqual(left, asVar3);
+            }
+        }
+        return result;
+    }
+
     [MethodImpl(MethodImplOptions.NoInlining)]
     static Vector128<double> AdvSimd_Arm64_CompareEqual_Vector128_Double_Zero(Vector128<double> left)
     {
@@ -564,6 +604,9 @@ static int Tests_AdvSimd()
         if (!ValidateResult_Vector128<float>(AdvSimd_CompareEqual_Vector128_Single_CreateZeroZeroZeroZero_AsVariable(Vector128<float>.Zero), Single.NaN))
             result = -1;
 
+        if (!ValidateResult_Vector128<float>(AdvSimd_CompareEqual_Vector128_Single_CreateZeroZeroZeroZero_AsVariableLoop(Vector128<float>.Zero), Single.NaN))
+            result = -1;
+
         // End CompareEqual Tests
 
         // Begin CompareGreaterThan Tests
@@ -747,6 +790,9 @@ static int Tests_AdvSimd_Arm64()
         if (!ValidateResult_Vector128<long>(AdvSimd_Arm64_CompareEqual_Vector128_Int64_Zero(Vector128<long>.Zero), -1))
             result = -1;
 
+        if (!ValidateResult_Vector128<long>(AdvSimd_Arm64_CompareEqual_Vector128_Long_AsVariableLoop(Vector128<long>.Zero), -1))
+            result = -1;
+
         // Vector64
 
         if (!ValidateResult_Vector64<float>(AdvSimd_Arm64_CompareEqualScalar_Vector64_Single_Zero(Vector64<float>.Zero), Vector64.CreateScalar(Single.NaN)))