Skip to content

Commit f32bf01

Browse files
committed
Changes to IsValidForShuffle & jit format
- Ensure we use IsValidForShuffle correctly (i.e., ensure all cases that could be emittet at some point, are able to be done) - jit format changes
1 parent f381cf0 commit f32bf01

File tree

5 files changed

+37
-13
lines changed

5 files changed

+37
-13
lines changed

src/coreclr/jit/compiler.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4796,7 +4796,7 @@ class Compiler
47964796
bool mustExpand);
47974797

47984798
#ifdef FEATURE_HW_INTRINSICS
4799-
bool IsValidForShuffle(GenTree* indices, unsigned simdSize, var_types simdBaseType) const;
4799+
bool IsValidForShuffle(GenTree* indices, unsigned simdSize, var_types simdBaseType, bool* canBecomeValid) const;
48004800

48014801
GenTree* impHWIntrinsic(NamedIntrinsic intrinsic,
48024802
CORINFO_CLASS_HANDLE clsHnd,

src/coreclr/jit/gentree.cpp

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18444,9 +18444,13 @@ unsigned GenTreeVecCon::ElementCount(unsigned simdSize, var_types simdBaseType)
1844418444
return simdSize / genTypeSize(simdBaseType);
1844518445
}
1844618446

18447-
bool Compiler::IsValidForShuffle(GenTree* indices, unsigned simdSize, var_types simdBaseType) const
18447+
bool Compiler::IsValidForShuffle(GenTree* indices, unsigned simdSize, var_types simdBaseType, bool* canBecomeValid) const
1844818448
{
1844918449
#if defined(TARGET_XARCH)
18450+
if (canBecomeValid)
18451+
{
18452+
*canBecomeValid = false;
18453+
}
1845018454
size_t elementSize = genTypeSize(simdBaseType);
1845118455
size_t elementCount = simdSize / elementSize;
1845218456

@@ -18480,11 +18484,20 @@ bool Compiler::IsValidForShuffle(GenTree* indices, unsigned simdSize, var_types
1848018484
if (!indices->IsCnsVec() && !compOpportunisticallyDependsOn(InstructionSet_SSSE3))
1848118485
{
1848218486
// the variable implementation for Vector128 Shuffle always needs SSSE3
18487+
// however, this can become valid later if it becomes constant
18488+
if (canBecomeValid)
18489+
{
18490+
*canBecomeValid = true;
18491+
}
1848318492
return false;
1848418493
}
1848518494
}
1848618495
#endif // TARGET_XARCH
1848718496

18497+
if (canBecomeValid)
18498+
{
18499+
*canBecomeValid = true;
18500+
}
1848818501
return true;
1848918502
}
1849018503

@@ -25501,14 +25514,14 @@ GenTree* Compiler::gtNewSimdShuffleNodeVariable(
2550125514
else if (elementSize == 8 && simdSize == 16 && compOpportunisticallyDependsOn(InstructionSet_AVX512F_VL))
2550225515
{
2550325516
GenTree* op1Copy = fgMakeMultiUse(&op1); // just use op1 again for the other variable
25504-
retNode = gtNewSimdHWIntrinsicNode(type, op1, op2, op1Copy, NI_AVX512F_VL_PermuteVar2x64x2, simdBaseJitType,
25505-
simdSize);
25517+
retNode
25518+
= gtNewSimdHWIntrinsicNode(type, op1, op2, op1Copy, NI_AVX512F_VL_PermuteVar2x64x2, simdBaseJitType,
25519+
simdSize);
2550625520
}
2550725521
else if (elementSize == 8 && simdSize == 16 && compOpportunisticallyDependsOn(InstructionSet_AVX10v1))
2550825522
{
2550925523
GenTree* op1Copy = fgMakeMultiUse(&op1); // just use op1 again for the other variable
25510-
retNode = gtNewSimdHWIntrinsicNode(type, op1, op2, op1Copy, NI_AVX10v1_PermuteVar2x64x2, simdBaseJitType,
25511-
simdSize);
25524+
retNode = gtNewSimdHWIntrinsicNode(type, op1, op2, op1Copy, NI_AVX10v1_PermuteVar2x64x2, simdBaseJitType, simdSize);
2551225525
}
2551325526
else
2551425527
{

src/coreclr/jit/hwintrinsicarm64.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2259,11 +2259,13 @@ GenTree* Compiler::impSpecialIntrinsic(NamedIntrinsic intrinsic,
22592259

22602260
GenTree* indices = impStackTop(0).val;
22612261

2262-
if (!IsValidForShuffle(indices, simdSize, simdBaseType))
2262+
// Check if the required intrinsics to emit are available.
2263+
if (!IsValidForShuffle(indices, simdSize, simdBaseType, nullptr))
22632264
{
22642265
break;
22652266
}
22662267

2268+
// If the indices might become constant later, then we don't emit for now, delay until later.
22672269
if (!indices->IsCnsVec())
22682270
{
22692271
assert(sig->numArgs == 2);

src/coreclr/jit/hwintrinsicxarch.cpp

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3650,12 +3650,14 @@ GenTree* Compiler::impSpecialIntrinsic(NamedIntrinsic intrinsic,
36503650

36513651
GenTree* indices = impStackTop(0).val;
36523652

3653-
if (!IsValidForShuffle(indices, simdSize, simdBaseType))
3654-
{
3655-
break;
3656-
}
3653+
// Check if the required intrinsics are available to emit now (validForShuffle). If we have variable
3654+
// indices that might become possible to emit later (due to them becoming constant), this will be
3655+
// indicated in canBecomeValidForShuffle; otherwise, it's just the same as validForShuffle.
3656+
bool canBecomeValidForShuffle = false;
3657+
bool validForShuffle = !IsValidForShuffle(indices, simdSize, simdBaseType, &canBecomeValidForShuffle);
36573658

3658-
if (!indices->IsCnsVec())
3659+
// If the indices might become constant later, then we don't emit for now, delay until later.
3660+
if (canBecomeValidForShuffle && ((!validForShuffle && canBecomeValidForShuffle) || !indices->IsCnsVec()))
36593661
{
36603662
assert(sig->numArgs == 2);
36613663

@@ -3673,6 +3675,12 @@ GenTree* Compiler::impSpecialIntrinsic(NamedIntrinsic intrinsic,
36733675
}
36743676
}
36753677

3678+
// If it isn't valid for shuffle (and can't become valid later, due to above block), then give up now.
3679+
if (!validForShuffle)
3680+
{
3681+
return nullptr;
3682+
}
3683+
36763684
if (sig->numArgs == 2)
36773685
{
36783686
op2 = impSIMDPopStack();

src/coreclr/jit/rationalize.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,8 @@ void Rationalizer::RewriteHWIntrinsicAsUserCall(GenTree** use, ArrayStack<GenTre
344344
GenTree* op1 = operands[0];
345345
GenTree* op2 = operands[1];
346346

347-
if (!comp->IsValidForShuffle(op2, simdSize, simdBaseType))
347+
// Check if the required intrinsics to emit are available.
348+
if (!comp->IsValidForShuffle(op2, simdSize, simdBaseType, nullptr))
348349
{
349350
break;
350351
}

0 commit comments

Comments
 (0)