Expose various new Create and conversion APIs for the Vector types (#…

…103462)

* Expose various new Create and conversion APIs for the Vector types

* Remove simdashwintrinsic handling for Vector2/3

* Apply formatting patch

* Ensure creation from a span checks the right element count for Vector2/3

* Ensure that we create more correct WithElement nodes

* Ensure that Vector128.AsVector128Unsafe preserves the fact it produces TYP_SIMD16

src/coreclr/System.Private.CoreLib/src/System/Math.CoreCLR.cs

@@ -92,6 +92,7 @@ public static partial class Math
         [MethodImpl(MethodImplOptions.InternalCall)]
         public static extern double Sin(double a);
 
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public static unsafe (double Sin, double Cos) SinCos(double x)
         {
             double sin, cos;

src/coreclr/System.Private.CoreLib/src/System/MathF.CoreCLR.cs

@@ -89,6 +89,7 @@ public static partial class MathF
         [MethodImpl(MethodImplOptions.InternalCall)]
         public static extern float Sin(float x);
 
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public static unsafe (float Sin, float Cos) SinCos(float x)
         {
             float sin, cos;

src/coreclr/jit/compiler.h

@@ -4562,8 +4562,7 @@ class Compiler
         GenTreeLclVarCommon* data, WCHAR* cns, int len, int dataOffset, StringComparison cmpMode);
     GenTreeStrCon* impGetStrConFromSpan(GenTree* span);
 
-    GenTree* impIntrinsic(GenTree*                newobjThis,
-                          CORINFO_CLASS_HANDLE    clsHnd,
+    GenTree* impIntrinsic(CORINFO_CLASS_HANDLE    clsHnd,
                           CORINFO_METHOD_HANDLE   method,
                           CORINFO_SIG_INFO*       sig,
                           unsigned                methodFlags,
@@ -4628,7 +4627,6 @@ class Compiler
                                   CORINFO_CLASS_HANDLE  clsHnd,
                                   CORINFO_METHOD_HANDLE method,
                                   CORINFO_SIG_INFO*     sig,
-                                  GenTree*              newobjThis,
                                   bool                  mustExpand);
 
 protected:
@@ -4640,7 +4638,6 @@ class Compiler
                                          var_types            retType,
                                          CorInfoType          simdBaseJitType,
                                          unsigned             simdSize,
-                                         GenTree*             newobjThis,
                                          bool                 mustExpand);
 
     GenTree* impSpecialIntrinsic(NamedIntrinsic        intrinsic,
@@ -4653,10 +4650,7 @@ class Compiler
                                  unsigned              simdSize,
                                  bool                  mustExpand);
 
-    GenTree* getArgForHWIntrinsic(var_types            argType,
-                                  CORINFO_CLASS_HANDLE argClass,
-                                  bool                 expectAddr = false,
-                                  GenTree*             newobjThis = nullptr);
+    GenTree* getArgForHWIntrinsic(var_types argType, CORINFO_CLASS_HANDLE argClass);
     GenTree* impNonConstFallback(NamedIntrinsic intrinsic, var_types simdType, CorInfoType simdBaseJitType);
     GenTree* addRangeCheckIfNeeded(
         NamedIntrinsic intrinsic, GenTree* immOp, bool mustExpand, int immLowerBound, int immUpperBound);

src/coreclr/jit/fgbasic.cpp

@@ -1395,15 +1395,10 @@ void Compiler::fgFindJumpTargets(const BYTE* codeAddr, IL_OFFSET codeSize, Fixed
                             case NI_Vector64_CreateScalar:
                             case NI_Vector64_CreateScalarUnsafe:
 #endif // TARGET_ARM64
-                            case NI_Vector2_Create:
-                            case NI_Vector2_CreateBroadcast:
-                            case NI_Vector3_Create:
-                            case NI_Vector3_CreateBroadcast:
-                            case NI_Vector3_CreateFromVector2:
                             case NI_Vector128_Create:
                             case NI_Vector128_CreateScalar:
                             case NI_Vector128_CreateScalarUnsafe:
-                            case NI_VectorT_CreateBroadcast:
+                            case NI_VectorT_Create:
 #if defined(TARGET_XARCH)
                             case NI_BMI1_TrailingZeroCount:
                             case NI_BMI1_X64_TrailingZeroCount:

src/coreclr/jit/gentree.cpp

@@ -27176,6 +27176,7 @@ GenTree* Compiler::gtNewSimdWithElementNode(
 
     assert(varTypeIsArithmetic(simdBaseType));
     assert(op2->IsCnsIntOrI());
+    assert(varTypeIsArithmetic(op3));
 
     ssize_t imm8  = op2->AsIntCon()->IconValue();
     ssize_t count = simdSize / genTypeSize(simdBaseType);

src/coreclr/jit/hwintrinsic.cpp

@@ -745,16 +745,11 @@ bool HWIntrinsicInfo::isImmOp(NamedIntrinsic id, const GenTree* op)
 // Arguments:
 //    argType    -- the required type of argument
 //    argClass   -- the class handle of argType
-//    expectAddr -- if true indicates we are expecting type stack entry to be a TYP_BYREF.
-//    newobjThis -- For CEE_NEWOBJ, this is the temp grabbed for the allocated uninitialized object.
 //
 // Return Value:
 //     the validated argument
 //
-GenTree* Compiler::getArgForHWIntrinsic(var_types            argType,
-                                        CORINFO_CLASS_HANDLE argClass,
-                                        bool                 expectAddr,
-                                        GenTree*             newobjThis)
+GenTree* Compiler::getArgForHWIntrinsic(var_types argType, CORINFO_CLASS_HANDLE argClass)
 {
     GenTree* arg = nullptr;
 
@@ -768,31 +763,12 @@ GenTree* Compiler::getArgForHWIntrinsic(var_types            argType,
         }
         assert(varTypeIsSIMD(argType));
 
-        if (newobjThis == nullptr)
-        {
-            if (expectAddr)
-            {
-                arg = gtNewLoadValueNode(argType, impPopStack().val);
-            }
-            else
-            {
-                arg = impSIMDPopStack();
-            }
-            assert(varTypeIsSIMDOrMask(arg));
-        }
-        else
-        {
-            assert(newobjThis->IsLclVarAddr());
-            arg = newobjThis;
-
-            // push newobj result on type stack
-            unsigned lclNum = arg->AsLclVarCommon()->GetLclNum();
-            impPushOnStack(gtNewLclvNode(lclNum, lvaGetRealType(lclNum)), verMakeTypeInfo(argClass));
-        }
+        arg = impSIMDPopStack();
+        assert(varTypeIsSIMDOrMask(arg));
     }
     else
     {
-        assert(varTypeIsArithmetic(argType) || ((argType == TYP_BYREF) && (newobjThis == nullptr)));
+        assert(varTypeIsArithmetic(argType) || (argType == TYP_BYREF));
 
         arg = impPopStack().val;
         assert(varTypeIsArithmetic(arg->TypeGet()) || ((argType == TYP_BYREF) && arg->TypeIs(TYP_BYREF)));

src/coreclr/jit/hwintrinsicarm64.cpp

@@ -721,6 +721,18 @@ GenTree* Compiler::impSpecialIntrinsic(NamedIntrinsic        intrinsic,
             break;
         }
 
+        case NI_Vector128_AsVector128Unsafe:
+        {
+            assert(sig->numArgs == 1);
+            assert(retType == TYP_SIMD16);
+            assert(simdBaseJitType == CORINFO_TYPE_FLOAT);
+            assert((simdSize == 8) || (simdSize == 12));
+
+            op1     = impSIMDPopStack();
+            retNode = gtNewSimdHWIntrinsicNode(retType, op1, NI_Vector128_AsVector128Unsafe, simdBaseJitType, simdSize);
+            break;
+        }
+
         case NI_Vector64_op_BitwiseAnd:
         case NI_Vector128_op_BitwiseAnd:
         {

src/coreclr/jit/hwintrinsicxarch.cpp

@@ -1279,6 +1279,18 @@ GenTree* Compiler::impSpecialIntrinsic(NamedIntrinsic        intrinsic,
             break;
         }
 
+        case NI_Vector128_AsVector128Unsafe:
+        {
+            assert(sig->numArgs == 1);
+            assert(retType == TYP_SIMD16);
+            assert(simdBaseJitType == CORINFO_TYPE_FLOAT);
+            assert((simdSize == 8) || (simdSize == 12));
+
+            op1     = impSIMDPopStack();
+            retNode = gtNewSimdHWIntrinsicNode(retType, op1, NI_Vector128_AsVector128Unsafe, simdBaseJitType, simdSize);
+            break;
+        }
+
         case NI_Vector256_AsVector:
         case NI_Vector256_AsVector256:
         {

src/coreclr/jit/importercalls.cpp

@@ -242,7 +242,7 @@ var_types Compiler::impImportCall(OPCODE                  opcode,
             }
 #endif // FEATURE_READYTORUN
 
-            call = impIntrinsic(newobjThis, clsHnd, methHnd, sig, mflags, pResolvedToken, isReadonlyCall, isTailCall,
+            call = impIntrinsic(clsHnd, methHnd, sig, mflags, pResolvedToken, isReadonlyCall, isTailCall,
                                 opcode == CEE_CALLVIRT, pConstrainedResolvedToken,
                                 callInfo->thisTransform R2RARG(&entryPoint), &ni, &isSpecialIntrinsic);
 
@@ -2818,7 +2818,6 @@ GenTree* Compiler::impCreateSpanIntrinsic(CORINFO_SIG_INFO* sig)
 // impIntrinsic: possibly expand intrinsic call into alternate IR sequence
 //
 // Arguments:
-//    newobjThis - for constructor calls, the tree for the newly allocated object
 //    clsHnd - handle for the intrinsic method's class
 //    method - handle for the intrinsic method
 //    sig    - signature of the intrinsic method
@@ -2861,8 +2860,7 @@ GenTree* Compiler::impCreateSpanIntrinsic(CORINFO_SIG_INFO* sig)
 //    identified as "must expand" if they are invoked from within their
 //    own method bodies.
 //
-GenTree* Compiler::impIntrinsic(GenTree*                newobjThis,
-                                CORINFO_CLASS_HANDLE    clsHnd,
+GenTree* Compiler::impIntrinsic(CORINFO_CLASS_HANDLE    clsHnd,
                                 CORINFO_METHOD_HANDLE   method,
                                 CORINFO_SIG_INFO*       sig,
                                 unsigned                methodFlags,
@@ -3088,7 +3086,7 @@ GenTree* Compiler::impIntrinsic(GenTree*                newobjThis,
 
             if (isIntrinsic)
             {
-                GenTree* hwintrinsic = impSimdAsHWIntrinsic(ni, clsHnd, method, sig, newobjThis, mustExpand);
+                GenTree* hwintrinsic = impSimdAsHWIntrinsic(ni, clsHnd, method, sig, mustExpand);
 
                 if (hwintrinsic == nullptr)
                 {

src/coreclr/jit/lclmorph.cpp

@@ -1307,9 +1307,10 @@ class LocalAddressVisitor final : public GenTreeVisitor<LocalAddressVisitor>
                 break;
 
 #ifdef FEATURE_HW_INTRINSICS
-                // We have two cases we want to handle:
-                // 1. Vector2/3/4 and Quaternion where we have 4x float fields
+                // We have three cases we want to handle:
+                // 1. Vector2/3/4 and Quaternion where we have 2-4x float fields
                 // 2. Plane where we have 1x Vector3 and 1x float field
+                // 3. Accesses of halves of larger SIMD types
 
             case IndirTransform::GetElement:
             {
@@ -1321,24 +1322,29 @@ class LocalAddressVisitor final : public GenTreeVisitor<LocalAddressVisitor>
                 {
                     case TYP_FLOAT:
                     {
+                        // Handle case 1 or the float field of case 2
                         GenTree* indexNode = m_compiler->gtNewIconNode(offset / genTypeSize(elementType));
                         hwiNode            = m_compiler->gtNewSimdGetElementNode(elementType, lclNode, indexNode,
                                                                                  CORINFO_TYPE_FLOAT, genTypeSize(varDsc));
                         break;
                     }
+
                     case TYP_SIMD12:
                     {
+                        // Handle the Vector3 field of case 2
                         assert(genTypeSize(varDsc) == 16);
                         hwiNode = m_compiler->gtNewSimdHWIntrinsicNode(elementType, lclNode, NI_Vector128_AsVector3,
                                                                        CORINFO_TYPE_FLOAT, 16);
                         break;
                     }
+
                     case TYP_SIMD8:
 #if defined(FEATURE_SIMD) && defined(TARGET_XARCH)
                     case TYP_SIMD16:
                     case TYP_SIMD32:
 #endif
                     {
+                        // Handle case 3
                         assert(genTypeSize(elementType) * 2 == genTypeSize(varDsc));
                         if (offset == 0)
                         {
@@ -1374,29 +1380,44 @@ class LocalAddressVisitor final : public GenTreeVisitor<LocalAddressVisitor>
                 {
                     case TYP_FLOAT:
                     {
+                        // Handle case 1 or the float field of case 2
                         GenTree* indexNode = m_compiler->gtNewIconNode(offset / genTypeSize(elementType));
                         hwiNode =
                             m_compiler->gtNewSimdWithElementNode(varDsc->TypeGet(), simdLclNode, indexNode, elementNode,
                                                                  CORINFO_TYPE_FLOAT, genTypeSize(varDsc));
                         break;
                     }
+
                     case TYP_SIMD12:
                     {
+                        // Handle the Vector3 field of case 2
                         assert(varDsc->TypeGet() == TYP_SIMD16);
 
-                        // We inverse the operands here and take elementNode as the main value and simdLclNode[3] as the
-                        // new value. This gives us a new TYP_SIMD16 with all elements in the right spots
-                        GenTree* indexNode = m_compiler->gtNewIconNode(3, TYP_INT);
-                        hwiNode = m_compiler->gtNewSimdWithElementNode(TYP_SIMD16, elementNode, indexNode, simdLclNode,
+                        // We effectively inverse the operands here and take elementNode as the main value and
+                        // simdLclNode[3] as the new value. This gives us a new TYP_SIMD16 with all elements in the
+                        // right spots
+
+                        elementNode = m_compiler->gtNewSimdHWIntrinsicNode(TYP_SIMD16, elementNode,
+                                                                           NI_Vector128_AsVector128Unsafe,
+                                                                           CORINFO_TYPE_FLOAT, 12);
+
+                        GenTree* indexNode1 = m_compiler->gtNewIconNode(3, TYP_INT);
+                        simdLclNode         = m_compiler->gtNewSimdGetElementNode(TYP_FLOAT, simdLclNode, indexNode1,
+                                                                                  CORINFO_TYPE_FLOAT, 16);
+
+                        GenTree* indexNode2 = m_compiler->gtNewIconNode(3, TYP_INT);
+                        hwiNode = m_compiler->gtNewSimdWithElementNode(TYP_SIMD16, elementNode, indexNode2, simdLclNode,
                                                                        CORINFO_TYPE_FLOAT, 16);
                         break;
                     }
+
                     case TYP_SIMD8:
 #if defined(FEATURE_SIMD) && defined(TARGET_XARCH)
                     case TYP_SIMD16:
                     case TYP_SIMD32:
 #endif
                     {
+                        // Handle case 3
                         assert(genTypeSize(elementType) * 2 == genTypeSize(varDsc));
                         if (offset == 0)
                         {
@@ -1412,6 +1433,7 @@ class LocalAddressVisitor final : public GenTreeVisitor<LocalAddressVisitor>
 
                         break;
                     }
+
                     default:
                         unreached();
                 }
@@ -1541,7 +1563,7 @@ class LocalAddressVisitor final : public GenTreeVisitor<LocalAddressVisitor>
             if (varTypeIsSIMD(varDsc))
             {
                 // We have three cases we want to handle:
-                // 1. Vector2/3/4 and Quaternion where we have 4x float fields
+                // 1. Vector2/3/4 and Quaternion where we have 2-4x float fields
                 // 2. Plane where we have 1x Vector3 and 1x float field
                 // 3. Accesses of halves of larger SIMD types
 

src/coreclr/jit/lower.cpp

@@ -1752,18 +1752,23 @@ void Lowering::LowerArg(GenTreeCall* call, CallArg* callArg, bool late)
         }
         else if (arg->OperIs(GT_HWINTRINSIC))
         {
-            GenTreeJitIntrinsic* jitIntrinsic = reinterpret_cast<GenTreeJitIntrinsic*>(arg);
+            GenTreeHWIntrinsic* hwintrinsic = arg->AsHWIntrinsic();
 
             // For HWIntrinsic, there are some intrinsics like ExtractVector128 which have
-            // a gtType of TYP_SIMD16 but a SimdSize of 32, so we need to include that in
-            // the assert below.
+            // a gtType of TYP_SIMD16 but a SimdSize of 32, so we can't necessarily assert
+            // the simd size
 
-            assert((jitIntrinsic->GetSimdSize() == 12) || (jitIntrinsic->GetSimdSize() == 16) ||
-                   (jitIntrinsic->GetSimdSize() == 32) || (jitIntrinsic->GetSimdSize() == 64));
-
-            if (jitIntrinsic->GetSimdSize() == 12)
+            if (hwintrinsic->GetSimdSize() == 12)
             {
-                type = TYP_SIMD12;
+                if (hwintrinsic->GetHWIntrinsicId() != NI_Vector128_AsVector128Unsafe)
+                {
+                    // Most nodes that have a simdSize of 12 are actually producing a TYP_SIMD12
+                    // and have been massaged to TYP_SIMD16 to match the actual product size. This
+                    // is not the case for NI_Vector128_AsVector128Unsafe which is explicitly taking
+                    // a TYP_SIMD12 and producing a TYP_SIMD16.
+
+                    type = TYP_SIMD12;
+                }
             }
         }
     }