Skip to content

Fix HFA/HVA classification #37499

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.

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
Jun 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -614,7 +614,7 @@ CORINFO_CLASS_HANDLE getArgClass(CORINFO_SIG_INFO* sig, /* IN */
);

// Returns type of HFA for valuetype
CorInfoType getHFAType(CORINFO_CLASS_HANDLE hClass);
CorInfoHFAElemType getHFAType(CORINFO_CLASS_HANDLE hClass);

/*****************************************************************************
* ICorErrorInfo contains methods to deal with SEH exceptions being thrown
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2680,7 +2680,7 @@ CORINFO_CLASS_HANDLE MethodContext::repGetArgClass(CORINFO_SIG_INFO* sig,
return (CORINFO_CLASS_HANDLE)value.result;
}

void MethodContext::recGetHFAType(CORINFO_CLASS_HANDLE clsHnd, CorInfoType result)
void MethodContext::recGetHFAType(CORINFO_CLASS_HANDLE clsHnd, CorInfoHFAElemType result)
{
if (GetHFAType == nullptr)
GetHFAType = new LightWeightMap<DWORDLONG, DWORD>();
Expand All @@ -2696,7 +2696,7 @@ void MethodContext::dmpGetHFAType(DWORDLONG key, DWORD value)
return;
}

CorInfoType MethodContext::repGetHFAType(CORINFO_CLASS_HANDLE clsHnd)
CorInfoHFAElemType MethodContext::repGetHFAType(CORINFO_CLASS_HANDLE clsHnd)
{
DWORD value;

Expand All @@ -2706,7 +2706,7 @@ CorInfoType MethodContext::repGetHFAType(CORINFO_CLASS_HANDLE clsHnd)

value = GetHFAType->Get((DWORDLONG)clsHnd);
DEBUG_REP(dmpGetHFAType((DWORDLONG)clsHnd, value));
return (CorInfoType)value;
return (CorInfoHFAElemType)value;
}

void MethodContext::recGetMethodInfo(CORINFO_METHOD_HANDLE ftn,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -867,9 +867,9 @@ class MethodContext
void dmpGetArgClass(const GetArgClassValue& key, const Agnostic_GetArgClass_Value& value);
CORINFO_CLASS_HANDLE repGetArgClass(CORINFO_SIG_INFO* sig, CORINFO_ARG_LIST_HANDLE args, DWORD* exceptionCode);

void recGetHFAType(CORINFO_CLASS_HANDLE clsHnd, CorInfoType result);
void recGetHFAType(CORINFO_CLASS_HANDLE clsHnd, CorInfoHFAElemType result);
void dmpGetHFAType(DWORDLONG key, DWORD value);
CorInfoType repGetHFAType(CORINFO_CLASS_HANDLE clsHnd);
CorInfoHFAElemType repGetHFAType(CORINFO_CLASS_HANDLE clsHnd);

void recGetMethodInfo(CORINFO_METHOD_HANDLE ftn, CORINFO_METHOD_INFO* info, bool result, DWORD exceptionCode);
void dmpGetMethodInfo(DWORDLONG key, const Agnostic_GetMethodInfo& value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1314,10 +1314,10 @@ CORINFO_CLASS_HANDLE interceptor_ICJI::getArgClass(CORINFO_SIG_INFO* sig,
}

// Returns type of HFA for valuetype
CorInfoType interceptor_ICJI::getHFAType(CORINFO_CLASS_HANDLE hClass)
CorInfoHFAElemType interceptor_ICJI::getHFAType(CORINFO_CLASS_HANDLE hClass)
{
mc->cr->AddCall("getHFAType");
CorInfoType temp = original_ICorJitInfo->getHFAType(hClass);
CorInfoHFAElemType temp = original_ICorJitInfo->getHFAType(hClass);
this->mc->recGetHFAType(hClass, temp);
return temp;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1009,7 +1009,7 @@ CORINFO_CLASS_HANDLE interceptor_ICJI::getArgClass(CORINFO_SIG_INFO* sig,
}

// Returns type of HFA for valuetype
CorInfoType interceptor_ICJI::getHFAType(CORINFO_CLASS_HANDLE hClass)
CorInfoHFAElemType interceptor_ICJI::getHFAType(CORINFO_CLASS_HANDLE hClass)
{
mcs->AddCall("getHFAType");
return original_ICorJitInfo->getHFAType(hClass);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -902,7 +902,7 @@ CORINFO_CLASS_HANDLE interceptor_ICJI::getArgClass(CORINFO_SIG_INFO* sig,
}

// Returns type of HFA for valuetype
CorInfoType interceptor_ICJI::getHFAType(CORINFO_CLASS_HANDLE hClass)
CorInfoHFAElemType interceptor_ICJI::getHFAType(CORINFO_CLASS_HANDLE hClass)
{
return original_ICorJitInfo->getHFAType(hClass);
}
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/src/ToolBox/superpmi/superpmi/icorjitinfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1088,10 +1088,10 @@ CORINFO_CLASS_HANDLE MyICJI::getArgClass(CORINFO_SIG_INFO* sig, /* IN */
}

// Returns type of HFA for valuetype
CorInfoType MyICJI::getHFAType(CORINFO_CLASS_HANDLE hClass)
CorInfoHFAElemType MyICJI::getHFAType(CORINFO_CLASS_HANDLE hClass)
{
jitInstance->mc->cr->AddCall("getHFAType");
CorInfoType value = jitInstance->mc->repGetHFAType(hClass);
CorInfoHFAElemType value = jitInstance->mc->repGetHFAType(hClass);
return value;
}

Expand Down
11 changes: 11 additions & 0 deletions src/coreclr/src/inc/corhdr.h
Original file line number Diff line number Diff line change
Expand Up @@ -1908,6 +1908,17 @@ typedef enum NativeTypeArrayFlags
ntaReserved = 0xfffe // All the reserved bits.
} NativeTypeArrayFlags;

//
// Enum used for HFA type recognition.
// Supported across architectures, so that it can be used in altjits and cross-compilation.
typedef enum CorInfoHFAElemType : unsigned {
CORINFO_HFA_ELEM_NONE,
CORINFO_HFA_ELEM_FLOAT,
CORINFO_HFA_ELEM_DOUBLE,
CORINFO_HFA_ELEM_VECTOR64,
CORINFO_HFA_ELEM_VECTOR128,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There aren't any architectures that have HVA for Vector256?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like __vectorcall for x64 will eventually fit this bill: https://godbolt.org/z/DMX7Y2, but not for ARM32/ARM64 (which don't support Vector256<T>) nor the default calling conventions for x86 or x64 on Unix/Windows.

System V does treat a simple wrapper over __m256 as if it was directly __m256 however, not sure if that is classified as an HFA/HVA by the JIT...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently the runtime (JIT + VM) handles the System V ABI completely separately. HFA/HVA is ARM32/ARM64 only.

Comment on lines +1918 to +1919
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically VECTOR64 and VECTOR128 are HVAs, not HFAs. For that reason I have renamed HFA to either HA or HomogenousAggregate everywhere in managed code. If we introduce and expose a new enumeration, I would prefer to name it correctly from the beginning. However, I realize there are lots of hfa usage in JIT code, so leaving it up to you.

} CorInfoHFAElemType;

//
// Opaque types for security properties and values.
//
Expand Down
14 changes: 7 additions & 7 deletions src/coreclr/src/inc/corinfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -217,12 +217,12 @@ TODO: Talk about initializing strutures before use
#endif
#endif

SELECTANY const GUID JITEEVersionIdentifier = { /* 8b2226a2-ac30-4f5c-ae5c-926c792ecdb9 */
0x8b2226a2,
0xac30,
0x4f5c,
{ 0xae, 0x5c, 0x92, 0x6c, 0x79, 0x2e, 0xcd, 0xb9 }
};
SELECTANY const GUID JITEEVersionIdentifier = { /* 2ca8d539-5db9-4831-8f1b-ade425f036bd */
0x2ca8d539,
0x5db9,
0x4831,
{0x8f, 0x1b, 0xad, 0xe4, 0x25, 0xf0, 0x36, 0xbd}
};

//////////////////////////////////////////////////////////////////////////////////////////////////////////
//
Expand Down Expand Up @@ -2743,7 +2743,7 @@ class ICorStaticInfo
) = 0;

// Returns type of HFA for valuetype
virtual CorInfoType getHFAType (
virtual CorInfoHFAElemType getHFAType (
CORINFO_CLASS_HANDLE hClass
) = 0;

Expand Down
23 changes: 6 additions & 17 deletions src/coreclr/src/jit/codegencommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10108,30 +10108,19 @@ unsigned Compiler::GetHfaCount(GenTree* tree)

var_types Compiler::GetHfaType(CORINFO_CLASS_HANDLE hClass)
{
var_types result = TYP_UNDEF;
#ifdef FEATURE_HFA
if (hClass != NO_CLASS_HANDLE)
{
#ifdef FEATURE_HFA
CorInfoType corType = info.compCompHnd->getHFAType(hClass);
#if defined(TARGET_ARM64) && defined(FEATURE_SIMD)
if (corType == CORINFO_TYPE_VALUECLASS)
CorInfoHFAElemType elemKind = info.compCompHnd->getHFAType(hClass);
if (elemKind != CORINFO_HFA_ELEM_NONE)
{
// This is a vector type.
// HVAs are only supported on ARM64, and only for homogeneous aggregates of 8 or 16 byte vectors.
// For 8-byte vectors corType will be returned as CORINFO_TYPE_DOUBLE.
result = TYP_SIMD16;
// This type may not appear elsewhere, but it will occupy a floating point register.
compFloatingPointUsed = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it safe to drop compFloatingPointUsed = true;?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, that was a mistake

}
else
#endif // TARGET_ARM64 && FEATURE_SIMD
if (corType != CORINFO_TYPE_UNDEF)
{
result = JITtype2varType(corType);
}
#endif // FEATURE_HFA
return HfaTypeFromElemKind(elemKind);
}
return result;
#endif // FEATURE_HFA
return TYP_UNDEF;
}

//------------------------------------------------------------------------
Expand Down
69 changes: 37 additions & 32 deletions src/coreclr/src/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -139,53 +139,50 @@ const int BAD_STK_OFFS = 0xBAADF00D; // for LclVarDsc::lvStkOffs
// HFA info shared by LclVarDsc and fgArgTabEntry
//------------------------------------------------------------------------
#ifdef FEATURE_HFA
enum HfaElemKind : unsigned int
inline bool IsHfa(CorInfoHFAElemType kind)
{
HFA_ELEM_NONE,
HFA_ELEM_FLOAT,
HFA_ELEM_DOUBLE,
HFA_ELEM_SIMD16
};
inline bool IsHfa(HfaElemKind kind)
{
return kind != HFA_ELEM_NONE;
return kind != CORINFO_HFA_ELEM_NONE;
}
inline var_types HfaTypeFromElemKind(HfaElemKind kind)
inline var_types HfaTypeFromElemKind(CorInfoHFAElemType kind)
{
switch (kind)
{
case HFA_ELEM_FLOAT:
case CORINFO_HFA_ELEM_FLOAT:
return TYP_FLOAT;
case HFA_ELEM_DOUBLE:
case CORINFO_HFA_ELEM_DOUBLE:
return TYP_DOUBLE;
#ifdef FEATURE_SIMD
case HFA_ELEM_SIMD16:
case CORINFO_HFA_ELEM_VECTOR64:
return TYP_SIMD8;
case CORINFO_HFA_ELEM_VECTOR128:
return TYP_SIMD16;
#endif
case HFA_ELEM_NONE:
case CORINFO_HFA_ELEM_NONE:
return TYP_UNDEF;
default:
assert(!"Invalid HfaElemKind");
return TYP_UNDEF;
}
}
inline HfaElemKind HfaElemKindFromType(var_types type)
inline CorInfoHFAElemType HfaElemKindFromType(var_types type)
{
switch (type)
{
case TYP_FLOAT:
return HFA_ELEM_FLOAT;
return CORINFO_HFA_ELEM_FLOAT;
case TYP_DOUBLE:
return HFA_ELEM_DOUBLE;
return CORINFO_HFA_ELEM_DOUBLE;
#ifdef FEATURE_SIMD
case TYP_SIMD8:
return CORINFO_HFA_ELEM_VECTOR64;
case TYP_SIMD16:
return HFA_ELEM_SIMD16;
return CORINFO_HFA_ELEM_VECTOR128;
#endif
case TYP_UNDEF:
return HFA_ELEM_NONE;
return CORINFO_HFA_ELEM_NONE;
default:
assert(!"Invalid HFA Type");
return HFA_ELEM_NONE;
return CORINFO_HFA_ELEM_NONE;
}
}
#endif // FEATURE_HFA
Expand Down Expand Up @@ -484,8 +481,8 @@ class LclVarDsc
unsigned char lvIsMultiRegRet : 1; // true if this is a multireg LclVar struct assigned from a multireg call

#ifdef FEATURE_HFA
HfaElemKind _lvHfaElemKind : 2; // What kind of an HFA this is (HFA_ELEM_NONE if it is not an HFA).
#endif // FEATURE_HFA
CorInfoHFAElemType _lvHfaElemKind : 3; // What kind of an HFA this is (CORINFO_HFA_ELEM_NONE if it is not an HFA).
#endif // FEATURE_HFA

#ifdef DEBUG
// TODO-Cleanup: See the note on lvSize() - this flag is only in use by asserts that are checking for struct
Expand Down Expand Up @@ -591,18 +588,19 @@ class LclVarDsc
#elif defined(TARGET_ARM64)
switch (_lvHfaElemKind)
{
case HFA_ELEM_NONE:
case CORINFO_HFA_ELEM_NONE:
assert(!"lvHfaSlots called for non-HFA");
break;
case HFA_ELEM_FLOAT:
case CORINFO_HFA_ELEM_FLOAT:
assert((lvExactSize % 4) == 0);
slots = lvExactSize >> 2;
break;
case HFA_ELEM_DOUBLE:
case CORINFO_HFA_ELEM_DOUBLE:
case CORINFO_HFA_ELEM_VECTOR64:
assert((lvExactSize % 8) == 0);
slots = lvExactSize >> 3;
break;
case HFA_ELEM_SIMD16:
case CORINFO_HFA_ELEM_VECTOR128:
assert((lvExactSize % 16) == 0);
slots = lvExactSize >> 4;
break;
Expand Down Expand Up @@ -919,7 +917,10 @@ class LclVarDsc
void SetHfaType(var_types type)
{
#ifdef FEATURE_HFA
_lvHfaElemKind = HfaElemKindFromType(type);
CorInfoHFAElemType elemKind = HfaElemKindFromType(type);
_lvHfaElemKind = elemKind;
// Ensure we've allocated enough bits.
assert(_lvHfaElemKind == elemKind);
#endif // FEATURE_HFA
}

Expand Down Expand Up @@ -1447,7 +1448,7 @@ struct fgArgTabEntry
bool _isSplit : 1; // True when this argument is split between the registers and OutArg area
#endif // FEATURE_ARG_SPLIT
#ifdef FEATURE_HFA
HfaElemKind _hfaElemKind : 2; // What kind of an HFA this is (HFA_ELEM_NONE if it is not an HFA).
CorInfoHFAElemType _hfaElemKind : 3; // What kind of an HFA this is (CORINFO_HFA_ELEM_NONE if it is not an HFA).
#endif

bool isLateArg()
Expand Down Expand Up @@ -1610,7 +1611,10 @@ struct fgArgTabEntry
if (!IsHfaArg())
{
// We haven't previously set this; do so now.
_hfaElemKind = HfaElemKindFromType(type);
CorInfoHFAElemType elemKind = HfaElemKindFromType(type);
_hfaElemKind = elemKind;
// Ensure we've allocated enough bits.
assert(_hfaElemKind == elemKind);
if (isPassedInRegisters())
{
numRegs = numHfaRegs;
Expand Down Expand Up @@ -2126,10 +2130,11 @@ class Compiler
#endif // ARM_SOFTFP

//-------------------------------------------------------------------------
// Functions to handle homogeneous floating-point aggregates (HFAs) in ARM.
// Functions to handle homogeneous floating-point aggregates (HFAs) in ARM/ARM64.
// HFAs are one to four element structs where each element is the same
// type, either all float or all double. They are treated specially
// in the ARM Procedure Call Standard, specifically, they are passed in
// type, either all float or all double. We handle HVAs (one to four elements of
// vector types) uniformly with HFAs. HFAs are treated specially
// in the ARM/ARM64 Procedure Call Standards, specifically, they are passed in
// floating-point registers instead of the general purpose registers.
//

Expand Down
12 changes: 4 additions & 8 deletions src/coreclr/src/jit/morph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -773,6 +773,7 @@ void fgArgTabEntry::Dump()
printf("fgArgTabEntry[arg %u", argNum);
printf(" %d.%s", GetNode()->gtTreeID, GenTree::OpName(GetNode()->OperGet()));
printf(" %s", varTypeName(argType));
printf(" (%s)", passedByRef ? "By ref" : "By value");
if (GetRegNum() != REG_STK)
{
printf(", %u reg%s:", numRegs, numRegs == 1 ? "" : "s");
Expand Down Expand Up @@ -1005,7 +1006,7 @@ fgArgTabEntry* fgArgInfo::AddRegArg(unsigned argNum,
curArgTabEntry->needPlace = false;
curArgTabEntry->processed = false;
#ifdef FEATURE_HFA
curArgTabEntry->_hfaElemKind = HFA_ELEM_NONE;
curArgTabEntry->_hfaElemKind = CORINFO_HFA_ELEM_NONE;
#endif
curArgTabEntry->isBackFilled = false;
curArgTabEntry->isNonStandard = false;
Expand Down Expand Up @@ -1087,7 +1088,7 @@ fgArgTabEntry* fgArgInfo::AddStkArg(unsigned argNum,
curArgTabEntry->needPlace = false;
curArgTabEntry->processed = false;
#ifdef FEATURE_HFA
curArgTabEntry->_hfaElemKind = HFA_ELEM_NONE;
curArgTabEntry->_hfaElemKind = CORINFO_HFA_ELEM_NONE;
#endif
curArgTabEntry->isBackFilled = false;
curArgTabEntry->isNonStandard = false;
Expand Down Expand Up @@ -4423,12 +4424,7 @@ GenTree* Compiler::fgMorphMultiregStructArg(GenTree* arg, fgArgTabEntry* fgEntry
)
{
// We have a HFA struct.
// Note that GetHfaType may not be the same as elemType, since TYP_SIMD8 is handled the same as TYP_DOUBLE.
var_types useElemType = elemType;
#if defined(TARGET_ARM64) & defined(FEATURE_SIMD)
useElemType = (elemType == TYP_SIMD8) ? TYP_DOUBLE : useElemType;
#endif // TARGET_ARM64 && FEATURE_SIMD
noway_assert(useElemType == varDsc->GetHfaType());
noway_assert(elemType == varDsc->GetHfaType());
noway_assert(elemSize == genTypeSize(elemType));
noway_assert(elemCount == (varDsc->lvExactSize / elemSize));
noway_assert(elemSize * elemCount == varDsc->lvExactSize);
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/src/tools/Common/JitInterface/CorInfoBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ unsafe partial class CorInfoImpl
[UnmanagedFunctionPointerAttribute(default(CallingConvention))]
delegate CORINFO_CLASS_STRUCT_* __getArgClass(IntPtr _this, IntPtr* ppException, CORINFO_SIG_INFO* sig, CORINFO_ARG_LIST_STRUCT_* args);
[UnmanagedFunctionPointerAttribute(default(CallingConvention))]
delegate CorInfoType __getHFAType(IntPtr _this, IntPtr* ppException, CORINFO_CLASS_STRUCT_* hClass);
delegate CorInfoHFAElemType __getHFAType(IntPtr _this, IntPtr* ppException, CORINFO_CLASS_STRUCT_* hClass);
[UnmanagedFunctionPointerAttribute(default(CallingConvention))]
delegate HRESULT __GetErrorHRESULT(IntPtr _this, IntPtr* ppException, _EXCEPTION_POINTERS* pExceptionPointers);
[UnmanagedFunctionPointerAttribute(default(CallingConvention))]
Expand Down Expand Up @@ -1743,7 +1743,7 @@ static CorInfoTypeWithMod _getArgType(IntPtr thisHandle, IntPtr* ppException, CO
}
}

static CorInfoType _getHFAType(IntPtr thisHandle, IntPtr* ppException, CORINFO_CLASS_STRUCT_* hClass)
static CorInfoHFAElemType _getHFAType(IntPtr thisHandle, IntPtr* ppException, CORINFO_CLASS_STRUCT_* hClass)
{
var _this = GetThis(thisHandle);
try
Expand All @@ -1753,7 +1753,7 @@ static CorInfoType _getHFAType(IntPtr thisHandle, IntPtr* ppException, CORINFO_C
catch (Exception ex)
{
*ppException = _this.AllocException(ex);
return default(CorInfoType);
return default(CorInfoHFAElemType);
}
}

Expand Down
Loading