Fix SIMD Scalar Move Encoding: VEX.L should be 0
authorLi Tian <litian2025@gmail.com>
Mon, 28 Nov 2016 22:48:01 +0000 (14:48 -0800)
committerLi Tian <litian2025@gmail.com>
Wed, 14 Dec 2016 23:22:25 +0000 (15:22 -0800)
For SIMD Scalar Move instructions such as vmovlpd, vmovlps, vmovhps,
vmovhps and vmovss on AVX system, JIT should ensure that those
instructions are encoded with VEX.L=0, because encoding them with
VEX.L=1 may encounter unpredictable behavior across different
processor generations. The reason of VEX.L is encoded with 1 is
because JIT calls compiler->getSIMDVectorType() which returns
EA_32BYTE, and it is been passed into emitter AddVexPrefix() which
ends up encoded VEX.L=1, the fix is to pass target type and base
type for those instructions to ensure that VEX.L=0 at emitter
AddVexPrefix()

Fix #8328

src/jit/codegenlinear.h
src/jit/simdcodegenxarch.cpp

index 25ad3f5..fd6a59e 100644 (file)
@@ -68,7 +68,8 @@ enum SIMDScalarMoveType
 };
 
 instruction getOpForSIMDIntrinsic(SIMDIntrinsicID intrinsicId, var_types baseType, unsigned* ival = nullptr);
-void genSIMDScalarMove(var_types type, regNumber target, regNumber src, SIMDScalarMoveType moveType);
+void genSIMDScalarMove(
+    var_types targetType, var_types type, regNumber target, regNumber src, SIMDScalarMoveType moveType);
 void genSIMDZero(var_types targetType, var_types baseType, regNumber targetReg);
 void genSIMDIntrinsicInit(GenTreeSIMD* simdNode);
 void genSIMDIntrinsicInitN(GenTreeSIMD* simdNode);
index 20db803..96d2048 100644 (file)
@@ -464,7 +464,8 @@ instruction CodeGen::getOpForSIMDIntrinsic(SIMDIntrinsicID intrinsicId, var_type
 // to target mm reg, zeroing out the upper bits if and only if specified.
 //
 // Arguments:
-//    type             the type of value to be moved
+//    targetType       the target type
+//    baseType         the base type of value to be moved
 //    targetReg        the target reg
 //    srcReg           the src reg
 //    moveType         action to be performed on target upper bits
@@ -475,10 +476,10 @@ instruction CodeGen::getOpForSIMDIntrinsic(SIMDIntrinsicID intrinsicId, var_type
 // Notes:
 //    This is currently only supported for floating point types.
 //
-void CodeGen::genSIMDScalarMove(var_types type, regNumber targetReg, regNumber srcReg, SIMDScalarMoveType moveType)
+void CodeGen::genSIMDScalarMove(
+    var_types targetType, var_types baseType, regNumber targetReg, regNumber srcReg, SIMDScalarMoveType moveType)
 {
-    var_types targetType = compiler->getSIMDVectorType();
-    assert(varTypeIsFloating(type));
+    assert(varTypeIsFloating(baseType));
 #ifdef FEATURE_AVX_SUPPORT
     if (compiler->getSIMDInstructionSet() == InstructionSet_AVX)
     {
@@ -487,17 +488,17 @@ void CodeGen::genSIMDScalarMove(var_types type, regNumber targetReg, regNumber s
             case SMT_PreserveUpper:
                 if (srcReg != targetReg)
                 {
-                    instruction ins = ins_Store(type);
+                    instruction ins = ins_Store(baseType);
                     if (getEmitter()->IsThreeOperandMoveAVXInstruction(ins))
                     {
                         // In general, when we use a three-operands move instruction, we want to merge the src with
                         // itself. This is an exception in that we actually want the "merge" behavior, so we must
                         // specify it with all 3 operands.
-                        inst_RV_RV_RV(ins, targetReg, targetReg, srcReg, emitTypeSize(targetType));
+                        inst_RV_RV_RV(ins, targetReg, targetReg, srcReg, emitTypeSize(baseType));
                     }
                     else
                     {
-                        inst_RV_RV(ins, targetReg, srcReg, targetType, emitTypeSize(targetType));
+                        inst_RV_RV(ins, targetReg, srcReg, baseType, emitTypeSize(baseType));
                     }
                 }
                 break;
@@ -516,9 +517,9 @@ void CodeGen::genSIMDScalarMove(var_types type, regNumber targetReg, regNumber s
             case SMT_ZeroInitUpper_SrcHasUpperZeros:
                 if (srcReg != targetReg)
                 {
-                    instruction ins = ins_Copy(type);
+                    instruction ins = ins_Copy(baseType);
                     assert(!getEmitter()->IsThreeOperandMoveAVXInstruction(ins));
-                    inst_RV_RV(ins, targetReg, srcReg, targetType, emitTypeSize(targetType));
+                    inst_RV_RV(ins, targetReg, srcReg, baseType, emitTypeSize(baseType));
                 }
                 break;
 
@@ -536,7 +537,7 @@ void CodeGen::genSIMDScalarMove(var_types type, regNumber targetReg, regNumber s
             case SMT_PreserveUpper:
                 if (srcReg != targetReg)
                 {
-                    inst_RV_RV(ins_Store(type), targetReg, srcReg, targetType, emitTypeSize(targetType));
+                    inst_RV_RV(ins_Store(baseType), targetReg, srcReg, baseType, emitTypeSize(baseType));
                 }
                 break;
 
@@ -545,22 +546,22 @@ void CodeGen::genSIMDScalarMove(var_types type, regNumber targetReg, regNumber s
                 {
                     // There is no guarantee that upper bits of op1Reg are zero.
                     // We achieve this by using left logical shift 12-bytes and right logical shift 12 bytes.
-                    instruction ins = getOpForSIMDIntrinsic(SIMDIntrinsicShiftLeftInternal, type);
+                    instruction ins = getOpForSIMDIntrinsic(SIMDIntrinsicShiftLeftInternal, baseType);
                     getEmitter()->emitIns_R_I(ins, EA_16BYTE, srcReg, 12);
-                    ins = getOpForSIMDIntrinsic(SIMDIntrinsicShiftRightInternal, type);
+                    ins = getOpForSIMDIntrinsic(SIMDIntrinsicShiftRightInternal, baseType);
                     getEmitter()->emitIns_R_I(ins, EA_16BYTE, srcReg, 12);
                 }
                 else
                 {
                     genSIMDZero(targetType, TYP_FLOAT, targetReg);
-                    inst_RV_RV(ins_Store(type), targetReg, srcReg);
+                    inst_RV_RV(ins_Store(baseType), targetReg, srcReg);
                 }
                 break;
 
             case SMT_ZeroInitUpper_SrcHasUpperZeros:
                 if (srcReg != targetReg)
                 {
-                    inst_RV_RV(ins_Copy(type), targetReg, srcReg, targetType, emitTypeSize(targetType));
+                    inst_RV_RV(ins_Copy(baseType), targetReg, srcReg, baseType, emitTypeSize(baseType));
                 }
                 break;
 
@@ -676,7 +677,7 @@ void CodeGen::genSIMDIntrinsicInit(GenTreeSIMD* simdNode)
             SIMDScalarMoveType moveType =
                 op1->IsCnsFltOrDbl() || op1->isMemoryOp() ? SMT_ZeroInitUpper_SrcHasUpperZeros : SMT_ZeroInitUpper;
 
-            genSIMDScalarMove(TYP_FLOAT, targetReg, op1Reg, moveType);
+            genSIMDScalarMove(targetType, TYP_FLOAT, targetReg, op1Reg, moveType);
 
             if (size == 8)
             {
@@ -786,7 +787,7 @@ void CodeGen::genSIMDIntrinsicInitN(GenTreeSIMD* simdNode)
         {
             getEmitter()->emitIns_R_I(insLeftShift, EA_16BYTE, vectorReg, baseTypeSize);
         }
-        genSIMDScalarMove(baseType, vectorReg, operandReg, SMT_PreserveUpper);
+        genSIMDScalarMove(targetType, baseType, vectorReg, operandReg, SMT_PreserveUpper);
 
         offset += baseTypeSize;
     }