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.

Sign up for GitHub

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

Share more of the TYP_MASK handling and support rewriting TYP_MASK operands in rationalization #103288

Merged
merged 6 commits into from
Jun 12, 2024
Merged

Share more of the TYP_MASK handling and support rewriting TYP_MASK operands in rationalization #103288

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fa83743
Share more of the TYP_MASK handling and support rewriting TYP_MASK op…
tannergooding Jun 11, 2024
e56c756
Ensure we pass in TYP_MASK, not the simdType
tannergooding Jun 11, 2024
d9c7c68
Apply formatting patch
tannergooding Jun 11, 2024
cbc5213
Fix copy/paste error, pass in clsHnd for the argument
tannergooding Jun 11, 2024
84657f9
Ensure that we normalize sigType before inserting the CvtMaskToVector…
tannergooding Jun 11, 2024
7241d3f
Ensure that we get the vector node on Arm64 (ConvertVectorToMask has …
tannergooding Jun 12, 2024
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
14 changes: 8 additions & 6 deletions src/coreclr/jit/compiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3185,6 +3185,10 @@ class Compiler
GenTree* gtNewSimdAbsNode(
var_types type, GenTree* op1, CorInfoType simdBaseJitType, unsigned simdSize);

#if defined(TARGET_ARM64)
GenTree* gtNewSimdAllTrueMaskNode(CorInfoType simdBaseJitType, unsigned simdSize);
#endif

GenTree* gtNewSimdBinOpNode(genTreeOps op,
var_types type,
GenTree* op1,
Expand Down Expand Up @@ -3223,6 +3227,8 @@ class Compiler
CorInfoType simdBaseJitType,
unsigned simdSize);

GenTree* gtNewSimdCvtMaskToVectorNode(var_types type, GenTree* op1, CorInfoType simdBaseJitType, unsigned simdSize);

GenTree* gtNewSimdCvtNode(var_types type,
GenTree* op1,
CorInfoType simdTargetBaseJitType,
Expand All @@ -3235,6 +3241,8 @@ class Compiler
CorInfoType simdSourceBaseJitType,
unsigned simdSize);

GenTree* gtNewSimdCvtVectorToMaskNode(var_types type, GenTree* op1, CorInfoType simdBaseJitType, unsigned simdSize);

GenTree* gtNewSimdCreateBroadcastNode(
var_types type, GenTree* op1, CorInfoType simdBaseJitType, unsigned simdSize);

Expand Down Expand Up @@ -3516,12 +3524,6 @@ class Compiler

GenTreeIndir* gtNewMethodTableLookup(GenTree* obj);

#if defined(TARGET_ARM64)
GenTree* gtNewSimdConvertVectorToMaskNode(var_types type, GenTree* node, CorInfoType simdBaseJitType, unsigned simdSize);
GenTree* gtNewSimdConvertMaskToVectorNode(GenTreeHWIntrinsic* node, var_types type);
GenTree* gtNewSimdAllTrueMaskNode(CorInfoType simdBaseJitType, unsigned simdSize);
#endif

//------------------------------------------------------------------------
// Other GenTree functions

Expand Down
84 changes: 77 additions & 7 deletions src/coreclr/jit/gentree.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21530,6 +21530,35 @@ GenTree* Compiler::gtNewSimdCeilNode(var_types type, GenTree* op1, CorInfoType s
return gtNewSimdHWIntrinsicNode(type, op1, intrinsic, simdBaseJitType, simdSize);
}

//------------------------------------------------------------------------
// gtNewSimdCvtMaskToVectorNode: Convert a HW instrinsic mask node to a vector
//
// Arguments:
// type -- The type of the node to convert to
// op1 -- The node to convert
// simdBaseJitType -- the base jit type of the converted node
// simdSize -- the simd size of the converted node
//
// Return Value:
// The node converted to the given type
//
GenTree* Compiler::gtNewSimdCvtMaskToVectorNode(var_types type,
GenTree* op1,
CorInfoType simdBaseJitType,
unsigned simdSize)
{
assert(varTypeIsMask(op1));
assert(varTypeIsSIMD(type));

#if defined(TARGET_XARCH)
return gtNewSimdHWIntrinsicNode(type, op1, NI_EVEX_ConvertMaskToVector, simdBaseJitType, simdSize);
#elif defined(TARGET_ARM64)
return gtNewSimdHWIntrinsicNode(type, op1, NI_Sve_ConvertMaskToVector, simdBaseJitType, simdSize);
#else
#error Unsupported platform
#endif // !TARGET_XARCH && !TARGET_ARM64
}

GenTree* Compiler::gtNewSimdCvtNode(var_types type,
GenTree* op1,
CorInfoType simdTargetBaseJitType,
Expand Down Expand Up @@ -21892,6 +21921,37 @@ GenTree* Compiler::gtNewSimdCvtNativeNode(var_types type,
return gtNewSimdHWIntrinsicNode(type, op1, hwIntrinsicID, simdSourceBaseJitType, simdSize);
}

//------------------------------------------------------------------------
// gtNewSimdCvtVectorToMaskNode: Convert a HW instrinsic vector node to a mask
//
// Arguments:
// type -- The type of the mask to produce.
// op1 -- The node to convert
// simdBaseJitType -- the base jit type of the converted node
// simdSize -- the simd size of the converted node
//
// Return Value:
// The node converted to the a mask type
//
GenTree* Compiler::gtNewSimdCvtVectorToMaskNode(var_types type,
GenTree* op1,
CorInfoType simdBaseJitType,
unsigned simdSize)
{
assert(varTypeIsMask(type));
Copy link
Contributor

@TIHan TIHan Jun 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Do we need to pass a TYP_MASK for type every time we call gtNewSimdCvtVectorToMaskNode ? Since we only have one TYP_MASK, feels like we could just get rid of this parameter.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's here to support any future expansion desired. For example, it may be important to differentiate mask based on size.

assert(varTypeIsSIMD(op1));

#if defined(TARGET_XARCH)
return gtNewSimdHWIntrinsicNode(TYP_MASK, op1, NI_EVEX_ConvertVectorToMask, simdBaseJitType, simdSize);
#elif defined(TARGET_ARM64)
// We use cmpne which requires an embedded mask.
GenTree* trueMask = gtNewSimdAllTrueMaskNode(simdBaseJitType, simdSize);
return gtNewSimdHWIntrinsicNode(TYP_MASK, trueMask, op1, NI_Sve_ConvertVectorToMask, simdBaseJitType, simdSize);
#else
#error Unsupported platform
#endif // !TARGET_XARCH && !TARGET_ARM64
}

GenTree* Compiler::gtNewSimdCmpOpNode(
genTreeOps op, var_types type, GenTree* op1, GenTree* op2, CorInfoType simdBaseJitType, unsigned simdSize)
{
Expand Down Expand Up @@ -22569,19 +22629,15 @@ GenTree* Compiler::gtNewSimdCmpOpNode(

assert(intrinsic != NI_Illegal);

#if defined(TARGET_XARCH)
if (needsConvertMaskToVector)
{
GenTree* retNode = gtNewSimdHWIntrinsicNode(TYP_MASK, op1, op2, intrinsic, simdBaseJitType, simdSize);
return gtNewSimdHWIntrinsicNode(type, retNode, NI_EVEX_ConvertMaskToVector, simdBaseJitType, simdSize);
return gtNewSimdCvtMaskToVectorNode(type, retNode, simdBaseJitType, simdSize);
}
else
{
return gtNewSimdHWIntrinsicNode(type, op1, op2, intrinsic, simdBaseJitType, simdSize);
}
#else
return gtNewSimdHWIntrinsicNode(type, op1, op2, intrinsic, simdBaseJitType, simdSize);
#endif
}

GenTree* Compiler::gtNewSimdCmpOpAllNode(
Expand Down Expand Up @@ -27157,6 +27213,20 @@ bool GenTreeHWIntrinsic::OperIsCreateScalarUnsafe() const
}
}

//------------------------------------------------------------------------
// OperIsBitwiseHWIntrinsic: Is the operation a bitwise logic operation.
//
// Arguments:
// oper -- The operation to check
//
// Return Value:
// Whether oper is a bitwise logic intrinsic node.
//
bool GenTreeHWIntrinsic::OperIsBitwiseHWIntrinsic(genTreeOps oper)
{
return (oper == GT_AND) || (oper == GT_AND_NOT) || (oper == GT_OR) || (oper == GT_XOR);
}

//------------------------------------------------------------------------
// OperIsBitwiseHWIntrinsic: Is this HWIntrinsic a bitwise logic intrinsic node.
//
Expand All @@ -27165,8 +27235,8 @@ bool GenTreeHWIntrinsic::OperIsCreateScalarUnsafe() const
//
bool GenTreeHWIntrinsic::OperIsBitwiseHWIntrinsic() const
{
genTreeOps Oper = HWOperGet();
return Oper == GT_AND || Oper == GT_OR || Oper == GT_XOR || Oper == GT_AND_NOT;
genTreeOps oper = HWOperGet();
return OperIsBitwiseHWIntrinsic(oper);
}

//------------------------------------------------------------------------
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/jit/gentree.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6505,6 +6505,8 @@ struct GenTreeHWIntrinsic : public GenTreeJitIntrinsic
}
#endif

static bool OperIsBitwiseHWIntrinsic(genTreeOps oper);

bool OperIsMemoryLoad(GenTree** pAddr = nullptr) const;
bool OperIsMemoryStore(GenTree** pAddr = nullptr) const;
bool OperIsMemoryLoadOrStore() const;
Expand Down
13 changes: 9 additions & 4 deletions src/coreclr/jit/hwintrinsic.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1657,15 +1657,14 @@ GenTree* Compiler::impHWIntrinsic(NamedIntrinsic intrinsic,
// HWInstrinsic requires a mask for op2
if (!varTypeIsMask(op2))
{
retNode->AsHWIntrinsic()->Op(2) =
gtNewSimdConvertVectorToMaskNode(retType, op2, simdBaseJitType, simdSize);
retNode->AsHWIntrinsic()->Op(2) = gtNewSimdCvtVectorToMaskNode(TYP_MASK, op2, simdBaseJitType, simdSize);
Copy link
Member

Choose a reason for hiding this comment

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

probably add an assert in gtNewSimdCvtVectorToMaskNode() that type == TYP_MASK?

Copy link
Member Author

Choose a reason for hiding this comment

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

That assert already exists and is what caught the failure.

}
}

if (!varTypeIsMask(op1))
{
// Op1 input is a vector. HWInstrinsic requires a mask.
retNode->AsHWIntrinsic()->Op(1) = gtNewSimdConvertVectorToMaskNode(retType, op1, simdBaseJitType, simdSize);
retNode->AsHWIntrinsic()->Op(1) = gtNewSimdCvtVectorToMaskNode(TYP_MASK, op1, simdBaseJitType, simdSize);
}

if (HWIntrinsicInfo::IsMultiReg(intrinsic))
Expand All @@ -1682,7 +1681,13 @@ GenTree* Compiler::impHWIntrinsic(NamedIntrinsic intrinsic,
// HWInstrinsic returns a mask, but all returns must be vectors, so convert mask to vector.
assert(HWIntrinsicInfo::ReturnsPerElementMask(intrinsic));
assert(nodeRetType == TYP_MASK);
retNode = gtNewSimdConvertMaskToVectorNode(retNode->AsHWIntrinsic(), retType);

GenTreeHWIntrinsic* op = retNode->AsHWIntrinsic();

CorInfoType simdBaseJitType = op->GetSimdBaseJitType();
unsigned simdSize = op->GetSimdSize();

retNode = gtNewSimdCvtMaskToVectorNode(retType, op, simdBaseJitType, simdSize);
}
#endif // defined(TARGET_ARM64)

Expand Down
44 changes: 1 addition & 43 deletions src/coreclr/jit/hwintrinsicarm64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2582,7 +2582,7 @@ GenTree* Compiler::impSpecialIntrinsic(NamedIntrinsic intrinsic,
// HWInstrinsic requires a mask for op2
if (!varTypeIsMask(op2))
{
op2 = gtNewSimdConvertVectorToMaskNode(retType, op2, simdBaseJitType, simdSize);
op2 = gtNewSimdCvtVectorToMaskNode(TYP_MASK, op2, simdBaseJitType, simdSize);
}

retNode = gtNewSimdHWIntrinsicNode(retType, op1, op2, intrinsic, simdBaseJitType, simdSize);
Expand All @@ -2603,48 +2603,6 @@ GenTree* Compiler::impSpecialIntrinsic(NamedIntrinsic intrinsic,
return retNode;
}

//------------------------------------------------------------------------
// gtNewSimdConvertMaskToVectorNode: Convert a HW instrinsic vector node to a mask
//
// Arguments:
// node -- The node to convert
// simdBaseJitType -- the base jit type of the converted node
// simdSize -- the simd size of the converted node
//
// Return Value:
// The node converted to the a mask type
//
GenTree* Compiler::gtNewSimdConvertVectorToMaskNode(var_types type,
GenTree* node,
CorInfoType simdBaseJitType,
unsigned simdSize)
{
assert(varTypeIsSIMD(node));

// ConvertVectorToMask uses cmpne which requires an embedded mask.
GenTree* trueMask = gtNewSimdAllTrueMaskNode(simdBaseJitType, simdSize);
return gtNewSimdHWIntrinsicNode(TYP_MASK, trueMask, node, NI_Sve_ConvertVectorToMask, simdBaseJitType, simdSize);
}

//------------------------------------------------------------------------
// gtNewSimdConvertMaskToVectorNode: Convert a HW instrinsic mask node to a vector
//
// Arguments:
// node -- The node to convert
// type -- The type of the node to convert to
//
// Return Value:
// The node converted to the given type
//
GenTree* Compiler::gtNewSimdConvertMaskToVectorNode(GenTreeHWIntrinsic* node, var_types type)
{
assert(varTypeIsMask(node));
assert(varTypeIsSIMD(type));

return gtNewSimdHWIntrinsicNode(type, node, NI_Sve_ConvertMaskToVector, node->GetSimdBaseJitType(),
node->GetSimdSize());
}

//------------------------------------------------------------------------
// gtNewSimdEmbeddedMaskNode: Create an embedded mask
//
Expand Down
Loading
Loading