Skip to content

Commit f83162d

Browse files
tannergoodingtannergooding
committed
Avoid some messiness around operand swapping
1 parent 0db9b67 commit f83162d

File tree

2 files changed

+171
-49
lines changed

2 files changed

+171
-49
lines changed

‎src/coreclr/jit/hwintrinsicxarch.cpp‎

Lines changed: 76 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -606,18 +606,11 @@ int HWIntrinsicInfo::lookupIval(Compiler* comp, NamedIntrinsic id, var_types sim
606606
{
607607
if (varTypeIsFloating(simdBaseType))
608608
{
609-
if (comp->compOpportunisticallyDependsOn(InstructionSet_AVX))
610-
{
611-
return static_cast<int>(FloatComparisonMode::OrderedGreaterThanSignaling);
612-
}
613-
614609
// CompareGreaterThan is not directly supported in hardware without AVX support.
615-
// We will return the inverted case here and lowering will itself swap the ops
616-
// to ensure the emitted code remains correct. This simplifies the overall logic
617-
// here and for other use cases.
610+
// Lowering ensures we swap the operands and change to the correct ID.
618611

619-
assert(id != NI_AVX_CompareGreaterThan);
620-
return static_cast<int>(FloatComparisonMode::OrderedLessThanSignaling);
612+
assert(comp->compIsaSupportedDebugOnly(InstructionSet_AVX));
613+
return static_cast<int>(FloatComparisonMode::OrderedGreaterThanSignaling);
621614
}
622615
else if ((id == NI_AVX512F_CompareGreaterThanMask) && varTypeIsUnsigned(simdBaseType))
623616
{
@@ -655,18 +648,11 @@ int HWIntrinsicInfo::lookupIval(Compiler* comp, NamedIntrinsic id, var_types sim
655648
{
656649
if (varTypeIsFloating(simdBaseType))
657650
{
658-
if (comp->compOpportunisticallyDependsOn(InstructionSet_AVX))
659-
{
660-
return static_cast<int>(FloatComparisonMode::OrderedGreaterThanOrEqualSignaling);
661-
}
662-
663651
// CompareGreaterThanOrEqual is not directly supported in hardware without AVX support.
664-
// We will return the inverted case here and lowering will itself swap the ops
665-
// to ensure the emitted code remains correct. This simplifies the overall logic
666-
// here and for other use cases.
652+
// Lowering ensures we swap the operands and change to the correct ID.
667653

668-
assert(id != NI_AVX_CompareGreaterThanOrEqual);
669-
return static_cast<int>(FloatComparisonMode::OrderedLessThanOrEqualSignaling);
654+
assert(comp->compIsaSupportedDebugOnly(InstructionSet_AVX));
655+
return static_cast<int>(FloatComparisonMode::OrderedGreaterThanOrEqualSignaling);
670656
}
671657
else
672658
{
@@ -723,18 +709,11 @@ int HWIntrinsicInfo::lookupIval(Compiler* comp, NamedIntrinsic id, var_types sim
723709
{
724710
if (varTypeIsFloating(simdBaseType))
725711
{
726-
if (comp->compOpportunisticallyDependsOn(InstructionSet_AVX))
727-
{
728-
return static_cast<int>(FloatComparisonMode::UnorderedNotGreaterThanSignaling);
729-
}
730-
731712
// CompareNotGreaterThan is not directly supported in hardware without AVX support.
732-
// We will return the inverted case here and lowering will itself swap the ops
733-
// to ensure the emitted code remains correct. This simplifies the overall logic
734-
// here and for other use cases.
713+
// Lowering ensures we swap the operands and change to the correct ID.
735714

736-
assert(id != NI_AVX_CompareGreaterThan);
737-
return static_cast<int>(FloatComparisonMode::UnorderedNotLessThanSignaling);
715+
assert(comp->compIsaSupportedDebugOnly(InstructionSet_AVX));
716+
return static_cast<int>(FloatComparisonMode::UnorderedNotGreaterThanSignaling);
738717
}
739718
else
740719
{
@@ -772,18 +751,11 @@ int HWIntrinsicInfo::lookupIval(Compiler* comp, NamedIntrinsic id, var_types sim
772751
{
773752
if (varTypeIsFloating(simdBaseType))
774753
{
775-
if (comp->compOpportunisticallyDependsOn(InstructionSet_AVX))
776-
{
777-
return static_cast<int>(FloatComparisonMode::UnorderedNotGreaterThanOrEqualSignaling);
778-
}
779-
780754
// CompareNotGreaterThanOrEqual is not directly supported in hardware without AVX support.
781-
// We will return the inverted case here and lowering will itself swap the ops
782-
// to ensure the emitted code remains correct. This simplifies the overall logic
783-
// here and for other use cases.
755+
// Lowering ensures we swap the operands and change to the correct ID.
784756

785-
assert(id != NI_AVX_CompareNotGreaterThanOrEqual);
786-
return static_cast<int>(FloatComparisonMode::UnorderedNotLessThanOrEqualSignaling);
757+
assert(comp->compIsaSupportedDebugOnly(InstructionSet_AVX));
758+
return static_cast<int>(FloatComparisonMode::UnorderedNotGreaterThanOrEqualSignaling);
787759
}
788760
else
789761
{
@@ -3404,6 +3376,38 @@ GenTree* Compiler::impSpecialIntrinsic(NamedIntrinsic intrinsic,
34043376
op1 = impCloneExpr(op1, &clonedOp1, CHECK_SPILL_ALL,
34053377
nullptr DEBUGARG("Clone op1 for Sse.CompareScalarGreaterThan"));
34063378

3379+
switch (intrinsic)
3380+
{
3381+
case NI_SSE_CompareScalarGreaterThan:
3382+
{
3383+
intrinsic = NI_SSE_CompareScalarLessThan;
3384+
break;
3385+
}
3386+
3387+
case NI_SSE_CompareScalarGreaterThanOrEqual:
3388+
{
3389+
intrinsic = NI_SSE_CompareScalarLessThanOrEqual;
3390+
break;
3391+
}
3392+
3393+
case NI_SSE_CompareScalarNotGreaterThan:
3394+
{
3395+
intrinsic = NI_SSE_CompareScalarNotLessThan;
3396+
break;
3397+
}
3398+
3399+
case NI_SSE_CompareScalarNotGreaterThanOrEqual:
3400+
{
3401+
intrinsic = NI_SSE_CompareScalarNotLessThanOrEqual;
3402+
break;
3403+
}
3404+
3405+
default:
3406+
{
3407+
unreached();
3408+
}
3409+
}
3410+
34073411
retNode = gtNewSimdHWIntrinsicNode(TYP_SIMD16, op2, op1, intrinsic, simdBaseJitType, simdSize);
34083412
retNode = gtNewSimdHWIntrinsicNode(TYP_SIMD16, clonedOp1, retNode, NI_SSE_MoveScalar, simdBaseJitType,
34093413
simdSize);
@@ -3463,6 +3467,38 @@ GenTree* Compiler::impSpecialIntrinsic(NamedIntrinsic intrinsic,
34633467
op1 = impCloneExpr(op1, &clonedOp1, CHECK_SPILL_ALL,
34643468
nullptr DEBUGARG("Clone op1 for Sse2.CompareScalarGreaterThan"));
34653469

3470+
switch (intrinsic)
3471+
{
3472+
case NI_SSE2_CompareScalarGreaterThan:
3473+
{
3474+
intrinsic = NI_SSE2_CompareScalarLessThan;
3475+
break;
3476+
}
3477+
3478+
case NI_SSE2_CompareScalarGreaterThanOrEqual:
3479+
{
3480+
intrinsic = NI_SSE2_CompareScalarLessThanOrEqual;
3481+
break;
3482+
}
3483+
3484+
case NI_SSE2_CompareScalarNotGreaterThan:
3485+
{
3486+
intrinsic = NI_SSE2_CompareScalarNotLessThan;
3487+
break;
3488+
}
3489+
3490+
case NI_SSE2_CompareScalarNotGreaterThanOrEqual:
3491+
{
3492+
intrinsic = NI_SSE2_CompareScalarNotLessThanOrEqual;
3493+
break;
3494+
}
3495+
3496+
default:
3497+
{
3498+
unreached();
3499+
}
3500+
}
3501+
34663502
retNode = gtNewSimdHWIntrinsicNode(TYP_SIMD16, op2, op1, intrinsic, simdBaseJitType, simdSize);
34673503
retNode = gtNewSimdHWIntrinsicNode(TYP_SIMD16, clonedOp1, retNode, NI_SSE2_MoveScalar, simdBaseJitType,
34683504
simdSize);

‎src/coreclr/jit/lowerxarch.cpp‎

Lines changed: 95 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1919,6 +1919,68 @@ GenTree* Lowering::LowerHWIntrinsic(GenTreeHWIntrinsic* node)
19191919
}
19201920

19211921
// pre-AVX doesn't actually support these intrinsics in hardware so we need to swap the operands around
1922+
NamedIntrinsic newIntrinsicId = NI_Illegal;
1923+
1924+
switch (intrinsicId)
1925+
{
1926+
case NI_SSE_CompareGreaterThan:
1927+
{
1928+
newIntrinsicId = NI_SSE_CompareLessThan;
1929+
break;
1930+
}
1931+
1932+
case NI_SSE_CompareGreaterThanOrEqual:
1933+
{
1934+
newIntrinsicId = NI_SSE_CompareLessThanOrEqual;
1935+
break;
1936+
}
1937+
1938+
case NI_SSE_CompareNotGreaterThan:
1939+
{
1940+
newIntrinsicId = NI_SSE_CompareNotLessThan;
1941+
break;
1942+
}
1943+
1944+
case NI_SSE_CompareNotGreaterThanOrEqual:
1945+
{
1946+
newIntrinsicId = NI_SSE_CompareNotLessThanOrEqual;
1947+
break;
1948+
}
1949+
1950+
case NI_SSE2_CompareGreaterThan:
1951+
{
1952+
newIntrinsicId = NI_SSE2_CompareLessThan;
1953+
break;
1954+
}
1955+
1956+
case NI_SSE2_CompareGreaterThanOrEqual:
1957+
{
1958+
newIntrinsicId = NI_SSE2_CompareLessThanOrEqual;
1959+
break;
1960+
}
1961+
1962+
case NI_SSE2_CompareNotGreaterThan:
1963+
{
1964+
newIntrinsicId = NI_SSE2_CompareNotLessThan;
1965+
break;
1966+
}
1967+
1968+
case NI_SSE2_CompareNotGreaterThanOrEqual:
1969+
{
1970+
newIntrinsicId = NI_SSE2_CompareNotLessThanOrEqual;
1971+
break;
1972+
}
1973+
1974+
default:
1975+
{
1976+
unreached();
1977+
}
1978+
}
1979+
1980+
assert(newIntrinsicId != NI_Illegal);
1981+
assert(intrinsicId != newIntrinsicId);
1982+
1983+
node->ChangeHWIntrinsicId(newIntrinsicId);
19221984
std::swap(node->Op(1), node->Op(2));
19231985
break;
19241986
}
@@ -1933,7 +1995,39 @@ GenTree* Lowering::LowerHWIntrinsic(GenTreeHWIntrinsic* node)
19331995
}
19341996
assert(varTypeIsIntegral(node->GetSimdBaseType()));
19351997

1936-
// this isn't actually supported in hardware so we need to swap the operands around
1998+
// pre-AVX512 doesn't actually support these intrinsics in hardware so we need to swap the operands around
1999+
NamedIntrinsic newIntrinsicId = NI_Illegal;
2000+
2001+
switch (intrinsicId)
2002+
{
2003+
case NI_SSE2_CompareLessThan:
2004+
{
2005+
newIntrinsicId = NI_SSE2_CompareGreaterThan;
2006+
break;
2007+
}
2008+
2009+
case NI_SSE42_CompareLessThan:
2010+
{
2011+
newIntrinsicId = NI_SSE42_CompareGreaterThan;
2012+
break;
2013+
}
2014+
2015+
case NI_AVX2_CompareLessThan:
2016+
{
2017+
newIntrinsicId = NI_AVX2_CompareGreaterThan;
2018+
break;
2019+
}
2020+
2021+
default:
2022+
{
2023+
unreached();
2024+
}
2025+
}
2026+
2027+
assert(newIntrinsicId != NI_Illegal);
2028+
assert(intrinsicId != newIntrinsicId);
2029+
2030+
node->ChangeHWIntrinsicId(newIntrinsicId);
19372031
std::swap(node->Op(1), node->Op(2));
19382032
break;
19392033
}
@@ -3158,14 +3252,6 @@ GenTree* Lowering::LowerHWIntrinsicTernaryLogic(GenTreeHWIntrinsic* node)
31583252
case NI_AVX_CompareLessThan:
31593253
case NI_AVX2_CompareLessThan:
31603254
{
3161-
if (varTypeIsIntegral(cndNode->GetSimdBaseType()))
3162-
{
3163-
// CompareLessThan isn't supported in hardware prior to AVX512, as such
3164-
// the lowering of the node will have already swapped these around so that
3165-
// it would be treated as CompareGreaterThan instead. This swaps the operands
3166-
// back since it can be natively supported here instead
3167-
std::swap(selectFalse, selectTrue);
3168-
}
31693255
cndId = NI_AVX512F_CompareLessThanMask;
31703256
break;
31713257
}

0 commit comments

Comments
 (0)