[llvm] r296352 - [X86] Use APInt instead of SmallBitVector for tracking undef elements in constant pool shuffle decoding
Ahmed Bougacha via llvm-commits
llvm-commits at lists.llvm.org
Thu Mar 2 11:20:32 PST 2017
On Wed, Mar 1, 2017 at 2:46 PM, Craig Topper <craig.topper at gmail.com> wrote:
> This isn't as uncommon as you may think. SimplifyDemandedVectorElts in
> InstCombine uses APInt like this too. We also use APInt for
> knownzero/knownone for computeKnownBits in both InstCombine and DAGCombine.
I forgot about SimplifyDemandedVectorElts, but known-bits seems less
shocking, I guess: you're computing a mask of the same bitwidth as the
value.
But you have a point, I guess that ship has sailed..
Why is SmallBitVector (and BitVector?) even a thing then?
-Ahmed
> ~Craig
>
> On Wed, Mar 1, 2017 at 2:42 PM, Ahmed Bougacha <ahmed.bougacha at gmail.com>
> wrote:
>>
>> This seems really unfortunate.. It's very minor, but I found APInt
>> very unexpected in this context. Could we fix SmallBitVector to
>> actually be a "Small" template? Or honestly maybe just use
>> SmallVector.
>>
>> -Ahmed
>>
>>
>> On Mon, Feb 27, 2017 at 8:15 AM, Craig Topper via llvm-commits
>> <llvm-commits at lists.llvm.org> wrote:
>> > Author: ctopper
>> > Date: Mon Feb 27 10:15:25 2017
>> > New Revision: 296352
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=296352&view=rev
>> > Log:
>> > [X86] Use APInt instead of SmallBitVector for tracking undef elements in
>> > constant pool shuffle decoding
>> >
>> > Summary:
>> > SmallBitVector uses a malloc for more than 58 bits on a 64-bit target
>> > and more than 27 bits on a 32-bit target. Some of the vector types we deal
>> > with here use more than those number of elements and therefore cause a
>> > malloc.
>> >
>> > APInt on the other hand supports up to 64 bits without a malloc. That's
>> > the maximum number of bits we need here so we can avoid a malloc for all
>> > cases by using APInt. This will incur a minor increase in stack usage due to
>> > APInt storing the bit count separately from the data bits unlike
>> > SmallBitVector, but that should be ok.
>> >
>> > Reviewers: RKSimon
>> >
>> > Reviewed By: RKSimon
>> >
>> > Subscribers: llvm-commits
>> >
>> > Differential Revision: https://reviews.llvm.org/D30386
>> >
>> > Modified:
>> > llvm/trunk/lib/Target/X86/X86ShuffleDecodeConstantPool.cpp
>> >
>> > Modified: llvm/trunk/lib/Target/X86/X86ShuffleDecodeConstantPool.cpp
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ShuffleDecodeConstantPool.cpp?rev=296352&r1=296351&r2=296352&view=diff
>> >
>> > ==============================================================================
>> > --- llvm/trunk/lib/Target/X86/X86ShuffleDecodeConstantPool.cpp
>> > (original)
>> > +++ llvm/trunk/lib/Target/X86/X86ShuffleDecodeConstantPool.cpp Mon Feb
>> > 27 10:15:25 2017
>> > @@ -14,7 +14,7 @@
>> >
>> > #include "X86ShuffleDecodeConstantPool.h"
>> > #include "Utils/X86ShuffleDecode.h"
>> > -#include "llvm/ADT/SmallBitVector.h"
>> > +#include "llvm/ADT/APInt.h"
>> > #include "llvm/CodeGen/MachineValueType.h"
>> > #include "llvm/IR/Constants.h"
>> >
>> > @@ -25,7 +25,7 @@
>> > namespace llvm {
>> >
>> > static bool extractConstantMask(const Constant *C, unsigned
>> > MaskEltSizeInBits,
>> > - SmallBitVector &UndefElts,
>> > + APInt &UndefElts,
>> > SmallVectorImpl<uint64_t> &RawMask) {
>> > // It is not an error for shuffle masks to not be a vector of
>> > // MaskEltSizeInBits because the constant pool uniques constants by
>> > their
>> > @@ -73,7 +73,7 @@ static bool extractConstantMask(const Co
>> > "Unaligned shuffle mask size");
>> >
>> > unsigned NumMaskElts = CstSizeInBits / MaskEltSizeInBits;
>> > - UndefElts = SmallBitVector(NumMaskElts, false);
>> > + UndefElts = APInt(NumMaskElts, 0);
>> > RawMask.resize(NumMaskElts, 0);
>> >
>> > for (unsigned i = 0; i != NumMaskElts; ++i) {
>> > @@ -83,7 +83,7 @@ static bool extractConstantMask(const Co
>> > // Only treat the element as UNDEF if all bits are UNDEF, otherwise
>> > // treat it as zero.
>> > if (EltUndef.isAllOnesValue()) {
>> > - UndefElts[i] = true;
>> > + UndefElts.setBit(i);
>> > RawMask[i] = 0;
>> > continue;
>> > }
>> > @@ -103,7 +103,7 @@ void DecodePSHUFBMask(const Constant *C,
>> > "Unexpected vector size.");
>> >
>> > // The shuffle mask requires a byte vector.
>> > - SmallBitVector UndefElts;
>> > + APInt UndefElts;
>> > SmallVector<uint64_t, 32> RawMask;
>> > if (!extractConstantMask(C, 8, UndefElts, RawMask))
>> > return;
>> > @@ -144,7 +144,7 @@ void DecodeVPERMILPMask(const Constant *
>> > assert((ElSize == 32 || ElSize == 64) && "Unexpected vector element
>> > size.");
>> >
>> > // The shuffle mask requires elements the same size as the target.
>> > - SmallBitVector UndefElts;
>> > + APInt UndefElts;
>> > SmallVector<uint64_t, 8> RawMask;
>> > if (!extractConstantMask(C, ElSize, UndefElts, RawMask))
>> > return;
>> > @@ -179,7 +179,7 @@ void DecodeVPERMIL2PMask(const Constant
>> > assert((MaskTySize == 128 || MaskTySize == 256) && "Unexpected vector
>> > size.");
>> >
>> > // The shuffle mask requires elements the same size as the target.
>> > - SmallBitVector UndefElts;
>> > + APInt UndefElts;
>> > SmallVector<uint64_t, 8> RawMask;
>> > if (!extractConstantMask(C, ElSize, UndefElts, RawMask))
>> > return;
>> > @@ -230,7 +230,7 @@ void DecodeVPPERMMask(const Constant *C,
>> > "Unexpected vector size.");
>> >
>> > // The shuffle mask requires a byte vector.
>> > - SmallBitVector UndefElts;
>> > + APInt UndefElts;
>> > SmallVector<uint64_t, 32> RawMask;
>> > if (!extractConstantMask(C, 8, UndefElts, RawMask))
>> > return;
>> > @@ -285,7 +285,7 @@ void DecodeVPERMVMask(const Constant *C,
>> > "Unexpected vector element size.");
>> >
>> > // The shuffle mask requires elements the same size as the target.
>> > - SmallBitVector UndefElts;
>> > + APInt UndefElts;
>> > SmallVector<uint64_t, 8> RawMask;
>> > if (!extractConstantMask(C, ElSize, UndefElts, RawMask))
>> > return;
>> > @@ -313,7 +313,7 @@ void DecodeVPERMV3Mask(const Constant *C
>> > "Unexpected vector element size.");
>> >
>> > // The shuffle mask requires elements the same size as the target.
>> > - SmallBitVector UndefElts;
>> > + APInt UndefElts;
>> > SmallVector<uint64_t, 8> RawMask;
>> > if (!extractConstantMask(C, ElSize, UndefElts, RawMask))
>> > return;
>> >
>> >
>> > _______________________________________________
>> > llvm-commits mailing list
>> > llvm-commits at lists.llvm.org
>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
More information about the llvm-commits
mailing list