[llvm] r334958 - [SLPVectorizer] Tidyup isShuffle helper

[Simon Pilgrim via llvm-commits]

I'll take a look - its correctly seeing this as a full 256-bit vector 
2op shuffle now so has exposed what looks like a copy+paste issue with 
TTI shuffle costs.


On 20/06/2018 22:57, douglas.yung at sony.com wrote:
> Hi Simon,
>
> Some of our internal tests noticed a regression in the codegen quality for horizontal adds/subs after this change. I have put the details in PR37882. Can you take a look?
>
> Douglas Yung
>
>> -----Original Message-----
>> From: llvm-commits [mailto:llvm-commits-bounces at lists.llvm.org] On
>> Behalf Of Simon Pilgrim via llvm-commits
>> Sent: Monday, June 18, 2018 9:25
>> To: llvm-commits at lists.llvm.org
>> Subject: [llvm] r334958 - [SLPVectorizer] Tidyup isShuffle helper
>>
>> Author: rksimon
>> Date: Mon Jun 18 09:25:01 2018
>> New Revision: 334958
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=334958&view=rev
>> Log:
>> [SLPVectorizer] Tidyup isShuffle helper
>>
>> Ensure we keep track of the input vectors in all cases instead of just
>> for SK_Select.
>>
>> Ideally we'd reuse the shuffle mask pattern matching in
>> TargetTransformInfo::getInstructionThroughput here to easily add
>> support for all TargetTransformInfo::ShuffleKind without mass code
>> duplication, I've added a TODO for now but D48236 should help us here.
>>
>> Differential Revision: https://reviews.llvm.org/D48023
>>
>> Modified:
>>      llvm/trunk/lib/Transforms/Vectorize/SLPVectorizer.cpp
>>
>> Modified: llvm/trunk/lib/Transforms/Vectorize/SLPVectorizer.cpp
>> URL: http://llvm.org/viewvc/llvm-
>> project/llvm/trunk/lib/Transforms/Vectorize/SLPVectorizer.cpp?rev=33495
>> 8&r1=334957&r2=334958&view=diff
>> =======================================================================
>> =======
>> --- llvm/trunk/lib/Transforms/Vectorize/SLPVectorizer.cpp (original)
>> +++ llvm/trunk/lib/Transforms/Vectorize/SLPVectorizer.cpp Mon Jun 18
>> 09:25:01 2018
>> @@ -246,13 +246,15 @@ static bool isSplat(ArrayRef<Value *> VL
>>   /// %ins4 = insertelement <4 x i8> %ins3, i8 %9, i32 3
>>   /// ret <4 x i8> %ins4
>>   /// InstCombiner transforms this into a shuffle and vector mul
>> +/// TODO: Can we split off and reuse the shuffle mask detection from
>> +/// TargetTransformInfo::getInstructionThroughput?
>>   static Optional<TargetTransformInfo::ShuffleKind>
>>   isShuffle(ArrayRef<Value *> VL) {
>>     auto *EI0 = cast<ExtractElementInst>(VL[0]);
>>     unsigned Size = EI0->getVectorOperandType()->getVectorNumElements();
>>     Value *Vec1 = nullptr;
>>     Value *Vec2 = nullptr;
>> -  enum ShuffleMode {Unknown, FirstAlternate, SecondAlternate,
>> Permute};
>> +  enum ShuffleMode { Unknown, Select, Permute };
>>     ShuffleMode CommonShuffleMode = Unknown;
>>     for (unsigned I = 0, E = VL.size(); I < E; ++I) {
>>       auto *EI = cast<ExtractElementInst>(VL[I]);
>> @@ -272,7 +274,11 @@ isShuffle(ArrayRef<Value *> VL) {
>>         continue;
>>       // For correct shuffling we have to have at most 2 different
>> vector operands
>>       // in all extractelement instructions.
>> -    if (Vec1 && Vec2 && Vec != Vec1 && Vec != Vec2)
>> +    if (!Vec1 || Vec1 == Vec)
>> +      Vec1 = Vec;
>> +    else if (!Vec2 || Vec2 == Vec)
>> +      Vec2 = Vec;
>> +    else
>>         return None;
>>       if (CommonShuffleMode == Permute)
>>         continue;
>> @@ -282,37 +288,10 @@ isShuffle(ArrayRef<Value *> VL) {
>>         CommonShuffleMode = Permute;
>>         continue;
>>       }
>> -    // Check the shuffle mode for the current operation.
>> -    if (!Vec1)
>> -      Vec1 = Vec;
>> -    else if (Vec != Vec1)
>> -      Vec2 = Vec;
>> -    // Example: shufflevector A, B, <0,5,2,7>
>> -    // I is odd and IntIdx for A == I - FirstAlternate shuffle.
>> -    // I is even and IntIdx for B == I - FirstAlternate shuffle.
>> -    // Example: shufflevector A, B, <4,1,6,3>
>> -    // I is even and IntIdx for A == I - SecondAlternate shuffle.
>> -    // I is odd and IntIdx for B == I - SecondAlternate shuffle.
>> -    const bool IIsEven = I & 1;
>> -    const bool CurrVecIsA = Vec == Vec1;
>> -    const bool IIsOdd = !IIsEven;
>> -    const bool CurrVecIsB = !CurrVecIsA;
>> -    ShuffleMode CurrentShuffleMode =
>> -        ((IIsOdd && CurrVecIsA) || (IIsEven && CurrVecIsB)) ?
>> FirstAlternate
>> -                                                            :
>> SecondAlternate;
>> -    // Common mode is not set or the same as the shuffle mode of the
>> current
>> -    // operation - alternate.
>> -    if (CommonShuffleMode == Unknown)
>> -      CommonShuffleMode = CurrentShuffleMode;
>> -    // Common shuffle mode is not the same as the shuffle mode of the
>> current
>> -    // operation - permutation.
>> -    if (CommonShuffleMode != CurrentShuffleMode)
>> -      CommonShuffleMode = Permute;
>> +    CommonShuffleMode = Select;
>>     }
>>     // If we're not crossing lanes in different vectors, consider it as
>> blending.
>> -  if ((CommonShuffleMode == FirstAlternate ||
>> -       CommonShuffleMode == SecondAlternate) &&
>> -      Vec2)
>> +  if (CommonShuffleMode == Select && Vec2)
>>       return TargetTransformInfo::SK_Select;
>>     // If Vec2 was never used, we have a permutation of a single vector,
>> otherwise
>>     // we have permutation of 2 vectors.
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits