Skip to content

Commit

Permalink
Fix various asserts that were found by Antigen (#104625)
Browse files Browse the repository at this point in the history
* Ensure that we create a valid GT_IND node

* Remove a bad assert in the associative morphs

* Ensure that we check for GT_NULLCHECK when handling containment

* Fix a bad assert for x64

* Ensure we properly check for ConvertMaskToVector

* Fix the memory size used for some vbroadcast instructions in disasm and asserts

* Ensure rewriting WithElement takes into account unsupported ISAs

* Ensure we check FEATURE_HW_INTRINSICS

* Apply formatting patch
  • Loading branch information
tannergooding committed Jul 11, 2024
1 parent da8a603 commit 8ba8249
Show file tree
Hide file tree
Showing 7 changed files with 60 additions and 36 deletions.
4 changes: 4 additions & 0 deletions src/coreclr/jit/emit.h
Original file line number Diff line number Diff line change
Expand Up @@ -4212,6 +4212,10 @@ emitAttr emitter::emitGetBaseMemOpSize(instrDesc* id) const
return EA_16BYTE;
}

case INS_vbroadcastf32x8:
case INS_vbroadcasti32x8:
case INS_vbroadcasti64x4:
case INS_vbroadcastf64x4:
case INS_vextractf32x8:
case INS_vextracti32x8:
case INS_vextractf64x4:
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30706,7 +30706,7 @@ GenTree* Compiler::gtFoldExprHWIntrinsic(GenTreeHWIntrinsic* tree)

case NI_Vector256_ToVector512:
{
assert(retType == TYP_SIMD32);
assert(retType == TYP_SIMD64);
assert(cnsNode->gtType == TYP_SIMD32);
cnsNode->AsVecCon()->gtSimd64Val.v256[1] = {};

Expand Down
48 changes: 26 additions & 22 deletions src/coreclr/jit/gentree.h
Original file line number Diff line number Diff line change
Expand Up @@ -1640,6 +1640,32 @@ struct GenTree

bool OperIsHWIntrinsic(NamedIntrinsic intrinsicId) const;

bool OperIsConvertMaskToVector() const
{
#if defined(FEATURE_HW_INTRINSICS)
#if defined(TARGET_XARCH)
return OperIsHWIntrinsic(NI_EVEX_ConvertMaskToVector);
#elif defined(TARGET_ARM64)
return OperIsHWIntrinsic(NI_Sve_ConvertMaskToVector);
#endif // !TARGET_XARCH && !TARGET_ARM64
#else
return false;
#endif // FEATURE_HW_INTRINSICS
}

bool OperIsConvertVectorToMask() const
{
#if defined(FEATURE_HW_INTRINSICS)
#if defined(TARGET_XARCH)
return OperIsHWIntrinsic(NI_EVEX_ConvertVectorToMask);
#elif defined(TARGET_ARM64)
return OperIsHWIntrinsic(NI_Sve_ConvertVectorToMask);
#endif // !TARGET_XARCH && !TARGET_ARM64
#else
return false;
#endif // FEATURE_HW_INTRINSICS
}

// This is here for cleaner GT_LONG #ifdefs.
static bool OperIsLong(genTreeOps gtOper)
{
Expand Down Expand Up @@ -6499,28 +6525,6 @@ struct GenTreeHWIntrinsic : public GenTreeJitIntrinsic
bool OperIsBitwiseHWIntrinsic() const;
bool OperIsEmbRoundingEnabled() const;

bool OperIsConvertMaskToVector() const
{
#if defined(TARGET_XARCH)
return GetHWIntrinsicId() == NI_EVEX_ConvertMaskToVector;
#elif defined(TARGET_ARM64)
return GetHWIntrinsicId() == NI_Sve_ConvertMaskToVector;
#else
return false;
#endif // TARGET_ARM64 && FEATURE_MASKED_HW_INTRINSICS
}

bool OperIsConvertVectorToMask() const
{
#if defined(TARGET_XARCH)
return GetHWIntrinsicId() == NI_EVEX_ConvertVectorToMask;
#elif defined(TARGET_ARM64)
return GetHWIntrinsicId() == NI_Sve_ConvertVectorToMask;
#else
return false;
#endif
}

bool OperRequiresAsgFlag() const;
bool OperRequiresCallFlag() const;
bool OperRequiresGlobRefFlag() const;
Expand Down
12 changes: 7 additions & 5 deletions src/coreclr/jit/lowerarmarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1264,9 +1264,10 @@ GenTree* Lowering::LowerHWIntrinsic(GenTreeHWIntrinsic* node)

if (isContainableMemory || !op2->OperIsConst())
{
unsigned simdSize = node->GetSimdSize();
var_types simdBaseType = node->GetSimdBaseType();
var_types simdType = Compiler::getSIMDTypeForSize(simdSize);
unsigned simdSize = node->GetSimdSize();
CorInfoType simdBaseJitType = node->GetSimdBaseJitType();
var_types simdBaseType = node->GetSimdBaseType();
var_types simdType = Compiler::getSIMDTypeForSize(simdSize);

// We're either already loading from memory or we need to since
// we don't know what actual index is going to be retrieved.
Expand Down Expand Up @@ -1355,7 +1356,7 @@ GenTree* Lowering::LowerHWIntrinsic(GenTreeHWIntrinsic* node)
}

// Finally we can indirect the memory address to get the actual value
GenTreeIndir* indir = comp->gtNewIndir(simdBaseType, addr);
GenTreeIndir* indir = comp->gtNewIndir(JITtype2varType(simdBaseJitType), addr);
BlockRange().InsertBefore(node, indir);

LIR::Use use;
Expand Down Expand Up @@ -2339,7 +2340,8 @@ void Lowering::ContainCheckIndir(GenTreeIndir* indirNode)
MakeSrcContained(indirNode, addr);
}
}
else if (addr->OperIs(GT_LCL_ADDR) && IsContainableLclAddr(addr->AsLclFld(), indirNode->Size()))
else if (addr->OperIs(GT_LCL_ADDR) && !indirNode->OperIs(GT_NULLCHECK) &&
IsContainableLclAddr(addr->AsLclFld(), indirNode->Size()))
{
// These nodes go into an addr mode:
// - GT_LCL_ADDR is a stack addr mode.
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/lowerxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3063,7 +3063,7 @@ GenTree* Lowering::LowerHWIntrinsicCndSel(GenTreeHWIntrinsic* node)
// Next, determine if the target architecture supports BlendVariable
NamedIntrinsic blendVariableId = NI_Illegal;

bool isOp1CvtMaskToVector = op1->AsHWIntrinsic()->OperIsConvertMaskToVector();
bool isOp1CvtMaskToVector = op1->OperIsConvertMaskToVector();

if ((simdSize == 64) || isOp1CvtMaskToVector)
{
Expand Down
8 changes: 1 addition & 7 deletions src/coreclr/jit/morph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9977,7 +9977,7 @@ GenTree* Compiler::fgOptimizeHWIntrinsic(GenTreeHWIntrinsic* node)
// We need both operands to be ConvertMaskToVector in
// order to optimize this to a direct mask operation

if (!op1->OperIsHWIntrinsic())
if (!op1->OperIsConvertMaskToVector())
{
break;
}
Expand All @@ -10003,11 +10003,6 @@ GenTree* Compiler::fgOptimizeHWIntrinsic(GenTreeHWIntrinsic* node)
GenTreeHWIntrinsic* cvtOp1 = op1->AsHWIntrinsic();
GenTreeHWIntrinsic* cvtOp2 = op2->AsHWIntrinsic();

if (!cvtOp1->OperIsConvertMaskToVector())
{
break;
}

if (!cvtOp2->OperIsConvertMaskToVector())
{
break;
Expand Down Expand Up @@ -10448,7 +10443,6 @@ GenTree* Compiler::fgOptimizeHWIntrinsicAssociative(GenTreeHWIntrinsic* tree)
{
return nullptr;
}
assert(intrinOp1->GetHWIntrinsicId() == intrinsicId);

if (needsMatchingBaseType && (intrinOp1->GetSimdBaseType() != simdBaseType))
{
Expand Down
20 changes: 20 additions & 0 deletions src/coreclr/jit/rationalize.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,26 @@ void Rationalizer::RewriteHWIntrinsicAsUserCall(GenTree** use, ArrayStack<GenTre
break;
}

#if defined(TARGET_XARCH)
if (varTypeIsIntegral(simdBaseType))
{
if (varTypeIsLong(simdBaseType))
{
if (!comp->compOpportunisticallyDependsOn(InstructionSet_SSE41_X64))
{
break;
}
}
else if (!varTypeIsShort(simdBaseType))
{
if (!comp->compOpportunisticallyDependsOn(InstructionSet_SSE41))
{
break;
}
}
}
#endif // TARGET_XARCH

result = comp->gtNewSimdWithElementNode(retType, op1, op2, op3, simdBaseJitType, simdSize);
break;
}
Expand Down

0 comments on commit 8ba8249

Please sign in to comment.