[llvm] r370659 - [InstCombine] recognize bswap disguised as shufflevector

Philip Reames via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 28 11:06:22 PDT 2019


(Very late response..)

On 9/10/2019 2:40 PM, Roman Lebedev wrote:
> On Wed, Sep 11, 2019 at 12:29 AM Sanjay Patel via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
>> Hmm...I had not thought of that option. Either way (independent of what we choose as canonical), we still need to add some infrastructure to match the pattern because we're missing it currently.
> IMHO it's not good canonicalization especially since we have intrinsic for it.
> We'd be "arbitrarily" adding one-off vector instruction into
> previously-scalar codepath.
> E.g. KnownBits aggregates things for vectors to be globally-correct,
> instead of tracking each lane separately.
Honestly, this sounds like a weakness in our known bits analysis we 
should fix.  We should be able to treat small vectors and integers 
equivalently.   The fact we can't is incredibly unfortunate.
>
>> There's a related question of shift/logic canonicalization posed here:
>> https://bugs.llvm.org/show_bug.cgi?id=43146
>> ...and I'm still not sure what the answer to that question is.
> https://reviews.llvm.org/D67383 seems related.
>
> Roman.
>
>> On Tue, Sep 10, 2019 at 3:44 PM Philip Reames <listmail at philipreames.com> wrote:
>>> Just a thought, maybe we don't need a bswap intrinsic at all and should
>>> canonicalize to the shuffle form?  If bswap is a good lowering for a
>>> particular shuffle, shouldn't the backed be selecting that anyway?
>>>
>>> Philip
>>>
>>> On 9/2/19 6:33 AM, Sanjay Patel via llvm-commits wrote:
>>>> Author: spatel
>>>> Date: Mon Sep  2 06:33:20 2019
>>>> New Revision: 370659
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=370659&view=rev
>>>> Log:
>>>> [InstCombine] recognize bswap disguised as shufflevector
>>>>
>>>> bitcast <N x i8> (shuf X, undef, <N, N-1,...0>) to i{N*8} --> bswap (bitcast X to i{N*8})
>>>>
>>>> In PR43146:
>>>> https://bugs.llvm.org/show_bug.cgi?id=43146
>>>> ...we have a more complicated case where SLP is making a mess of bswap. This patch won't
>>>> do anything for that currently, but we need to improve bswap recognition in instcombine,
>>>> SLP, and/or a standalone pass to avoid that problem.
>>>>
>>>> This is limited using the data-layout so we don't try to do this transform with actual
>>>> vector types. The backend does not appear to have folds to convert in either direction,
>>>> so we don't want to mess up something that is actually better lowered as a shuffle.
>>>>
>>>> On x86, we're trading something like this:
>>>>
>>>>     vmovd      %edi, %xmm0
>>>>     vpshufb    LCPI0_0(%rip), %xmm0, %xmm0 ## xmm0 = xmm0[3,2,1,0,u,u,u,u,u,u,u,u,u,u,u,u]
>>>>     vmovd      %xmm0, %eax
>>>>
>>>> For:
>>>>
>>>>     movl       %edi, %eax
>>>>     bswapl     %eax
>>>>
>>>> Differential Revision: https://reviews.llvm.org/D66965
>>>>
>>>> Modified:
>>>>       llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp
>>>>       llvm/trunk/test/Transforms/InstCombine/bswap.ll
>>>>
>>>> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp?rev=370659&r1=370658&r2=370659&view=diff
>>>> ==============================================================================
>>>> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp (original)
>>>> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp Mon Sep  2 06:33:20 2019
>>>> @@ -2416,6 +2416,22 @@ Instruction *InstCombiner::visitBitCast(
>>>>            return new ShuffleVectorInst(LHS, RHS, Shuf->getOperand(2));
>>>>          }
>>>>        }
>>>> +
>>>> +    // A bitcasted-to-scalar and byte-reversing shuffle is better recognized as
>>>> +    // a byte-swap:
>>>> +    // bitcast <N x i8> (shuf X, undef, <N, N-1,...0>) --> bswap (bitcast X)
>>>> +    // TODO: We should match the related pattern for bitreverse.
>>>> +    if (DestTy->isIntegerTy() &&
>>>> +        DL.isLegalInteger(DestTy->getScalarSizeInBits()) &&
>>>> +        SrcTy->getScalarSizeInBits() == 8 && NumShufElts % 2 == 0 &&
>>>> +        Shuf->hasOneUse() && Shuf->isReverse()) {
>>>> +      assert(ShufOp0->getType() == SrcTy && "Unexpected shuffle mask");
>>>> +      assert(isa<UndefValue>(ShufOp1) && "Unexpected shuffle op");
>>>> +      Function *Bswap =
>>>> +          Intrinsic::getDeclaration(CI.getModule(), Intrinsic::bswap, DestTy);
>>>> +      Value *ScalarX = Builder.CreateBitCast(ShufOp0, DestTy);
>>>> +      return IntrinsicInst::Create(Bswap, { ScalarX });
>>>> +    }
>>>>      }
>>>>
>>>>      // Handle the A->B->A cast, and there is an intervening PHI node.
>>>>
>>>> Modified: llvm/trunk/test/Transforms/InstCombine/bswap.ll
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/bswap.ll?rev=370659&r1=370658&r2=370659&view=diff
>>>> ==============================================================================
>>>> --- llvm/trunk/test/Transforms/InstCombine/bswap.ll (original)
>>>> +++ llvm/trunk/test/Transforms/InstCombine/bswap.ll Mon Sep  2 06:33:20 2019
>>>> @@ -233,8 +233,8 @@ define i16 @test10(i32 %a) {
>>>>
>>>>    define i32 @shuf_4bytes(<4 x i8> %x) {
>>>>    ; CHECK-LABEL: @shuf_4bytes(
>>>> -; CHECK-NEXT:    [[BSWAP:%.*]] = shufflevector <4 x i8> [[X:%.*]], <4 x i8> undef, <4 x i32> <i32 3, i32 2, i32 1, i32 0>
>>>> -; CHECK-NEXT:    [[CAST:%.*]] = bitcast <4 x i8> [[BSWAP]] to i32
>>>> +; CHECK-NEXT:    [[TMP1:%.*]] = bitcast <4 x i8> [[X:%.*]] to i32
>>>> +; CHECK-NEXT:    [[CAST:%.*]] = call i32 @llvm.bswap.i32(i32 [[TMP1]])
>>>>    ; CHECK-NEXT:    ret i32 [[CAST]]
>>>>    ;
>>>>      %bswap = shufflevector <4 x i8> %x, <4 x i8> undef, <4 x i32> <i32 3, i32 2, i32 1, i32 0>
>>>> @@ -244,9 +244,9 @@ define i32 @shuf_4bytes(<4 x i8> %x) {
>>>>
>>>>    define i32 @shuf_load_4bytes(<4 x i8>* %p) {
>>>>    ; CHECK-LABEL: @shuf_load_4bytes(
>>>> -; CHECK-NEXT:    [[X:%.*]] = load <4 x i8>, <4 x i8>* [[P:%.*]], align 4
>>>> -; CHECK-NEXT:    [[BSWAP:%.*]] = shufflevector <4 x i8> [[X]], <4 x i8> undef, <4 x i32> <i32 3, i32 2, i32 undef, i32 0>
>>>> -; CHECK-NEXT:    [[CAST:%.*]] = bitcast <4 x i8> [[BSWAP]] to i32
>>>> +; CHECK-NEXT:    [[TMP1:%.*]] = bitcast <4 x i8>* [[P:%.*]] to i32*
>>>> +; CHECK-NEXT:    [[X1:%.*]] = load i32, i32* [[TMP1]], align 4
>>>> +; CHECK-NEXT:    [[CAST:%.*]] = call i32 @llvm.bswap.i32(i32 [[X1]])
>>>>    ; CHECK-NEXT:    ret i32 [[CAST]]
>>>>    ;
>>>>      %x = load <4 x i8>, <4 x i8>* %p
>>>> @@ -257,9 +257,7 @@ define i32 @shuf_load_4bytes(<4 x i8>* %
>>>>
>>>>    define i32 @shuf_bitcast_twice_4bytes(i32 %x) {
>>>>    ; CHECK-LABEL: @shuf_bitcast_twice_4bytes(
>>>> -; CHECK-NEXT:    [[CAST1:%.*]] = bitcast i32 [[X:%.*]] to <4 x i8>
>>>> -; CHECK-NEXT:    [[BSWAP:%.*]] = shufflevector <4 x i8> [[CAST1]], <4 x i8> undef, <4 x i32> <i32 undef, i32 2, i32 1, i32 0>
>>>> -; CHECK-NEXT:    [[CAST2:%.*]] = bitcast <4 x i8> [[BSWAP]] to i32
>>>> +; CHECK-NEXT:    [[CAST2:%.*]] = call i32 @llvm.bswap.i32(i32 [[X:%.*]])
>>>>    ; CHECK-NEXT:    ret i32 [[CAST2]]
>>>>    ;
>>>>      %cast1 = bitcast i32 %x to <4 x i8>
>>>> @@ -268,6 +266,7 @@ define i32 @shuf_bitcast_twice_4bytes(i3
>>>>      ret i32 %cast2
>>>>    }
>>>>
>>>> +; Negative test - extra use
>>>>    declare void @use(<4 x i8>)
>>>>
>>>>    define i32 @shuf_4bytes_extra_use(<4 x i8> %x) {
>>>> @@ -283,6 +282,8 @@ define i32 @shuf_4bytes_extra_use(<4 x i
>>>>      ret i32 %cast
>>>>    }
>>>>
>>>> +; Negative test - scalar type is not in the data layout
>>>> +
>>>>    define i128 @shuf_16bytes(<16 x i8> %x) {
>>>>    ; CHECK-LABEL: @shuf_16bytes(
>>>>    ; CHECK-NEXT:    [[BSWAP:%.*]] = shufflevector <16 x i8> [[X:%.*]], <16 x i8> undef, <16 x i32> <i32 15, i32 14, i32 13, i32 12, i32 11, i32 10, i32 9, i32 8, i32 7, i32 6, i32 5, i32 4, i32 3, i32 2, i32 1, i32 0>
>>>> @@ -294,6 +295,8 @@ define i128 @shuf_16bytes(<16 x i8> %x)
>>>>      ret i128 %cast
>>>>    }
>>>>
>>>> +; Negative test - don't touch widening shuffles (for now)
>>>> +
>>>>    define i32 @shuf_2bytes_widening(<2 x i8> %x) {
>>>>    ; CHECK-LABEL: @shuf_2bytes_widening(
>>>>    ; CHECK-NEXT:    [[BSWAP:%.*]] = shufflevector <2 x i8> [[X:%.*]], <2 x i8> undef, <4 x i32> <i32 1, i32 0, i32 undef, i32 undef>
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at lists.llvm.org
>>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


More information about the llvm-commits mailing list