Skip to content

Ensure that GenTreeIntrinsic doesn't incorrectly overwrite the gtEntryPoint #102843

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
tannergooding merged 1 commit into from
May 30, 2024
Merged

Ensure that GenTreeIntrinsic doesn't incorrectly overwrite the gtEntryPoint #102843

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 6 additions & 0 deletions src/coreclr/inc/utilcode.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,12 @@ typedef LPSTR LPUTF8;
#define DEBUGARG(x)
#endif

#if defined(FEATURE_READYTORUN)
#define R2RARG(x) , x
#else
#define R2RARG(x)
#endif

#ifndef sizeofmember
// Returns the size of a class or struct member.
#define sizeofmember(c,m) (sizeof(((c*)0)->m))
Expand Down
12 changes: 8 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 @@ -4531,7 +4531,8 @@ class Compiler
bool tailCall,
bool callvirt,
CORINFO_RESOLVED_TOKEN* pContstrainedResolvedToken,
CORINFO_THIS_TRANSFORM constraintCallThisTransform,
CORINFO_THIS_TRANSFORM constraintCallThisTransform
R2RARG(CORINFO_CONST_LOOKUP* entryPoint),
NamedIntrinsic* pIntrinsicName,
bool* isSpecialIntrinsic = nullptr);
GenTree* impEstimateIntrinsic(CORINFO_METHOD_HANDLE method,
Expand All @@ -4540,7 +4541,8 @@ class Compiler
NamedIntrinsic intrinsicName,
bool mustExpand);
GenTree* impMathIntrinsic(CORINFO_METHOD_HANDLE method,
CORINFO_SIG_INFO* sig,
CORINFO_SIG_INFO* sig
R2RARG(CORINFO_CONST_LOOKUP* entryPoint),
var_types callType,
NamedIntrinsic intrinsicName,
bool tailCall);
Expand Down Expand Up @@ -4576,7 +4578,8 @@ class Compiler
GenTree* impHWIntrinsic(NamedIntrinsic intrinsic,
CORINFO_CLASS_HANDLE clsHnd,
CORINFO_METHOD_HANDLE method,
CORINFO_SIG_INFO* sig,
CORINFO_SIG_INFO* sig
R2RARG(CORINFO_CONST_LOOKUP* entryPoint),
bool mustExpand);
GenTree* impSimdAsHWIntrinsic(NamedIntrinsic intrinsic,
CORINFO_CLASS_HANDLE clsHnd,
Expand All @@ -4600,7 +4603,8 @@ class Compiler
GenTree* impSpecialIntrinsic(NamedIntrinsic intrinsic,
CORINFO_CLASS_HANDLE clsHnd,
CORINFO_METHOD_HANDLE method,
CORINFO_SIG_INFO* sig,
CORINFO_SIG_INFO* sig
R2RARG(CORINFO_CONST_LOOKUP* entryPoint),
CorInfoType simdBaseJitType,
var_types retType,
unsigned simdSize,
Expand Down
35 changes: 8 additions & 27 deletions src/coreclr/jit/gentree.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9635,10 +9635,8 @@ GenTree* Compiler::gtCloneExpr(GenTree* tree)
case GT_INTRINSIC:
copy = new (this, GT_INTRINSIC)
GenTreeIntrinsic(tree->TypeGet(), tree->AsOp()->gtOp1, tree->AsOp()->gtOp2,
tree->AsIntrinsic()->gtIntrinsicName, tree->AsIntrinsic()->gtMethodHandle);
#ifdef FEATURE_READYTORUN
copy->AsIntrinsic()->gtEntryPoint = tree->AsIntrinsic()->gtEntryPoint;
#endif
tree->AsIntrinsic()->gtIntrinsicName,
tree->AsIntrinsic()->gtMethodHandle R2RARG(tree->AsIntrinsic()->gtEntryPoint));
break;

case GT_BOUNDS_CHECK:
Expand Down Expand Up @@ -9723,7 +9721,8 @@ GenTree* Compiler::gtCloneExpr(GenTree* tree)

if (tree->AsHWIntrinsic()->IsUserCall())
{
copy->AsHWIntrinsic()->SetMethodHandle(this, tree->AsHWIntrinsic()->GetMethodHandle());
copy->AsHWIntrinsic()->SetMethodHandle(this, tree->AsHWIntrinsic()->GetMethodHandle()
R2RARG(tree->AsHWIntrinsic()->GetEntryPoint()));
}
goto CLONE_MULTIOP_OPERANDS;
#endif
Expand Down Expand Up @@ -19591,13 +19590,15 @@ void GenTreeMultiOp::InitializeOperands(GenTree** operands, size_t operandCount)
// Arguments:
// comp - The compiler instance
// methodHandle - The method handle representing the fallback handling for the intrinsic
// entryPoint - The entry point information required for R2R scenarios
//
// Notes:
// We need to ensure that the operands are not tracked inline so that we can track the
// underlying method handle. See the comment in GenTreeJitIntrinsic around why the union
// of fields exists.
//
void GenTreeJitIntrinsic::SetMethodHandle(Compiler* comp, CORINFO_METHOD_HANDLE methodHandle)
void GenTreeJitIntrinsic::SetMethodHandle(Compiler* comp,
CORINFO_METHOD_HANDLE methodHandle R2RARG(CORINFO_CONST_LOOKUP entryPoint))
{
assert(OperIsHWIntrinsic() && !IsUserCall());
gtFlags |= GTF_HW_USER_CALL;
Expand All @@ -19619,31 +19620,11 @@ void GenTreeJitIntrinsic::SetMethodHandle(Compiler* comp, CORINFO_METHOD_HANDLE
}

gtMethodHandle = methodHandle;
gtEntryPoint = nullptr;
}

#if defined(FEATURE_READYTORUN)
//------------------------------------------------------------------------
// GenTreeJitIntrinsic::SetEntryPoint: Sets the entry point for an intrinsic
// so that it can be rewritten back to a user call in a later phase for R2R
// scenarios
//
// Arguments:
// comp - The compiler instance
// entryPoint - The entry point information required for R2R scenarios
//
// Notes:
// This requires SetMethodHandle to have been called first to ensure we aren't
// overwriting any inline operands
//
void GenTreeJitIntrinsic::SetEntryPoint(Compiler* comp, CORINFO_CONST_LOOKUP entryPoint)
{
assert(IsUserCall());
assert(gtEntryPoint == nullptr);

gtEntryPoint = new (comp, CMK_ASTNode) CORINFO_CONST_LOOKUP(entryPoint);
}
#endif // FEATURE_READYTORUN
}

var_types GenTreeJitIntrinsic::GetAuxiliaryType() const
{
Expand Down
24 changes: 14 additions & 10 deletions src/coreclr/jit/gentree.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5952,24 +5952,30 @@ struct GenTreeIntrinsic : public GenTreeOp
NamedIntrinsic gtIntrinsicName;
CORINFO_METHOD_HANDLE gtMethodHandle; // Method handle of the method which is treated as an intrinsic.

#ifdef FEATURE_READYTORUN
#if defined(FEATURE_READYTORUN)
// Call target lookup info for method call from a Ready To Run module
CORINFO_CONST_LOOKUP gtEntryPoint;
#endif
#endif // FEATURE_READYTORUN

GenTreeIntrinsic(var_types type, GenTree* op1, NamedIntrinsic intrinsicName, CORINFO_METHOD_HANDLE methodHandle)
GenTreeIntrinsic(var_types type,
GenTree* op1,
NamedIntrinsic intrinsicName,
CORINFO_METHOD_HANDLE methodHandle R2RARG(CORINFO_CONST_LOOKUP entryPoint))
: GenTreeOp(GT_INTRINSIC, type, op1, nullptr)
, gtIntrinsicName(intrinsicName)
, gtMethodHandle(methodHandle)
, gtMethodHandle(methodHandle) R2RARG(gtEntryPoint(entryPoint))
{
assert(intrinsicName != NI_Illegal);
}

GenTreeIntrinsic(
var_types type, GenTree* op1, GenTree* op2, NamedIntrinsic intrinsicName, CORINFO_METHOD_HANDLE methodHandle)
GenTreeIntrinsic(var_types type,
GenTree* op1,
GenTree* op2,
NamedIntrinsic intrinsicName,
CORINFO_METHOD_HANDLE methodHandle R2RARG(CORINFO_CONST_LOOKUP entryPoint))
: GenTreeOp(GT_INTRINSIC, type, op1, op2)
, gtIntrinsicName(intrinsicName)
, gtMethodHandle(methodHandle)
, gtMethodHandle(methodHandle) R2RARG(gtEntryPoint(entryPoint))
{
assert(intrinsicName != NI_Illegal);
}
Expand Down Expand Up @@ -6264,16 +6270,14 @@ struct GenTreeJitIntrinsic : public GenTreeMultiOp
return gtMethodHandle;
}

void SetMethodHandle(Compiler* comp, CORINFO_METHOD_HANDLE methodHandle);
void SetMethodHandle(Compiler* comp, CORINFO_METHOD_HANDLE methodHandle R2RARG(CORINFO_CONST_LOOKUP entryPoint));

#if defined(FEATURE_READYTORUN)
CORINFO_CONST_LOOKUP GetEntryPoint() const
{
assert(IsUserCall());
return *gtEntryPoint;
}

void SetEntryPoint(Compiler* comp, CORINFO_CONST_LOOKUP entryPoint);
#endif // FEATURE_READYTORUN

//-----------------------------------------------------------
Expand Down
7 changes: 4 additions & 3 deletions src/coreclr/jit/hwintrinsic.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1131,6 +1131,7 @@ bool Compiler::CheckHWIntrinsicImmRange(NamedIntrinsic intrinsic,
// clsHnd -- class handle containing the intrinsic function.
// method -- method handle of the intrinsic function.
// sig -- signature of the intrinsic call
// entryPoint -- The entry point information required for R2R scenarios
// mustExpand -- true if the intrinsic must return a GenTree*; otherwise, false

// Return Value:
Expand All @@ -1139,7 +1140,7 @@ bool Compiler::CheckHWIntrinsicImmRange(NamedIntrinsic intrinsic,
GenTree* Compiler::impHWIntrinsic(NamedIntrinsic intrinsic,
CORINFO_CLASS_HANDLE clsHnd,
CORINFO_METHOD_HANDLE method,
CORINFO_SIG_INFO* sig,
CORINFO_SIG_INFO* sig R2RARG(CORINFO_CONST_LOOKUP* entryPoint),
bool mustExpand)
{
// NextCallRetAddr requires a CALL, so return nullptr.
Expand Down Expand Up @@ -1577,8 +1578,8 @@ GenTree* Compiler::impHWIntrinsic(NamedIntrinsic intrinsic,
}
else
{
retNode =
impSpecialIntrinsic(intrinsic, clsHnd, method, sig, simdBaseJitType, nodeRetType, simdSize, mustExpand);
retNode = impSpecialIntrinsic(intrinsic, clsHnd, method, sig R2RARG(entryPoint), simdBaseJitType, nodeRetType,
simdSize, mustExpand);
}

#if defined(TARGET_ARM64)
Expand Down
5 changes: 3 additions & 2 deletions src/coreclr/jit/hwintrinsicarm64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,7 @@ GenTree* Compiler::impNonConstFallback(NamedIntrinsic intrinsic, var_types simdT
// clsHnd -- class handle containing the intrinsic function.
// method -- method handle of the intrinsic function.
// sig -- signature of the intrinsic call.
// entryPoint -- The entry point information required for R2R scenarios
// simdBaseJitType -- generic argument of the intrinsic.
// retType -- return type of the intrinsic.
// mustExpand -- true if the intrinsic must return a GenTree*; otherwise, false
Expand All @@ -492,7 +493,7 @@ GenTree* Compiler::impNonConstFallback(NamedIntrinsic intrinsic, var_types simdT
GenTree* Compiler::impSpecialIntrinsic(NamedIntrinsic intrinsic,
CORINFO_CLASS_HANDLE clsHnd,
CORINFO_METHOD_HANDLE method,
CORINFO_SIG_INFO* sig,
CORINFO_SIG_INFO* sig R2RARG(CORINFO_CONST_LOOKUP* entryPoint),
CorInfoType simdBaseJitType,
var_types retType,
unsigned simdSize,
Expand Down Expand Up @@ -1876,7 +1877,7 @@ GenTree* Compiler::impSpecialIntrinsic(NamedIntrinsic intrinsic,

retNode = gtNewSimdHWIntrinsicNode(retType, op1, op2, intrinsic, simdBaseJitType, simdSize);

retNode->AsHWIntrinsic()->SetMethodHandle(this, method);
retNode->AsHWIntrinsic()->SetMethodHandle(this, method R2RARG(*entryPoint));
break;
}

Expand Down
47 changes: 29 additions & 18 deletions src/coreclr/jit/hwintrinsicxarch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -955,6 +955,7 @@ GenTree* Compiler::impNonConstFallback(NamedIntrinsic intrinsic, var_types simdT
// clsHnd -- class handle containing the intrinsic function.
// method -- method handle of the intrinsic function.
// sig -- signature of the intrinsic call.
// entryPoint -- The entry point information required for R2R scenarios
// simdBaseJitType -- generic argument of the intrinsic.
// retType -- return type of the intrinsic.
// mustExpand -- true if the intrinsic must return a GenTree*; otherwise, false
Expand All @@ -965,7 +966,7 @@ GenTree* Compiler::impNonConstFallback(NamedIntrinsic intrinsic, var_types simdT
GenTree* Compiler::impSpecialIntrinsic(NamedIntrinsic intrinsic,
CORINFO_CLASS_HANDLE clsHnd,
CORINFO_METHOD_HANDLE method,
CORINFO_SIG_INFO* sig,
CORINFO_SIG_INFO* sig R2RARG(CORINFO_CONST_LOOKUP* entryPoint),
CorInfoType simdBaseJitType,
var_types retType,
unsigned simdSize,
Expand All @@ -986,6 +987,13 @@ GenTree* Compiler::impSpecialIntrinsic(NamedIntrinsic intrinsic,
assert(varTypeIsArithmetic(simdBaseType));
}

#if defined(FEATURE_READYTORUN)
CORINFO_CONST_LOOKUP emptyEntryPoint;

emptyEntryPoint.addr = nullptr;
emptyEntryPoint.accessType = IAT_VALUE;
#endif // FEATURE_READYTORUN

switch (intrinsic)
{
case NI_Vector128_Abs:
Expand Down Expand Up @@ -1100,8 +1108,8 @@ GenTree* Compiler::impSpecialIntrinsic(NamedIntrinsic intrinsic,
if (vectorTByteLength == YMM_REGSIZE_BYTES)
{
// Vector<T> is TYP_SIMD32, so we should treat this as a call to Vector128.ToVector256
return impSpecialIntrinsic(NI_Vector128_ToVector256, clsHnd, method, sig, simdBaseJitType, retType,
simdSize, mustExpand);
return impSpecialIntrinsic(NI_Vector128_ToVector256, clsHnd, method, sig R2RARG(&emptyEntryPoint),
simdBaseJitType, retType, simdSize, mustExpand);
}
else if (vectorTByteLength == XMM_REGSIZE_BYTES)
{
Expand Down Expand Up @@ -1210,8 +1218,8 @@ GenTree* Compiler::impSpecialIntrinsic(NamedIntrinsic intrinsic,
case TYP_SIMD32:
{
// Vector<T> is TYP_SIMD32, so we should treat this as a call to Vector256.GetLower
return impSpecialIntrinsic(NI_Vector256_GetLower, clsHnd, method, sig, simdBaseJitType, retType,
simdSize, mustExpand);
return impSpecialIntrinsic(NI_Vector256_GetLower, clsHnd, method, sig R2RARG(&emptyEntryPoint),
simdBaseJitType, retType, simdSize, mustExpand);
}

default:
Expand Down Expand Up @@ -1250,14 +1258,15 @@ GenTree* Compiler::impSpecialIntrinsic(NamedIntrinsic intrinsic,

if (intrinsic == NI_Vector256_AsVector)
{
return impSpecialIntrinsic(NI_Vector256_GetLower, clsHnd, method, sig, simdBaseJitType, retType,
simdSize, mustExpand);
return impSpecialIntrinsic(NI_Vector256_GetLower, clsHnd, method, sig R2RARG(&emptyEntryPoint),
simdBaseJitType, retType, simdSize, mustExpand);
}
else
{
assert(intrinsic == NI_Vector256_AsVector256);
return impSpecialIntrinsic(NI_Vector128_ToVector256, clsHnd, method, sig, simdBaseJitType,
retType, 16, mustExpand);
return impSpecialIntrinsic(NI_Vector128_ToVector256, clsHnd, method,
sig R2RARG(&emptyEntryPoint), simdBaseJitType, retType, 16,
mustExpand);
}
}
}
Expand All @@ -1283,14 +1292,14 @@ GenTree* Compiler::impSpecialIntrinsic(NamedIntrinsic intrinsic,

if (intrinsic == NI_Vector512_AsVector)
{
return impSpecialIntrinsic(NI_Vector512_GetLower, clsHnd, method, sig, simdBaseJitType, retType,
simdSize, mustExpand);
return impSpecialIntrinsic(NI_Vector512_GetLower, clsHnd, method, sig R2RARG(&emptyEntryPoint),
simdBaseJitType, retType, simdSize, mustExpand);
}
else
{
assert(intrinsic == NI_Vector512_AsVector512);
return impSpecialIntrinsic(NI_Vector256_ToVector512, clsHnd, method, sig, simdBaseJitType, retType,
32, mustExpand);
return impSpecialIntrinsic(NI_Vector256_ToVector512, clsHnd, method, sig R2RARG(&emptyEntryPoint),
simdBaseJitType, retType, 32, mustExpand);
}
break;
}
Expand All @@ -1303,14 +1312,16 @@ GenTree* Compiler::impSpecialIntrinsic(NamedIntrinsic intrinsic,

if (intrinsic == NI_Vector512_AsVector)
{
return impSpecialIntrinsic(NI_Vector512_GetLower128, clsHnd, method, sig, simdBaseJitType,
retType, simdSize, mustExpand);
return impSpecialIntrinsic(NI_Vector512_GetLower128, clsHnd, method,
sig R2RARG(&emptyEntryPoint), simdBaseJitType, retType, simdSize,
mustExpand);
}
else
{
assert(intrinsic == NI_Vector512_AsVector512);
return impSpecialIntrinsic(NI_Vector128_ToVector512, clsHnd, method, sig, simdBaseJitType,
retType, 16, mustExpand);
return impSpecialIntrinsic(NI_Vector128_ToVector512, clsHnd, method,
sig R2RARG(&emptyEntryPoint), simdBaseJitType, retType, 16,
mustExpand);
}
}
}
Expand Down Expand Up @@ -2897,7 +2908,7 @@ GenTree* Compiler::impSpecialIntrinsic(NamedIntrinsic intrinsic,

retNode = gtNewSimdHWIntrinsicNode(retType, op1, op2, intrinsic, simdBaseJitType, simdSize);

retNode->AsHWIntrinsic()->SetMethodHandle(this, method);
retNode->AsHWIntrinsic()->SetMethodHandle(this, method R2RARG(*entryPoint));
break;
}

Expand Down
Loading