Ensure that math calls into the CRT are tracked as needing vzeroupper (#112011)

tannergooding · web-flow · commit 6f221b41da8b · 2025-02-01T11:06:23.000-08:00
* Ensure that math calls into the CRT are tracked as needing vzeroupper

* Apply suggestions from code review

* Don't force vzeroupper for helpers written in managed
diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp
@@ -10156,7 +10156,7 @@ JITDBGAPI void __cdecl cTreeFlags(Compiler* comp, GenTree* tree)
                     {
                         chars += printf("[CALL_M_NOGCCHECK]");
                     }
-                    if (call->gtCallMoreFlags & GTF_CALL_M_SPECIAL_INTRINSIC)
+                    if (call->IsSpecialIntrinsic())
                     {
                         chars += printf("[CALL_M_SPECIAL_INTRINSIC]");
                     }
diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h
@@ -4765,7 +4765,8 @@ class Compiler
                               R2RARG(CORINFO_CONST_LOOKUP* entryPoint),
                               var_types             callType,
                               NamedIntrinsic        intrinsicName,
-                              bool                  tailCall);
+                              bool                  tailCall,
+                              bool*                 isSpecial);
     GenTree* impMinMaxIntrinsic(CORINFO_METHOD_HANDLE method,
                                 CORINFO_SIG_INFO*     sig,
                                 CorInfoType           callJitType,
diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp
@@ -2005,66 +2005,74 @@ bool GenTreeCall::NeedsVzeroupper(Compiler* comp)
     }
 
     bool needsVzeroupper = false;
+    bool checkSignature  = false;
 
-    if (IsPInvoke())
-    {
-        // The Intel optimization manual guidance in `3.11.5.3 Fixing Instruction Slowdowns` states:
-        //   Insert a VZEROUPPER to tell the hardware that the state of the higher registers is clean
-        //   between the VEX and the legacy SSE instructions. Often the best way to do this is to insert a
-        //   VZEROUPPER before returning from any function that uses VEX (that does not produce a VEX
-        //   register) and before any call to an unknown function.
+    // The Intel optimization manual guidance in `3.11.5.3 Fixing Instruction Slowdowns` states:
+    //   Insert a VZEROUPPER to tell the hardware that the state of the higher registers is clean
+    //   between the VEX and the legacy SSE instructions. Often the best way to do this is to insert a
+    //   VZEROUPPER before returning from any function that uses VEX (that does not produce a VEX
+    //   register) and before any call to an unknown function.
 
-        switch (gtCallType)
+    switch (gtCallType)
+    {
+        case CT_USER_FUNC:
+        case CT_INDIRECT:
         {
-            case CT_USER_FUNC:
-            case CT_INDIRECT:
-            {
-                // Since P/Invokes are not compiled by the runtime, they are typically "unknown" since they
-                // may use the legacy encoding. This includes both CT_USER_FUNC and CT_INDIRECT
+            // Since P/Invokes are not compiled by the runtime, they are typically "unknown" since they
+            // may use the legacy encoding. This includes both CT_USER_FUNC and CT_INDIRECT
 
+            if (IsPInvoke())
+            {
                 needsVzeroupper = true;
-                break;
             }
-
-            case CT_HELPER:
+            else if (IsSpecialIntrinsic())
             {
-                // Most helpers are well known to not use any floating-point or SIMD logic internally, but
-                // a few do exist so we need to ensure they are handled. They are identified by taking or
-                // returning a floating-point or SIMD type, regardless of how it is actually passed/returned.
+                checkSignature = true;
+            }
+            break;
+        }
 
-                if (varTypeUsesFloatReg(this))
+        case CT_HELPER:
+        {
+            // A few special cases exist that can't be found by signature alone, so we handle
+            // those explicitly here instead.
+            needsVzeroupper = IsHelperCall(comp, CORINFO_HELP_BULK_WRITEBARRIER);
+
+            // Most other helpers are well known to not use any floating-point or SIMD logic internally, but
+            // a few do exist so we need to ensure they are handled. They are identified by taking or
+            // returning a floating-point or SIMD type, regardless of how it is actually passed/returned but
+            // are excluded if we know they are implemented in managed.
+            checkSignature = !needsVzeroupper && !IsHelperCall(comp, CORINFO_HELP_DBL2INT_OVF) &&
+                             !IsHelperCall(comp, CORINFO_HELP_DBL2LNG_OVF) &&
+                             !IsHelperCall(comp, CORINFO_HELP_DBL2UINT_OVF) &&
+                             !IsHelperCall(comp, CORINFO_HELP_DBL2ULNG_OVF);
+            break;
+        }
+
+        default:
+        {
+            unreached();
+        }
+    }
+
+    if (checkSignature)
+    {
+        if (varTypeUsesFloatReg(this))
+        {
+            needsVzeroupper = true;
+        }
+        else
+        {
+            for (CallArg& arg : gtArgs.Args())
+            {
+                if (varTypeUsesFloatReg(arg.GetSignatureType()))
                 {
                     needsVzeroupper = true;
                     break;
                 }
-                else
-                {
-                    for (CallArg& arg : gtArgs.Args())
-                    {
-                        if (varTypeUsesFloatReg(arg.GetSignatureType()))
-                        {
-                            needsVzeroupper = true;
-                            break;
-                        }
-                    }
-                }
-                break;
-            }
-
-            default:
-            {
-                unreached();
             }
         }
     }
-
-    // Other special cases
-    //
-    if (!needsVzeroupper && IsHelperCall(comp, CORINFO_HELP_BULK_WRITEBARRIER))
-    {
-        needsVzeroupper = true;
-    }
-
     return needsVzeroupper;
 }
 #endif // TARGET_XARCH
@@ -13708,7 +13716,7 @@ GenTree* Compiler::gtFoldExprCall(GenTreeCall* call)
     assert(!call->gtArgs.AreArgsComplete());
 
     // Can only fold calls to special intrinsics.
-    if ((call->gtCallMoreFlags & GTF_CALL_M_SPECIAL_INTRINSIC) == 0)
+    if (!call->IsSpecialIntrinsic())
     {
         return call;
     }
@@ -17675,7 +17683,7 @@ Compiler::TypeProducerKind Compiler::gtGetTypeProducerKind(GenTree* tree)
                 return TPK_Handle;
             }
         }
-        else if (tree->AsCall()->gtCallMoreFlags & GTF_CALL_M_SPECIAL_INTRINSIC)
+        else if (tree->AsCall()->IsSpecialIntrinsic())
         {
             if (lookupNamedIntrinsic(tree->AsCall()->gtCallMethHnd) == NI_System_Object_GetType)
             {
@@ -19135,7 +19143,7 @@ CORINFO_CLASS_HANDLE Compiler::gtGetClassHandle(GenTree* tree, bool* pIsExact, b
         case GT_CALL:
         {
             GenTreeCall* call = obj->AsCall();
-            if (call->gtCallMoreFlags & GTF_CALL_M_SPECIAL_INTRINSIC)
+            if (call->IsSpecialIntrinsic())
             {
                 NamedIntrinsic ni = lookupNamedIntrinsic(call->gtCallMethHnd);
                 if ((ni == NI_System_Array_Clone) || (ni == NI_System_Object_MemberwiseClone))
diff --git a/src/coreclr/jit/helperexpansion.cpp b/src/coreclr/jit/helperexpansion.cpp
@@ -1605,7 +1605,7 @@ PhaseStatus Compiler::fgVNBasedIntrinsicExpansion()
 //
 bool Compiler::fgVNBasedIntrinsicExpansionForCall(BasicBlock** pBlock, Statement* stmt, GenTreeCall* call)
 {
-    if ((call->gtCallMoreFlags & GTF_CALL_M_SPECIAL_INTRINSIC) == 0)
+    if (!call->IsSpecialIntrinsic())
     {
         return false;
     }
diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp
@@ -1490,7 +1490,7 @@ var_types Compiler::impImportCall(OPCODE                  opcode,
                     {
                         spillStack = false;
                     }
-                    else if ((callNode->gtCallMoreFlags & GTF_CALL_M_SPECIAL_INTRINSIC) != 0)
+                    else if (callNode->IsSpecialIntrinsic())
                     {
                         spillStack = false;
                     }
@@ -4090,7 +4090,7 @@ GenTree* Compiler::impIntrinsic(CORINFO_CLASS_HANDLE    clsHnd,
                 if (impStackTop().val->OperIs(GT_RET_EXPR))
                 {
                     GenTreeCall* call = impStackTop().val->AsRetExpr()->gtInlineCandidate->AsCall();
-                    if (call->gtCallMoreFlags & GTF_CALL_M_SPECIAL_INTRINSIC)
+                    if (call->IsSpecialIntrinsic())
                     {
                         if (lookupNamedIntrinsic(call->gtCallMethHnd) == NI_System_Threading_Thread_get_CurrentThread)
                         {
@@ -4336,7 +4336,7 @@ GenTree* Compiler::impIntrinsic(CORINFO_CLASS_HANDLE    clsHnd,
             case NI_System_Math_Log2:
             case NI_System_Math_Log10:
             {
-                retNode = impMathIntrinsic(method, sig R2RARG(entryPoint), callType, ni, tailCall);
+                retNode = impMathIntrinsic(method, sig R2RARG(entryPoint), callType, ni, tailCall, &isSpecial);
                 break;
             }
 
@@ -4429,7 +4429,7 @@ GenTree* Compiler::impIntrinsic(CORINFO_CLASS_HANDLE    clsHnd,
             case NI_System_Math_Tanh:
             case NI_System_Math_Truncate:
             {
-                retNode = impMathIntrinsic(method, sig R2RARG(entryPoint), callType, ni, tailCall);
+                retNode = impMathIntrinsic(method, sig R2RARG(entryPoint), callType, ni, tailCall, &isSpecial);
                 break;
             }
 
@@ -9533,29 +9533,46 @@ GenTree* Compiler::impMathIntrinsic(CORINFO_METHOD_HANDLE method,
                                     CORINFO_SIG_INFO* sig R2RARG(CORINFO_CONST_LOOKUP* entryPoint),
                                     var_types             callType,
                                     NamedIntrinsic        intrinsicName,
-                                    bool                  tailCall)
+                                    bool                  tailCall,
+                                    bool*                 isSpecial)
 {
     GenTree* op1;
     GenTree* op2;
 
     assert(callType != TYP_STRUCT);
     assert(IsMathIntrinsic(intrinsicName));
+    assert(isSpecial != nullptr);
 
     op1 = nullptr;
 
+    bool isIntrinsicImplementedByUserCall = IsIntrinsicImplementedByUserCall(intrinsicName);
+
+    if (isIntrinsicImplementedByUserCall)
+    {
+#if defined(TARGET_XARCH)
+        // We want to track math intrinsics implemented as user calls as special
+        // to ensure we don't lose track of the fact it will call into native code
+        //
+        // This is used on xarch to track that it may need vzeroupper inserted to
+        // avoid the perf penalty on some hardware.
+
+        *isSpecial = true;
+#endif // TARGET_XARCH
+    }
+
 #if !defined(TARGET_X86)
     // Intrinsics that are not implemented directly by target instructions will
     // be re-materialized as users calls in rationalizer. For prefixed tail calls,
     // don't do this optimization, because
     //  a) For back compatibility reasons on desktop .NET Framework 4.6 / 4.6.1
     //  b) It will be non-trivial task or too late to re-materialize a surviving
     //     tail prefixed GT_INTRINSIC as tail call in rationalizer.
-    if (!IsIntrinsicImplementedByUserCall(intrinsicName) || !tailCall)
+    if (!isIntrinsicImplementedByUserCall || !tailCall)
 #else
     // On x86 RyuJIT, importing intrinsics that are implemented as user calls can cause incorrect calculation
     // of the depth of the stack if these intrinsics are used as arguments to another call. This causes bad
     // code generation for certain EH constructs.
-    if (!IsIntrinsicImplementedByUserCall(intrinsicName))
+    if (!isIntrinsicImplementedByUserCall)
 #endif
     {
         CORINFO_ARG_LIST_HANDLE arg = sig->args;
@@ -9588,7 +9605,7 @@ GenTree* Compiler::impMathIntrinsic(CORINFO_METHOD_HANDLE method,
                 NO_WAY("Unsupported number of args for Math Intrinsic");
         }
 
-        if (IsIntrinsicImplementedByUserCall(intrinsicName))
+        if (isIntrinsicImplementedByUserCall)
         {
             op1->gtFlags |= GTF_CALL;
         }
@@ -10073,7 +10090,7 @@ GenTree* Compiler::impMinMaxIntrinsic(CORINFO_METHOD_HANDLE method,
 #endif // FEATURE_HW_INTRINSICS && TARGET_XARCH
 
     // TODO-CQ: Returning this as an intrinsic blocks inlining and is undesirable
-    // return impMathIntrinsic(method, sig, callType, intrinsicName, tailCall);
+    // return impMathIntrinsic(method, sig, callType, intrinsicName, tailCall, isSpecial);
 
     return nullptr;
 }
diff --git a/src/coreclr/jit/importervectorization.cpp b/src/coreclr/jit/importervectorization.cpp
@@ -351,7 +351,7 @@ GenTreeStrCon* Compiler::impGetStrConFromSpan(GenTree* span)
         argCall = span->AsCall();
     }
 
-    if ((argCall != nullptr) && ((argCall->gtCallMoreFlags & GTF_CALL_M_SPECIAL_INTRINSIC) != 0))
+    if ((argCall != nullptr) && argCall->IsSpecialIntrinsic())
     {
         const NamedIntrinsic ni = lookupNamedIntrinsic(argCall->gtCallMethHnd);
         if ((ni == NI_System_MemoryExtensions_AsSpan) || (ni == NI_System_String_op_Implicit))
diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp
@@ -2817,7 +2817,7 @@ GenTree* Lowering::LowerCall(GenTree* node)
 
 #if defined(TARGET_AMD64) || defined(TARGET_ARM64)
     GenTree* nextNode = nullptr;
-    if (call->gtCallMoreFlags & GTF_CALL_M_SPECIAL_INTRINSIC)
+    if (call->IsSpecialIntrinsic())
     {
         switch (comp->lookupNamedIntrinsic(call->gtCallMethHnd))
         {
diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp
@@ -4975,7 +4975,7 @@ GenTree* Compiler::fgMorphPotentialTailCall(GenTreeCall* call)
 #endif
     };
 
-    if (call->gtCallMoreFlags & GTF_CALL_M_SPECIAL_INTRINSIC)
+    if (call->IsSpecialIntrinsic())
     {
         failTailCall("Might turn into an intrinsic");
         return nullptr;
@@ -6870,7 +6870,7 @@ GenTree* Compiler::fgMorphCall(GenTreeCall* call)
 #endif
     }
 
-    if ((call->gtCallMoreFlags & GTF_CALL_M_SPECIAL_INTRINSIC) != 0)
+    if (call->IsSpecialIntrinsic())
     {
         if (lookupNamedIntrinsic(call->AsCall()->gtCallMethHnd) ==
             NI_System_Text_UTF8Encoding_UTF8EncodingSealed_ReadUtf8)
@@ -6959,7 +6959,7 @@ GenTree* Compiler::fgMorphCall(GenTreeCall* call)
     // Morph Type.op_Equality, Type.op_Inequality, and Enum.HasFlag
     //
     // We need to do these before the arguments are morphed
-    if (!call->gtArgs.AreArgsComplete() && (call->gtCallMoreFlags & GTF_CALL_M_SPECIAL_INTRINSIC))
+    if (!call->gtArgs.AreArgsComplete() && call->IsSpecialIntrinsic())
     {
         // See if this is foldable
         GenTree* optTree = gtFoldExprCall(call);
diff --git a/src/coreclr/jit/rationalize.cpp b/src/coreclr/jit/rationalize.cpp
diff --git a/src/coreclr/jit/rationalize.h b/src/coreclr/jit/rationalize.h

Original file line number	Diff line number	Diff line change
`@@ -10156,7 +10156,7 @@ JITDBGAPI void __cdecl cTreeFlags(Compiler* comp, GenTree* tree)`
`10156`	`10156`	`{`
`10157`	`10157`	`chars += printf("[CALL_M_NOGCCHECK]");`
`10158`	`10158`	`}`
`10159`		`- if (call->gtCallMoreFlags & GTF_CALL_M_SPECIAL_INTRINSIC)`
	`10159`	`+ if (call->IsSpecialIntrinsic())`
`10160`	`10160`	`{`
`10161`	`10161`	`chars += printf("[CALL_M_SPECIAL_INTRINSIC]");`
`10162`	`10162`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1605,7 +1605,7 @@ PhaseStatus Compiler::fgVNBasedIntrinsicExpansion()`
`1605`	`1605`	`//`
`1606`	`1606`	`bool Compiler::fgVNBasedIntrinsicExpansionForCall(BasicBlock** pBlock, Statement* stmt, GenTreeCall* call)`
`1607`	`1607`	`{`
`1608`		`- if ((call->gtCallMoreFlags & GTF_CALL_M_SPECIAL_INTRINSIC) == 0)`
	`1608`	`+ if (!call->IsSpecialIntrinsic())`
`1609`	`1609`	`{`
`1610`	`1610`	`return false;`
`1611`	`1611`	`}`
Original file line number	Diff line number	Diff line change
`@@ -351,7 +351,7 @@ GenTreeStrCon* Compiler::impGetStrConFromSpan(GenTree* span)`
`351`	`351`	`argCall = span->AsCall();`
`352`	`352`	`}`
`353`	`353`
`354`		`- if ((argCall != nullptr) && ((argCall->gtCallMoreFlags & GTF_CALL_M_SPECIAL_INTRINSIC) != 0))`
	`354`	`+ if ((argCall != nullptr) && argCall->IsSpecialIntrinsic())`
`355`	`355`	`{`
`356`	`356`	`const NamedIntrinsic ni = lookupNamedIntrinsic(argCall->gtCallMethHnd);`
`357`	`357`	`if ((ni == NI_System_MemoryExtensions_AsSpan) \|\| (ni == NI_System_String_op_Implicit))`
Original file line number	Diff line number	Diff line change
`@@ -2817,7 +2817,7 @@ GenTree* Lowering::LowerCall(GenTree* node)`
`2817`	`2817`
`2818`	`2818`	`#if defined(TARGET_AMD64) \|\| defined(TARGET_ARM64)`
`2819`	`2819`	`GenTree* nextNode = nullptr;`
`2820`		`- if (call->gtCallMoreFlags & GTF_CALL_M_SPECIAL_INTRINSIC)`
	`2820`	`+ if (call->IsSpecialIntrinsic())`
`2821`	`2821`	`{`
`2822`	`2822`	`switch (comp->lookupNamedIntrinsic(call->gtCallMethHnd))`
`2823`	`2823`	`{`
Original file line number	Diff line number	Diff line change
`@@ -4975,7 +4975,7 @@ GenTree* Compiler::fgMorphPotentialTailCall(GenTreeCall* call)`
`4975`	`4975`	`#endif`
`4976`	`4976`	`};`
`4977`	`4977`
`4978`		`- if (call->gtCallMoreFlags & GTF_CALL_M_SPECIAL_INTRINSIC)`
	`4978`	`+ if (call->IsSpecialIntrinsic())`
`4979`	`4979`	`{`
`4980`	`4980`	`failTailCall("Might turn into an intrinsic");`
`4981`	`4981`	`return nullptr;`
`@@ -6870,7 +6870,7 @@ GenTree* Compiler::fgMorphCall(GenTreeCall* call)`
`6870`	`6870`	`#endif`
`6871`	`6871`	`}`
`6872`	`6872`
`6873`		`- if ((call->gtCallMoreFlags & GTF_CALL_M_SPECIAL_INTRINSIC) != 0)`
	`6873`	`+ if (call->IsSpecialIntrinsic())`
`6874`	`6874`	`{`
`6875`	`6875`	`if (lookupNamedIntrinsic(call->AsCall()->gtCallMethHnd) ==`
`6876`	`6876`	`NI_System_Text_UTF8Encoding_UTF8EncodingSealed_ReadUtf8)`
`@@ -6959,7 +6959,7 @@ GenTree* Compiler::fgMorphCall(GenTreeCall* call)`
`6959`	`6959`	`// Morph Type.op_Equality, Type.op_Inequality, and Enum.HasFlag`
`6960`	`6960`	`//`
`6961`	`6961`	`// We need to do these before the arguments are morphed`
`6962`		`- if (!call->gtArgs.AreArgsComplete() && (call->gtCallMoreFlags & GTF_CALL_M_SPECIAL_INTRINSIC))`
	`6962`	`+ if (!call->gtArgs.AreArgsComplete() && call->IsSpecialIntrinsic())`
`6963`	`6963`	`{`
`6964`	`6964`	`// See if this is foldable`
`6965`	`6965`	`GenTree* optTree = gtFoldExprCall(call);`