Skip to content

Fix SIMD MinMax constant special cases #133173

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

Merged
tannergooding merged 3 commits into from
Sep 4, 2026
Merged

Fix SIMD MinMax constant special cases #133173

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f3bc6de
Fix SIMD MinMax constant special cases
tannergooding Sep 3, 2026
633bdea
Only use the single constant min/max path when the fixup can express …
tannergooding Sep 3, 2026
8dcd25b
Cover signed NaNs in vector min/max tests
tannergooding Sep 4, 2026
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
168 changes: 115 additions & 53 deletions src/coreclr/jit/gentree.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20943,23 +20943,32 @@ bool GenTreeVecCon::IsBroadcast(var_types simdBaseType) const
bool GenTreeVecCon::IsNaN(var_types simdBaseType) const
{
assert(varTypeIsFloating(simdBaseType));
uint32_t elementCount = ElementCount(genTypeSize(gtType), simdBaseType);

for (uint32_t i = 0; i < elementCount; i++)
{
double element = GetElementFloating(simdBaseType, i);
unsigned simdSize = genTypeSize(gtType);
simd_t result = EvaluateSimdIsNaN(simdBaseType, gtSimdVal, simdSize);
return EvaluateSimdAllWhereAllBitsSet(simdBaseType, result, simdSize);
}

if (!FloatingPointUtils::isNaN(element))
{
return false;
}
}
//------------------------------------------------------------------------
// GenTreeVecCon::ContainsNaN: Determines if this vector constant contains a NaN
//
// Arguments:
// simdBaseType - the base type of the constant being checked
//
// Returns:
// true if any element is NaN; otherwise, false
//
bool GenTreeVecCon::ContainsNaN(var_types simdBaseType) const
{
assert(varTypeIsFloating(simdBaseType));

return true;
unsigned simdSize = genTypeSize(gtType);
simd_t result = EvaluateSimdIsNaN(simdBaseType, gtSimdVal, simdSize);
return EvaluateSimdAnyWhereAllBitsSet(simdBaseType, result, simdSize);
}

//------------------------------------------------------------------------
// GenTreeVecCon::IsNaN: Determines if this vector constant has all elements being -0
// GenTreeVecCon::IsNegativeZero: Determines if this vector constant has all elements being -0
//
// Arguments:
// simdBaseType - the base type of the constant being checked
Expand All @@ -20970,19 +20979,46 @@ bool GenTreeVecCon::IsNaN(var_types simdBaseType) const
bool GenTreeVecCon::IsNegativeZero(var_types simdBaseType) const
{
assert(varTypeIsFloating(simdBaseType));
uint32_t elementCount = ElementCount(genTypeSize(gtType), simdBaseType);

for (uint32_t i = 0; i < elementCount; i++)
{
double element = GetElementFloating(simdBaseType, i);
unsigned simdSize = genTypeSize(gtType);
simd_t result = EvaluateSimdIsNegativeZero(simdBaseType, gtSimdVal, simdSize);
return EvaluateSimdAllWhereAllBitsSet(simdBaseType, result, simdSize);
}

if (!FloatingPointUtils::isNegativeZero(element))
{
return false;
}
}
//------------------------------------------------------------------------
// GenTreeVecCon::ContainsNegativeZero: Determines if this vector constant contains -0
//
// Arguments:
// simdBaseType - the base type of the constant being checked
//
// Returns:
// true if any element is -0; otherwise, false
//
bool GenTreeVecCon::ContainsNegativeZero(var_types simdBaseType) const
{
assert(varTypeIsFloating(simdBaseType));

return true;
unsigned simdSize = genTypeSize(gtType);
simd_t result = EvaluateSimdIsNegativeZero(simdBaseType, gtSimdVal, simdSize);
return EvaluateSimdAnyWhereAllBitsSet(simdBaseType, result, simdSize);
}

//------------------------------------------------------------------------
// GenTreeVecCon::ContainsPositiveZero: Determines if this vector constant contains +0
//
// Arguments:
// simdBaseType - the base type of the constant being checked
//
// Returns:
// true if any element is +0; otherwise, false
//
bool GenTreeVecCon::ContainsPositiveZero(var_types simdBaseType) const
{
assert(varTypeIsFloating(simdBaseType));

unsigned simdSize = genTypeSize(gtType);
simd_t result = EvaluateSimdIsPositiveZero(simdBaseType, gtSimdVal, simdSize);
return EvaluateSimdAnyWhereAllBitsSet(simdBaseType, result, simdSize);
}

#if defined(FEATURE_MASKED_HW_INTRINSICS)
Expand Down Expand Up @@ -26502,8 +26538,15 @@ GenTree* Compiler::gtNewSimdMinMaxNode(var_types type,

if (!isMagnitude)
{
bool needsFixup = false;
bool canHandle = false;
// xarch min/max return op2 if both inputs are 0 of either sign or if either input
Comment thread
tannergooding marked this conversation as resolved.
// is NaN. We can exploit that to get the IEEE 754 behavior for free by ordering
// the operands such that the constant is the one that gets returned.

// Partially NaN constants cannot use operand ordering, while mixed zero constants
// require the per-element fixup below.
bool hasPartialNaN = !isScalar && cnsNode->AsVecCon()->ContainsNaN(simdBaseType);
bool needsFixup = false;
bool canHandle = false;
Comment thread
tannergooding marked this conversation as resolved.

if (isMax)
{
Expand All @@ -26512,10 +26555,10 @@ GenTree* Compiler::gtNewSimdMinMaxNode(var_types type,
// not be propagated for isNumber and to be propagated otherwise.
//
// This means for isNumber we want to do `max other, cns` and
// can only handle cns being -0 if Avx512F is supported. This is
// because if other was NaN, we want to return the non-NaN cns.
// But if cns was -0 and other was +0 we'd want to return +0 and
// so need to be able to fixup the result.
// cannot handle cns being -0. If other was NaN, we want to return
// the non-NaN cns. But if cns was -0 and other was +0 we'd want
// to return +0, and the ZERO fixup token cannot distinguish the
// opaque operand's sign.
//
// For !isNumber we have the inverse and want `max cns, other` and
// can only handle cns being +0 if Avx512F is supported. This is
Expand All @@ -26531,7 +26574,7 @@ GenTree* Compiler::gtNewSimdMinMaxNode(var_types type,
}
else
{
needsFixup = cnsNode->IsVectorNegativeZero(simdBaseType);
needsFixup |= cnsNode->AsVecCon()->ContainsNegativeZero(simdBaseType);
}
}
else if (isScalar)
Expand All @@ -26540,10 +26583,11 @@ GenTree* Compiler::gtNewSimdMinMaxNode(var_types type,
}
else
{
needsFixup = cnsNode->IsVectorZero();
needsFixup |= cnsNode->AsVecCon()->ContainsPositiveZero(simdBaseType);
}

if (!needsFixup || compOpportunisticallyDependsOn(InstructionSet_AVX512))
if (!hasPartialNaN &&
(!needsFixup || (!isNumber && compOpportunisticallyDependsOn(InstructionSet_AVX512))))
{
// Given the checks, op1 can safely be the cns and op2 the other node

Expand All @@ -26562,10 +26606,10 @@ GenTree* Compiler::gtNewSimdMinMaxNode(var_types type,
// not be propagated for isNumber and to be propagated otherwise.
//
// This means for isNumber we want to do `min other, cns` and
// can only handle cns being +0 if Avx512F is supported. This is
// because if other was NaN, we want to return the non-NaN cns.
// But if cns was +0 and other was -0 we'd want to return -0 and
// so need to be able to fixup the result.
// cannot handle cns being +0. If other was NaN, we want to return
// the non-NaN cns. But if cns was +0 and other was -0 we'd want
// to return -0, and the ZERO fixup token cannot distinguish the
// opaque operand's sign.
//
// For !isNumber we have the inverse and want `min cns, other` and
// can only handle cns being -0 if Avx512F is supported. This is
Expand All @@ -26581,7 +26625,7 @@ GenTree* Compiler::gtNewSimdMinMaxNode(var_types type,
}
else
{
needsFixup = cnsNode->IsVectorZero();
needsFixup |= cnsNode->AsVecCon()->ContainsPositiveZero(simdBaseType);
}
}
else if (isScalar)
Expand All @@ -26590,10 +26634,11 @@ GenTree* Compiler::gtNewSimdMinMaxNode(var_types type,
}
else
{
needsFixup = cnsNode->IsVectorNegativeZero(simdBaseType);
needsFixup |= cnsNode->AsVecCon()->ContainsNegativeZero(simdBaseType);
}

if (!needsFixup || compOpportunisticallyDependsOn(InstructionSet_AVX512))
if (!hasPartialNaN &&
(!needsFixup || (!isNumber && compOpportunisticallyDependsOn(InstructionSet_AVX512))))
{
// Given the checks, op1 can safely be the cns and op2 the other node

Expand Down Expand Up @@ -26627,18 +26672,16 @@ GenTree* Compiler::gtNewSimdMinMaxNode(var_types type,
retNode->AsHWIntrinsic()->Op(2) = op2;
gtUpdateNodeSideEffects(retNode);

GenTreeVecCon* tblVecCon = gtNewVconNode(type);

// FixupScalar(left, right, table, control) computes the input type of right
// Fixup(left, right, table, control) computes the input type of right
// adjusts it based on the table and then returns
//
// In our case, left is going to be the result of the RangeScalar operation
// and right is going to be op1 or op2. In the case op1/op2 is QNaN or SNaN
// we want to preserve it instead. Otherwise we want to preserve the original
// result computed by RangeScalar.
//
// If both inputs are NaN, then we'll end up taking op1 by virtue of it being
// the latter fixup.
// In our case, left is the result of the min/max operation and right is the
// opaque operand. The table preserves left except where the constant is the
// problematic zero.

GenTreeVecCon* tblVecCon = gtNewVconNode(type);
int64_t tblValue;
simd_t zeroMask = {};

if (isMax)
{
Expand All @@ -26651,9 +26694,13 @@ GenTree* Compiler::gtNewSimdMinMaxNode(var_types type,
// -VAL: 0b0000
// +VAL: 0b0000

const int64_t tblValue = 0x00000800;
tblVecCon->EvaluateBroadcastInPlace((simdBaseType == TYP_FLOAT) ? TYP_INT : TYP_LONG,
tblValue);
tblValue = 0x00000800;

if (!isScalar)
{
zeroMask =
EvaluateSimdIsPositiveZero(simdBaseType, cnsNode->AsVecCon()->gtSimdVal, simdSize);
}
}
else
{
Expand All @@ -26666,9 +26713,24 @@ GenTree* Compiler::gtNewSimdMinMaxNode(var_types type,
// -VAL: 0b0000
// +VAL: 0b0000

const int64_t tblValue = 0x00000700;
tblVecCon->EvaluateBroadcastInPlace((simdBaseType == TYP_FLOAT) ? TYP_INT : TYP_LONG,
tblValue);
tblValue = 0x00000700;

if (!isScalar)
{
zeroMask =
EvaluateSimdIsNegativeZero(simdBaseType, cnsNode->AsVecCon()->gtSimdVal, simdSize);
}
}

var_types tblType = (simdBaseType == TYP_FLOAT) ? TYP_INT : TYP_LONG;
tblVecCon->EvaluateBroadcastInPlace(tblType, tblValue);

if (!isScalar)
{
simd_t result = {};
EvaluateBinarySimd<simd_t>(GT_AND, false, tblType, &result, tblVecCon->gtSimdVal, zeroMask,
simdSize);
tblVecCon->gtSimdVal = result;
}

intrinsic = isScalar ? NI_AVX512_FixupScalar : NI_AVX512_Fixup;
Expand Down
6 changes: 6 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 @@ -7427,6 +7427,12 @@ struct GenTreeVecCon : public GenTree

bool IsNegativeZero(var_types simdBaseType) const;

bool ContainsNaN(var_types simdBaseType) const;

bool ContainsNegativeZero(var_types simdBaseType) const;

bool ContainsPositiveZero(var_types simdBaseType) const;

bool IsZero() const
{
switch (gtType)
Expand Down
Loading
Loading