Skip to content

Commit

Permalink
Ensure getMaxSIMDStructBytes doesn't report `compVerifyInstructionSet…
Browse files Browse the repository at this point in the history 
…Unusable` (dotnet#85370)
  • Loading branch information
@tannergooding
tannergooding committed May 6, 2023
1 parent ae1be12 commit 7bd4666
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 75 deletions.
4 changes: 1 addition & 3 deletions docs/design/coreclr/botr/vectors-and-intrinsics.md
View file
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
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8656,29 +8656,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 @@ -8701,14 +8678,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 @@ -8739,13 +8715,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 @@ -8969,44 +8943,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 @@ -9105,17 +9047,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
View file
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

0 comments on commit 7bd4666

Please sign in to comment.