Fix to second CoreFX SIMD test failure mentioned in issue dotnet/coreclr#2886.

Root Cause:
The following managed method is executed by the test

  Vector3 Vecor3.Normalize(Vector3)

Vector3 on Unix gets passed in two registers by caller of Normalize()
method.  Within prolog of Normalie(), RyuJIt homes Vector3 arg by
writing only 12 bytes of arg regs xmm0/xmm1. As a result the upper
4-bytes could end up garbage.  Further down test performs dot product
and the codegen of which makes the assumption that Vector3 types
when loaded in regs will have upper 4-bytes zero'ed out.  Since
that assumption is violated, incorrect results gets computed and
hence CoreFx test failure not fiding expected result.

Fix:  While homing Vector3 arg treat it as TYP_SIMD16 and home
16-bytes of arg regs xmm0/xmm1.

Also added a TODO comment on the assumptions RyuJIT is making on
Vector3 type.  Will be opening a Git issue for the same.

Commit migrated from https://github.com/dotnet/coreclr/commit/c12afaa189db63f67720c64e792e9846a58d7604

diff --git a/src/coreclr/src/jit/codegencommon.cpp b/src/coreclr/src/jit/codegencommon.cpp
index de2339e..b8eee4d 100644 (file)
--- a/src/coreclr/src/jit/codegencommon.cpp
+++ b/src/coreclr/src/jit/codegencommon.cpp
@@ -4012,8 +4012,31 @@ void            CodeGen::genFnPrologCalleeRegArgs(regNumber xtraReg,
             for (unsigned slotCounter = 0; slotCounter < structDesc.eightByteCount; slotCounter++)
             {
                 regNumber regNum = varDsc->lvRegNumForSlot(slotCounter);
+                var_types regType;
 
-                var_types regType = compiler->getEightByteType(structDesc, slotCounter);
+#ifdef FEATURE_SIMD
+                // Assumption 1:
+                // RyuJit backend depends on the assumption that on 64-Bit targets Vector3 size is rounded off
+                // to TARGET_POINTER_SIZE and hence Vector3 locals on stack can be treated as TYP_SIMD16 for
+                // reading and writing purposes.  Hence while homing a Vector3 type arg on stack we should
+                // home entire 16-bytes so that the upper-most 4-bytes will be zeroed when written to stack.
+                //
+                // Assumption 2:
+                // RyuJit backend is making another implicit assumption that Vector3 type args when passed in
+                // registers or on stack, the upper most 4-bytes will be zero.  
+                //
+                // TODO-64bit: assumptions 1 and 2 hold within RyuJIT generated code. It is not clear whether
+                // these assumptions hold when a Vector3 type arg is passed by native code. Example: PInvoke
+                // returning Vector3 type value or RPInvoke passing Vector3 type args.
+                if (varDsc->lvType == TYP_SIMD12)
+                {
+                    regType = TYP_DOUBLE;
+                }
+                else
+#endif
+                {
+                    regType = compiler->getEightByteType(structDesc, slotCounter);
+                }
                 
                 regArgNum = genMapRegNumToRegArgNum(regNum, regType);
                 

diff --git a/src/coreclr/src/jit/lower.cpp b/src/coreclr/src/jit/lower.cpp
index 024160a..aaeb026 100644 (file)
--- a/src/coreclr/src/jit/lower.cpp
+++ b/src/coreclr/src/jit/lower.cpp
@@ -601,9 +601,18 @@ void Lowering::LowerNode(GenTreePtr* ppTree, Compiler::fgWalkData* data)
         if ((*ppTree)->TypeGet() == TYP_SIMD12)
         {
 #ifdef _TARGET_64BIT_
-            // On 64-bit architectures, size of Vector3 local on stack will be
-            // rounded to TARGET_POINTER_SIZE and hence will be 16 bytes. Therefore,
-            // Vector3 locals on stack could be read/written as if they were TYP_SIMD16
+            // Assumption 1:
+            // RyuJit backend depends on the assumption that on 64-Bit targets Vector3 size is rounded off
+            // to TARGET_POINTER_SIZE and hence Vector3 locals on stack can be treated as TYP_SIMD16 for
+            // reading and writing purposes. 
+            //
+            // Assumption 2:
+            // RyuJit backend is making another implicit assumption that Vector3 type args when passed in
+            // registers or on stack, the upper most 4-bytes will be zero.  
+            //
+            // TODO-64bit: assumptions 1 and 2 hold within RyuJIT generated code. It is not clear whether
+            // these assumptions hold when a Vector3 type arg is passed by native code. Example: PInvoke
+            // returning Vector3 type value or RPInvoke passing Vector3 type args.
             (*ppTree)->gtType = TYP_SIMD16;
 #else
             NYI("Lowering of TYP_SIMD12 locals");