Skip to content
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

Ensure maxSIMDStructBytes doesn't report compVerifyInstructionSetUnusable #85370

Merged
merged 1 commit into from
May 6, 2023
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
4 changes: 1 addition & 3 deletions docs/design/coreclr/botr/vectors-and-intrinsics.md
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,5 @@ While the above api exists, it is not expected that general purpose code within
|`compExactlyDependsOn(isa)`| Use when making a decision to use or not use an instruction set when the decision will affect the semantics of the generated code. Should never be used in an assert. | Return whether or not an instruction set is supported. Calls notifyInstructionSetUsage with the result of that computation.
|`compOpportunisticallyDependsOn(isa)`| Use when making an opportunistic decision to use or not use an instruction set. Use when the instruction set usage is a "nice to have optimization opportunity", but do not use when a false result may change the semantics of the program. Should never be used in an assert. | Return whether or not an instruction set is supported. Calls notifyInstructionSetUsage if the instruction set is supported.
|`compIsaSupportedDebugOnly(isa)` | Use to assert whether or not an instruction set is supported | Return whether or not an instruction set is supported. Does not report anything. Only available in debug builds.
|`getSIMDVectorType()`| Use to get the TYP of a the `Vector<T>` type. | Determine the TYP of the `Vector<T>` type. If on the architecture the TYP may vary depending on whatever rules, this function will make sufficient use of the `notifyInstructionSetUsage` api to ensure that the TYP is consistent between compile time and runtime.
|`getSIMDVectorRegisterByteLength()` | Use to get the size of a `Vector<T>` value. | Determine the size of the `Vector<T>` type. If on the architecture the size may vary depending on whatever rules, this function will make sufficient use of the `notifyInstructionSetUsage` api to ensure that the size is consistent between compile time and runtime.
|`getSIMDVectorRegisterByteLength()` | Use to get the size of a `Vector<T>` value. | Determine the size of the `Vector<T>` type. If on the architecture the size may vary depending on whatever rules. Use `compExactlyDependsOn` to perform the queries so that the size is consistent between compile time and runtime.
|`maxSIMDStructBytes()`| Get the maximum number of bytes that might be used in a SIMD type during this compilation. | Query the set of instruction sets supported, and determine the largest simd type supported. Use `compOpportunisticallyDependsOn` to perform the queries so that the maximum size needed is the only one recorded.
|`largestEnregisterableStructSize()`| Get the maximum number of bytes that might be represented by a single register in this compilation. Use only as an optimization to avoid calling `impNormStructType` or `getBaseTypeAndSizeOfSIMDType`. | Query the set of instruction sets supported, and determine the largest simd type supported in this compilation, report that size.
73 changes: 2 additions & 71 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -8633,29 +8633,6 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
bool areArgumentsContiguous(GenTree* op1, GenTree* op2);
GenTree* CreateAddressNodeForSimdHWIntrinsicCreate(GenTree* tree, var_types simdBaseType, unsigned simdSize);

// Get the type for the hardware SIMD vector.
// This is the maximum SIMD type supported for this target.
var_types getSIMDVectorType()
{
#if defined(TARGET_XARCH)
if (compOpportunisticallyDependsOn(InstructionSet_AVX2))
{
// TODO-XArch-AVX512 : Return TYP_SIMD64 once Vector<T> supports AVX512.
return TYP_SIMD32;
}
else
{
compVerifyInstructionSetUnusable(InstructionSet_AVX2);
return TYP_SIMD16;
}
#elif defined(TARGET_ARM64)
return TYP_SIMD16;
#else
assert(!"getSIMDVectorType() unimplemented on target arch");
unreached();
#endif
}

// Get the size of the SIMD type in bytes
int getSIMDTypeSizeInBytes(CORINFO_CLASS_HANDLE typeHnd)
{
Expand All @@ -8678,14 +8655,13 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
unsigned getSIMDVectorRegisterByteLength()
{
#if defined(TARGET_XARCH)
if (compOpportunisticallyDependsOn(InstructionSet_AVX2))
if (compExactlyDependsOn(InstructionSet_AVX2))
{
// TODO-XArch-AVX512 : Return ZMM_REGSIZE_BYTES once Vector<T> supports AVX512.
return YMM_REGSIZE_BYTES;
}
else
{
compVerifyInstructionSetUnusable(InstructionSet_AVX2);
return XMM_REGSIZE_BYTES;
}
#elif defined(TARGET_ARM64)
Expand Down Expand Up @@ -8716,13 +8692,11 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
}
else
{
compVerifyInstructionSetUnusable(InstructionSet_AVX512F);
return YMM_REGSIZE_BYTES;
}
}
else
{
compVerifyInstructionSetUnusable(InstructionSet_AVX);
return XMM_REGSIZE_BYTES;
}
#elif defined(TARGET_ARM64)
Expand Down Expand Up @@ -8952,44 +8926,12 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
return threshold;
}

//------------------------------------------------------------------------
// largestEnregisterableStruct: The size in bytes of the largest struct that can be enregistered.
//
// Notes: It is not guaranteed that the struct of this size or smaller WILL be a
// candidate for enregistration.

unsigned largestEnregisterableStructSize()
{
#ifdef FEATURE_SIMD
#if defined(FEATURE_HW_INTRINSICS) && defined(TARGET_XARCH)
if (opts.IsReadyToRun())
{
// Return constant instead of maxSIMDStructBytes, as maxSIMDStructBytes performs
// checks that are effected by the current level of instruction set support would
// otherwise cause the highest level of instruction set support to be reported to crossgen2.
// and this api is only ever used as an optimization or assert, so no reporting should
// ever happen.
return ZMM_REGSIZE_BYTES;
}
#endif // defined(FEATURE_HW_INTRINSICS) && defined(TARGET_XARCH)
unsigned vectorRegSize = maxSIMDStructBytes();
assert(vectorRegSize >= TARGET_POINTER_SIZE);
return vectorRegSize;
#else // !FEATURE_SIMD
return TARGET_POINTER_SIZE;
#endif // !FEATURE_SIMD
}

// Use to determine if a struct *might* be a SIMD type. As this function only takes a size, many
// structs will fit the criteria.
bool structSizeMightRepresentSIMDType(size_t structSize)
{
#ifdef FEATURE_SIMD
// Do not use maxSIMDStructBytes as that api in R2R on X86 and X64 may notify the JIT
// about the size of a struct under the assumption that the struct size needs to be recorded.
// By using largestEnregisterableStructSize here, the detail of whether or not Vector256<T> is
// enregistered or not will not be messaged to the R2R compiler.
return (structSize >= minSIMDStructBytes()) && (structSize <= largestEnregisterableStructSize());
return (structSize >= minSIMDStructBytes()) && (structSize <= maxSIMDStructBytes());
#else
return false;
#endif // FEATURE_SIMD
Expand Down Expand Up @@ -9088,17 +9030,6 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
#endif
}

// Ensure that code will not execute if an instruction set is usable. Call only
// if the instruction set has previously reported as unusable, but the status
// has not yet been recorded to the AOT compiler.
void compVerifyInstructionSetUnusable(CORINFO_InstructionSet isa) const
{
// use compExactlyDependsOn to capture are record the use of the ISA.
bool isaUsable = compExactlyDependsOn(isa);
// Assert that the is unusable. If true, this function should never be called.
assert(!isaUsable);
}

// Answer the question: Is a particular ISA allowed to be used implicitly by optimizations?
// The result of this api call will match the target machine if the result is true.
// If the result is false, then the target machine may have support for the instruction.
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/lclvars.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2241,7 +2241,7 @@ Compiler::lvaStructFieldInfo Compiler::StructPromotionHelper::GetFieldInfo(CORIN
// We will only promote fields of SIMD types that fit into a SIMD register.
if (simdBaseJitType != CORINFO_TYPE_UNDEF)
{
if ((simdSize >= compiler->minSIMDStructBytes()) && (simdSize <= compiler->maxSIMDStructBytes()))
if (compiler->structSizeMightRepresentSIMDType(simdSize))
{
fieldInfo.fldType = compiler->getSIMDTypeForSize(simdSize);
fieldInfo.fldSize = simdSize;
Expand Down