Do not retype SIMD nodes (#70265)

* Do not retype SIMD nodes

It is not necessary: the only case where it is required, SIMD8 to
LONG bitcasts on Windows x64, is already handled by lowering.

It is dangerous: in case we CSE the retyped tree, its other uses
will be (effectively) retyped as well.

* Add a test

src/coreclr/jit/codegenarm64.cpp

@@ -2324,12 +2324,8 @@ void CodeGen::genSetRegToConst(regNumber targetReg, var_types targetType, GenTre
             switch (tree->TypeGet())
             {
 #if defined(FEATURE_SIMD)
-                case TYP_LONG:
-                case TYP_DOUBLE:
                 case TYP_SIMD8:
                 {
-                    // TODO-1stClassStructs: do not retype SIMD nodes
-
                     if (vecCon->IsAllBitsSet())
                     {
                         emit->emitIns_R_I(INS_mvni, attr, targetReg, 0, INS_OPTS_2S);

src/coreclr/jit/codegenxarch.cpp

@@ -537,12 +537,8 @@ void CodeGen::genSetRegToConst(regNumber targetReg, var_types targetType, GenTre
             switch (tree->TypeGet())
             {
 #if defined(FEATURE_SIMD)
-                case TYP_LONG:
-                case TYP_DOUBLE:
                 case TYP_SIMD8:
                 {
-                    // TODO-1stClassStructs: do not retype SIMD nodes
-
                     simd8_t              constValue = vecCon->gtSimd8Val;
                     CORINFO_FIELD_HANDLE hnd        = emit->emitSimd8Const(constValue);
 

src/coreclr/jit/gentree.cpp

@@ -2856,10 +2856,7 @@ unsigned Compiler::gtHashValue(GenTree* tree)
                     }
 
                     case TYP_SIMD8:
-                    case TYP_DOUBLE:
-                    case TYP_LONG:
                     {
-                        // TODO-1stClassStructs: do not retype SIMD nodes
                         add = genTreeHashAdd(ulo32(add), vecCon->gtSimd8Val.u32[1]);
                         add = genTreeHashAdd(ulo32(add), vecCon->gtSimd8Val.u32[0]);
                         break;
@@ -2873,7 +2870,6 @@ unsigned Compiler::gtHashValue(GenTree* tree)
                 }
 
                 add = genTreeHashAdd(ulo32(add), vecCon->GetSimdBaseType());
-                add = genTreeHashAdd(ulo32(add), vecCon->GetSimdSize());
                 break;
             }
 
@@ -6949,7 +6945,7 @@ GenTree* Compiler::gtNewSconNode(int CPX, CORINFO_MODULE_HANDLE scpHandle)
 
 GenTreeVecCon* Compiler::gtNewVconNode(var_types type, CorInfoType simdBaseJitType)
 {
-    GenTreeVecCon* vecCon = new (this, GT_CNS_VEC) GenTreeVecCon(type, simdBaseJitType, genTypeSize(type));
+    GenTreeVecCon* vecCon = new (this, GT_CNS_VEC) GenTreeVecCon(type, simdBaseJitType);
     return vecCon;
 }
 
@@ -11152,11 +11148,8 @@ void Compiler::gtDispConst(GenTree* tree)
             switch (vecCon->TypeGet())
             {
 #if defined(FEATURE_SIMD)
-                case TYP_LONG:
-                case TYP_DOUBLE:
                 case TYP_SIMD8:
                 {
-                    // TODO-1stClassStructs: do not retype SIMD nodes
                     simd8_t simdVal = vecCon->gtSimd8Val;
                     printf("<0x%08x, 0x%08x>", simdVal.u32[0], simdVal.u32[1]);
                     break;

src/coreclr/jit/gentree.h

@@ -3270,7 +3270,6 @@ struct GenTreeVecCon : public GenTree
     // size should be `gtType` and the handle should be looked up at callsites where required
 
     unsigned char gtSimdBaseJitType; // SIMD vector base JIT type
-    unsigned char gtSimdSize;        // SIMD vector size in bytes
 
 public:
     CorInfoType GetSimdBaseJitType() const
@@ -3286,17 +3285,6 @@ struct GenTreeVecCon : public GenTree
 
     var_types GetSimdBaseType() const;
 
-    unsigned char GetSimdSize() const
-    {
-        return gtSimdSize;
-    }
-
-    void SetSimdSize(unsigned simdSize)
-    {
-        gtSimdSize = (unsigned char)simdSize;
-        assert(gtSimdSize == simdSize);
-    }
-
 #if defined(FEATURE_HW_INTRINSICS)
     static bool IsHWIntrinsicCreateConstant(GenTreeHWIntrinsic* node, simd32_t& simd32Val);
 
@@ -3308,11 +3296,8 @@ struct GenTreeVecCon : public GenTree
         switch (gtType)
         {
 #if defined(FEATURE_SIMD)
-            case TYP_LONG:
-            case TYP_DOUBLE:
             case TYP_SIMD8:
             {
-                // TODO-1stClassStructs: do not retype SIMD nodes
                 return (gtSimd8Val.u64[0] == 0xFFFFFFFFFFFFFFFF);
             }
 
@@ -3353,11 +3338,8 @@ struct GenTreeVecCon : public GenTree
         switch (gtType)
         {
 #if defined(FEATURE_SIMD)
-            case TYP_LONG:
-            case TYP_DOUBLE:
             case TYP_SIMD8:
             {
-                // TODO-1stClassStructs: do not retype SIMD nodes
                 return (left->gtSimd8Val.u64[0] == right->gtSimd8Val.u64[0]);
             }
 
@@ -3395,11 +3377,8 @@ struct GenTreeVecCon : public GenTree
         switch (gtType)
         {
 #if defined(FEATURE_SIMD)
-            case TYP_LONG:
-            case TYP_DOUBLE:
             case TYP_SIMD8:
             {
-                // TODO-1stClassStructs: do not retype SIMD nodes
                 return (gtSimd8Val.u64[0] == 0x0000000000000000);
             }
 
@@ -3428,14 +3407,11 @@ struct GenTreeVecCon : public GenTree
         }
     }
 
-    GenTreeVecCon(var_types type, CorInfoType simdBaseJitType, unsigned simdSize)
-        : GenTree(GT_CNS_VEC, type)
-        , gtSimdBaseJitType((unsigned char)simdBaseJitType)
-        , gtSimdSize((unsigned char)simdSize)
+    GenTreeVecCon(var_types type, CorInfoType simdBaseJitType)
+        : GenTree(GT_CNS_VEC, type), gtSimdBaseJitType((unsigned char)simdBaseJitType)
     {
         assert(varTypeIsSIMD(type));
         assert(gtSimdBaseJitType == simdBaseJitType);
-        assert(gtSimdSize == simdSize);
     }
 
 #if DEBUGGABLE_GENTREE

src/coreclr/jit/instr.cpp

@@ -843,11 +843,8 @@ CodeGen::OperandDesc CodeGen::genOperandDesc(GenTree* op)
                 switch (op->TypeGet())
                 {
 #if defined(FEATURE_SIMD)
-                    case TYP_LONG:
-                    case TYP_DOUBLE:
                     case TYP_SIMD8:
                     {
-                        // TODO-1stClassStructs: do not retype SIMD nodes
                         simd8_t constValue = op->AsVecCon()->gtSimd8Val;
                         return OperandDesc(emit->emitSimd8Const(constValue));
                     }

src/coreclr/jit/morph.cpp

@@ -3404,11 +3404,24 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call)
                             argObj->gtType = structBaseType;
                         }
                     }
-                    else
+                    else if (argObj->OperIs(GT_LCL_FLD, GT_IND))
                     {
-                        // Not a GT_LCL_VAR, so we can just change the type on the node
+                        // We can just change the type on the node
                         argObj->gtType = structBaseType;
                     }
+                    else
+                    {
+#ifdef FEATURE_SIMD
+                        // We leave the SIMD8 <-> LONG (Windows x64) case to lowering. For SIMD8 <-> DOUBLE (Unix x64),
+                        // we do not need to do anything as both types already use floating-point registers.
+                        assert((argObj->TypeIs(TYP_SIMD8) &&
+                                ((structBaseType == TYP_LONG) || (structBaseType == TYP_DOUBLE))) ||
+                               argObj->TypeIs(structBaseType));
+#else  // !FEATURE_SIMD
+                        unreached();
+#endif // !FEATURE_SIMD
+                    }
+
                     assert(varTypeIsEnregisterable(argObj->TypeGet()) ||
                            (makeOutArgCopy && varTypeIsEnregisterable(structBaseType)));
                 }
@@ -9922,7 +9935,7 @@ GenTree* Compiler::getSIMDStructFromField(GenTree*     tree,
             {
                 ret                   = obj;
                 GenTreeVecCon* vecCon = obj->AsVecCon();
-                *simdSizeOut          = vecCon->GetSimdSize();
+                *simdSizeOut          = genTypeSize(vecCon);
                 *simdBaseJitTypeOut   = vecCon->GetSimdBaseJitType();
             }
         }

src/coreclr/jit/rationalize.cpp

@@ -670,16 +670,6 @@ Compiler::fgWalkResult Rationalizer::RewriteNode(GenTree** useEdge, Compiler::Ge
             unsigned     simdSize = simdNode->GetSimdSize();
             var_types    simdType = comp->getSIMDTypeForSize(simdSize);
 
-            // TODO-1stClassStructs: This should be handled more generally for enregistered or promoted
-            // structs that are passed or returned in a different register type than their enregistered
-            // type(s).
-            if (simdNode->gtType == TYP_I_IMPL && simdNode->GetSimdSize() == TARGET_POINTER_SIZE)
-            {
-                // This happens when it is consumed by a GT_RET_EXPR.
-                // It can only be a Vector2f or Vector2i.
-                assert(genTypeSize(simdNode->GetSimdBaseType()) == 4);
-                simdNode->gtType = TYP_SIMD8;
-            }
             // Certain SIMD trees require rationalizing.
             if (simdNode->AsSIMD()->GetSIMDIntrinsicId() == SIMDIntrinsicInitArray)
             {
@@ -704,54 +694,6 @@ Compiler::fgWalkResult Rationalizer::RewriteNode(GenTree** useEdge, Compiler::Ge
         break;
 #endif // FEATURE_SIMD
 
-#ifdef FEATURE_HW_INTRINSICS
-        case GT_HWINTRINSIC:
-        {
-            GenTreeHWIntrinsic* hwIntrinsicNode = node->AsHWIntrinsic();
-
-            if (!hwIntrinsicNode->isSIMD())
-            {
-                break;
-            }
-
-            // TODO-1stClassStructs: This should be handled more generally for enregistered or promoted
-            // structs that are passed or returned in a different register type than their enregistered
-            // type(s).
-            if ((hwIntrinsicNode->gtType == TYP_I_IMPL) && (hwIntrinsicNode->GetSimdSize() == TARGET_POINTER_SIZE))
-            {
-#ifdef TARGET_ARM64
-                // Special case for GetElement/ToScalar because they take Vector64<T> and return T
-                // and T can be long or ulong.
-                if (!((hwIntrinsicNode->GetHWIntrinsicId() == NI_Vector64_GetElement) ||
-                      (hwIntrinsicNode->GetHWIntrinsicId() == NI_Vector64_ToScalar)))
-#endif
-                {
-                    // This happens when it is consumed by a GT_RET_EXPR.
-                    // It can only be a Vector2f or Vector2i.
-                    assert(genTypeSize(hwIntrinsicNode->GetSimdBaseType()) == 4);
-                    hwIntrinsicNode->gtType = TYP_SIMD8;
-                }
-            }
-            break;
-        }
-#endif // FEATURE_HW_INTRINSICS
-
-#if defined(FEATURE_SIMD)
-        case GT_CNS_VEC:
-        {
-            GenTreeVecCon* vecCon = node->AsVecCon();
-
-            // TODO-1stClassStructs: do not retype SIMD nodes
-
-            if ((vecCon->TypeIs(TYP_I_IMPL)) && (vecCon->GetSimdSize() == TARGET_POINTER_SIZE))
-            {
-                assert(genTypeSize(vecCon->GetSimdBaseType()) == 4);
-                vecCon->gtType = TYP_SIMD8;
-            }
-            break;
-        }
-#endif // FEATURE_SIMD
-
         default:
             // Check that we don't have nodes not allowed in HIR here.
             assert((node->DebugOperKind() & DBK_NOTHIR) == 0);

src/coreclr/jit/valuenum.cpp

@@ -7959,16 +7959,6 @@ void Compiler::fgValueNumberTreeConst(GenTree* tree)
                 tree->gtVNPair.SetBoth(
                     vnStore->VNForHandle(ssize_t(tree->AsIntConCommon()->IconValue()), tree->GetIconHandleFlag()));
             }
-#ifdef FEATURE_SIMD
-            else if (tree->IsCnsVec())
-            {
-                // TODO-1stClassStructs: do not retype SIMD nodes
-                assert(varTypeIsLong(typ));
-
-                simd8_t simd8Val = tree->AsVecCon()->gtSimd8Val;
-                tree->gtVNPair.SetBoth(vnStore->VNForSimd8Con(simd8Val));
-            }
-#endif // FEATURE_SIMD
             else if ((typ == TYP_LONG) || (typ == TYP_ULONG))
             {
                 tree->gtVNPair.SetBoth(vnStore->VNForLongCon(INT64(tree->AsIntConCommon()->LngValue())));
@@ -8061,18 +8051,7 @@ void Compiler::fgValueNumberTreeConst(GenTree* tree)
 
         case TYP_DOUBLE:
         {
-#ifdef FEATURE_SIMD
-            if (tree->IsCnsVec())
-            {
-                // TODO-1stClassStructs: do not retype SIMD nodes
-                simd8_t simd8Val = tree->AsVecCon()->gtSimd8Val;
-                tree->gtVNPair.SetBoth(vnStore->VNForSimd8Con(simd8Val));
-            }
-            else
-#endif // FEATURE_SIMD
-            {
-                tree->gtVNPair.SetBoth(vnStore->VNForDoubleCon(tree->AsDblCon()->gtDconVal));
-            }
+            tree->gtVNPair.SetBoth(vnStore->VNForDoubleCon(tree->AsDblCon()->gtDconVal));
             break;
         }
 

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

@@ -0,0 +1,24 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Numerics;
+using System.Runtime.CompilerServices;
+
+public class Runtime_70124
+{
+    public static int Main()
+    {
+        return Problem(Vector2.One, Vector2.One) != new Vector2(3, 3) ? 101 : 100;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    private static Vector2 Problem(Vector2 a, Vector2 b)
+    {
+        CallForVtor2(a + b);
+
+        return (a + b) + a;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    private static void CallForVtor2(Vector2 vtor) { }
+}

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

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