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

Fix various asserts that were found by Antigen #104625

Merged
merged 11 commits into from
Jul 11, 2024
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:
Comment on lines +4215 to +4218
Copy link
Member Author

Choose a reason for hiding this comment

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

These return TYP_SIMD64 but take in TYP_SIMD32 and so we generated wrong disassembly and asserted when validating the alignment of the memory operand.

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
Loading