Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 1, 2025
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cbaed82
Ensure that math calls into the CRT are tracked as needing vzeroupper
tannergooding Jan 30, 2025
34fe382
Apply suggestions from code review
tannergooding Jan 31, 2025
1565f2b
Don't force vzeroupper for helpers written in managed
tannergooding Jan 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/coreclr/jit/compiler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10165,7 +10165,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]");
}
Expand Down
9 changes: 5 additions & 4 deletions src/coreclr/jit/compiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4014,7 +4014,7 @@ class Compiler

// false: we can add new tracked variables.
// true: We cannot add new 'tracked' variable
bool lvaTrackedFixed = false;
bool lvaTrackedFixed = false;

unsigned lvaCount; // total number of locals, which includes function arguments,
// special arguments, IL local variables, and JIT temporary variables
Expand Down Expand Up @@ -4763,7 +4763,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,
Expand Down Expand Up @@ -6924,15 +6925,15 @@ class Compiler
unsigned acdCount = 0;

// Get the index to use as part of the AddCodeDsc key for sharing throw blocks
unsigned bbThrowIndex(BasicBlock* blk, AcdKeyDesignator* dsg);
unsigned bbThrowIndex(BasicBlock* blk, AcdKeyDesignator* dsg);

struct AddCodeDscKey
{
public:
AddCodeDscKey(): acdKind(SCK_NONE), acdData(0) {}
AddCodeDscKey(SpecialCodeKind kind, BasicBlock* block, Compiler* comp);
AddCodeDscKey(AddCodeDsc* add);

static bool Equals(const AddCodeDscKey& x, const AddCodeDscKey& y)
{
return (x.acdData == y.acdData) && (x.acdKind == y.acdKind);
Expand Down
100 changes: 52 additions & 48 deletions src/coreclr/jit/gentree.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2005,66 +2005,70 @@ 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:
{
// 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;

// A few other 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);
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
Expand Down Expand Up @@ -13708,7 +13712,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;
}
Expand Down Expand Up @@ -17675,7 +17679,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)
{
Expand Down Expand Up @@ -19135,7 +19139,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))
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/helperexpansion.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
29 changes: 20 additions & 9 deletions src/coreclr/jit/importercalls.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -9533,29 +9533,40 @@ 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)
{
// 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
*isSpecial = true;
}

#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;
Expand Down Expand Up @@ -9588,7 +9599,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;
}
Expand Down Expand Up @@ -10073,7 +10084,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;
}
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/importervectorization.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/lower.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2764,7 +2764,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))
{
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/jit/morph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5021,7 +5021,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;
Expand Down Expand Up @@ -6916,7 +6916,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)
Expand Down Expand Up @@ -7005,7 +7005,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);
Expand Down
Loading
Loading