[llvm] r230907 - ArrayRef: Remove the equals helper with many arguments.

David Blaikie dblaikie at gmail.com
Mon Mar 2 12:50:06 PST 2015


On Mon, Mar 2, 2015 at 12:47 PM, Benjamin Kramer <benny.kra at gmail.com>
wrote:

> On Mon, Mar 2, 2015 at 9:34 PM, David Blaikie <dblaikie at gmail.com> wrote:
> >
> >
> > On Mon, Mar 2, 2015 at 12:32 PM, Benjamin Kramer <benny.kra at gmail.com>
> > wrote:
> >>
> >> On Mon, Mar 2, 2015 at 7:42 PM, David Blaikie <dblaikie at gmail.com>
> wrote:
> >> >
> >> >
> >> > On Sun, Mar 1, 2015 at 1:05 PM, Benjamin Kramer
> >> > <benny.kra at googlemail.com>
> >> > wrote:
> >> >>
> >> >> Author: d0k
> >> >> Date: Sun Mar  1 15:05:05 2015
> >> >> New Revision: 230907
> >> >>
> >> >> URL: http://llvm.org/viewvc/llvm-project?rev=230907&view=rev
> >> >> Log:
> >> >> ArrayRef: Remove the equals helper with many arguments.
> >> >>
> >> >> With initializer lists there is a really neat idiomatic way to write
> >> >> this, 'ArrayRef.equals({1, 2, 3, 4, 5})'. Remove the equal method
> which
> >> >> always had a hard limit on the number of arguments. I considered
> >> >> rewriting it with variadic templates but that's not really a good fit
> >> >> for a function with homogeneous arguments.
> >> >>
> >> >> 'ArrayRef == {1, 2, 3, 4, 5}' would've been even more awesome, but
> >> >> C++11
> >> >> doesn't allow init lists with binary operators.
> >> >
> >> >
> >> > Can we have an implicit ArrayRef ctor from init list? (not sure what
> the
> >> > storage of init lists is... )
> >>
> >> Yup, we already have that. That's the reason why the equals({}) thing
> >> works :)
> >
> >
> > But == {} doesn't? Different conversions, etc, perhaps?
> >
> > (not quite sure why we have a function called 'equals' anyway, rather
> than
> > just using op==)
>
> Initializer lists just don't work in a binary operator, apparently it
> was too hard to parse so wg21 didn't allow it (what a lame excuse).
> http://stackoverflow.com/a/11445905 has some details.
>

Awww :( Thanks for the context.


>
> - Ben
>
> >>
> >> >
> >> >>
> >> >>
> >> >> Modified:
> >> >>     llvm/trunk/include/llvm/ADT/ArrayRef.h
> >> >>     llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
> >> >>     llvm/trunk/unittests/ADT/ArrayRefTest.cpp
> >> >>
> >> >> Modified: llvm/trunk/include/llvm/ADT/ArrayRef.h
> >> >> URL:
> >> >>
> >> >>
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/ArrayRef.h?rev=230907&r1=230906&r2=230907&view=diff
> >> >>
> >> >>
> >> >>
> ==============================================================================
> >> >> --- llvm/trunk/include/llvm/ADT/ArrayRef.h (original)
> >> >> +++ llvm/trunk/include/llvm/ADT/ArrayRef.h Sun Mar  1 15:05:05 2015
> >> >> @@ -11,7 +11,6 @@
> >> >>  #define LLVM_ADT_ARRAYREF_H
> >> >>
> >> >>  #include "llvm/ADT/None.h"
> >> >> -#include "llvm/ADT/STLExtras.h"
> >> >>  #include "llvm/ADT/SmallVector.h"
> >> >>  #include <vector>
> >> >>
> >> >> @@ -44,19 +43,6 @@ namespace llvm {
> >> >>      /// The number of elements.
> >> >>      size_type Length;
> >> >>
> >> >> -    /// \brief A dummy "optional" type that is only created by
> >> >> implicit
> >> >> -    /// conversion from a reference to T.
> >> >> -    ///
> >> >> -    /// This type must *only* be used in a function argument or as a
> >> >> copy
> >> >> of
> >> >> -    /// a function argument, as otherwise it will hold a pointer to
> a
> >> >> temporary
> >> >> -    /// past that temporaries' lifetime.
> >> >> -    struct TRefOrNothing {
> >> >> -      const T *TPtr;
> >> >> -
> >> >> -      TRefOrNothing() : TPtr(nullptr) {}
> >> >> -      TRefOrNothing(const T &TRef) : TPtr(&TRef) {}
> >> >> -    };
> >> >> -
> >> >>    public:
> >> >>      /// @name Constructors
> >> >>      /// @{
> >> >> @@ -202,47 +188,6 @@ namespace llvm {
> >> >>      }
> >> >>
> >> >>      /// @}
> >> >> -    /// @{
> >> >> -    /// @name Convenience methods
> >> >> -
> >> >> -    /// @brief Predicate for testing that the array equals the exact
> >> >> sequence of
> >> >> -    /// arguments.
> >> >> -    ///
> >> >> -    /// Will return false if the size is not equal to the exact
> number
> >> >> of
> >> >> -    /// arguments given or if the array elements don't equal the
> >> >> argument
> >> >> -    /// elements in order. Currently supports up to 16 arguments,
> but
> >> >> can
> >> >> -    /// easily be extended.
> >> >> -    bool equals(TRefOrNothing Arg0 = TRefOrNothing(),
> >> >> -                TRefOrNothing Arg1 = TRefOrNothing(),
> >> >> -                TRefOrNothing Arg2 = TRefOrNothing(),
> >> >> -                TRefOrNothing Arg3 = TRefOrNothing(),
> >> >> -                TRefOrNothing Arg4 = TRefOrNothing(),
> >> >> -                TRefOrNothing Arg5 = TRefOrNothing(),
> >> >> -                TRefOrNothing Arg6 = TRefOrNothing(),
> >> >> -                TRefOrNothing Arg7 = TRefOrNothing(),
> >> >> -                TRefOrNothing Arg8 = TRefOrNothing(),
> >> >> -                TRefOrNothing Arg9 = TRefOrNothing(),
> >> >> -                TRefOrNothing Arg10 = TRefOrNothing(),
> >> >> -                TRefOrNothing Arg11 = TRefOrNothing(),
> >> >> -                TRefOrNothing Arg12 = TRefOrNothing(),
> >> >> -                TRefOrNothing Arg13 = TRefOrNothing(),
> >> >> -                TRefOrNothing Arg14 = TRefOrNothing(),
> >> >> -                TRefOrNothing Arg15 = TRefOrNothing()) {
> >> >> -      TRefOrNothing Args[] = {Arg0,  Arg1,  Arg2,  Arg3, Arg4,
> Arg5,
> >> >> -                              Arg6,  Arg7,  Arg8,  Arg9, Arg10,
> Arg11,
> >> >> -                              Arg12, Arg13, Arg14, Arg15};
> >> >> -      if (size() > array_lengthof(Args))
> >> >> -        return false;
> >> >> -
> >> >> -      for (unsigned i = 0, e = size(); i != e; ++i)
> >> >> -        if (Args[i].TPtr == nullptr || (*this)[i] != *Args[i].TPtr)
> >> >> -          return false;
> >> >> -
> >> >> -      // Either the size is exactly as many args, or the next arg
> must
> >> >> be
> >> >> null.
> >> >> -      return size() == array_lengthof(Args) || Args[size()].TPtr ==
> >> >> nullptr;
> >> >> -    }
> >> >> -
> >> >> -    /// @}
> >> >>    };
> >> >>
> >> >>    /// MutableArrayRef - Represent a mutable reference to an array (0
> >> >> or
> >> >> more
> >> >>
> >> >> Modified: llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
> >> >> URL:
> >> >>
> >> >>
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=230907&r1=230906&r2=230907&view=diff
> >> >>
> >> >>
> >> >>
> ==============================================================================
> >> >> --- llvm/trunk/lib/Target/X86/X86ISelLowering.cpp (original)
> >> >> +++ llvm/trunk/lib/Target/X86/X86ISelLowering.cpp Sun Mar  1 15:05:05
> >> >> 2015
> >> >> @@ -19149,8 +19149,8 @@ static bool combineX86ShuffleChain(SDVal
> >> >>    //
> >> >>    // FIXME: Should teach these routines about AVX vector widths.
> >> >>    if (FloatDomain && VT.getSizeInBits() == 128) {
> >> >> -    if (Mask.equals(0, 0) || Mask.equals(1, 1)) {
> >> >> -      bool Lo = Mask.equals(0, 0);
> >> >> +    if (Mask.equals({0, 0}) || Mask.equals({1, 1})) {
> >> >> +      bool Lo = Mask.equals({0, 0});
> >> >>        unsigned Shuffle;
> >> >>        MVT ShuffleVT;
> >> >>        // Check if we have SSE3 which will let us use MOVDDUP. That
> >> >> instruction
> >> >> @@ -19179,8 +19179,8 @@ static bool combineX86ShuffleChain(SDVal
> >> >>        return true;
> >> >>      }
> >> >>      if (Subtarget->hasSSE3() &&
> >> >> -        (Mask.equals(0, 0, 2, 2) || Mask.equals(1, 1, 3, 3))) {
> >> >> -      bool Lo = Mask.equals(0, 0, 2, 2);
> >> >> +        (Mask.equals({0, 0, 2, 2}) || Mask.equals({1, 1, 3, 3}))) {
> >> >> +      bool Lo = Mask.equals({0, 0, 2, 2});
> >> >>        unsigned Shuffle = Lo ? X86ISD::MOVSLDUP : X86ISD::MOVSHDUP;
> >> >>        MVT ShuffleVT = MVT::v4f32;
> >> >>        if (Depth == 1 && Root->getOpcode() == Shuffle)
> >> >> @@ -19193,8 +19193,8 @@ static bool combineX86ShuffleChain(SDVal
> >> >>                      /*AddTo*/ true);
> >> >>        return true;
> >> >>      }
> >> >> -    if (Mask.equals(0, 0, 1, 1) || Mask.equals(2, 2, 3, 3)) {
> >> >> -      bool Lo = Mask.equals(0, 0, 1, 1);
> >> >> +    if (Mask.equals({0, 0, 1, 1}) || Mask.equals({2, 2, 3, 3})) {
> >> >> +      bool Lo = Mask.equals({0, 0, 1, 1});
> >> >>        unsigned Shuffle = Lo ? X86ISD::UNPCKL : X86ISD::UNPCKH;
> >> >>        MVT ShuffleVT = MVT::v4f32;
> >> >>        if (Depth == 1 && Root->getOpcode() == Shuffle)
> >> >> @@ -19213,11 +19213,11 @@ static bool combineX86ShuffleChain(SDVal
> >> >>    // variants as none of these have single-instruction variants that
> >> >> are
> >> >>    // superior to the UNPCK formulation.
> >> >>    if (!FloatDomain && VT.getSizeInBits() == 128 &&
> >> >> -      (Mask.equals(0, 0, 1, 1, 2, 2, 3, 3) ||
> >> >> -       Mask.equals(4, 4, 5, 5, 6, 6, 7, 7) ||
> >> >> -       Mask.equals(0, 0, 1, 1, 2, 2, 3, 3, 4, 4, 5, 5, 6, 6, 7, 7)
> ||
> >> >> -       Mask.equals(8, 8, 9, 9, 10, 10, 11, 11, 12, 12, 13, 13, 14,
> 14,
> >> >> 15,
> >> >> -                   15))) {
> >> >> +      (Mask.equals({0, 0, 1, 1, 2, 2, 3, 3}) ||
> >> >> +       Mask.equals({4, 4, 5, 5, 6, 6, 7, 7}) ||
> >> >> +       Mask.equals({0, 0, 1, 1, 2, 2, 3, 3, 4, 4, 5, 5, 6, 6, 7, 7})
> >> >> ||
> >> >> +       Mask.equals(
> >> >> +           {8, 8, 9, 9, 10, 10, 11, 11, 12, 12, 13, 13, 14, 14, 15,
> >> >> 15}))) {
> >> >>      bool Lo = Mask[0] == 0;
> >> >>      unsigned Shuffle = Lo ? X86ISD::UNPCKL : X86ISD::UNPCKH;
> >> >>      if (Depth == 1 && Root->getOpcode() == Shuffle)
> >> >> @@ -19706,7 +19706,7 @@ static SDValue PerformTargetShuffleCombi
> >> >>      // See if this reduces to a PSHUFD which is no more expensive
> and
> >> >> can
> >> >>      // combine with more operations. Note that it has to at least
> flip
> >> >> the
> >> >>      // dwords as otherwise it would have been removed as a no-op.
> >> >> -    if (Mask[0] == 2 && Mask[1] == 3 && Mask[2] == 0 && Mask[3] ==
> 1)
> >> >> {
> >> >> +    if (makeArrayRef(Mask).equals({2, 3, 0, 1})) {
> >> >>        int DMask[] = {0, 1, 2, 3};
> >> >>        int DOffset = N.getOpcode() == X86ISD::PSHUFLW ? 0 : 2;
> >> >>        DMask[DOffset + 0] = DOffset + 1;
> >> >> @@ -19745,12 +19745,8 @@ static SDValue PerformTargetShuffleCombi
> >> >>          int MappedMask[8];
> >> >>          for (int i = 0; i < 8; ++i)
> >> >>            MappedMask[i] = 2 * DMask[WordMask[i] / 2] + WordMask[i] %
> >> >> 2;
> >> >> -        const int UnpackLoMask[] = {0, 0, 1, 1, 2, 2, 3, 3};
> >> >> -        const int UnpackHiMask[] = {4, 4, 5, 5, 6, 6, 7, 7};
> >> >> -        if (std::equal(std::begin(MappedMask), std::end(MappedMask),
> >> >> -                       std::begin(UnpackLoMask)) ||
> >> >> -            std::equal(std::begin(MappedMask), std::end(MappedMask),
> >> >> -                       std::begin(UnpackHiMask))) {
> >> >> +        if (makeArrayRef(MappedMask).equals({0, 0, 1, 1, 2, 2, 3,
> 3})
> >> >> ||
> >> >> +            makeArrayRef(MappedMask).equals({4, 4, 5, 5, 6, 6, 7,
> 7}))
> >> >> {
> >> >>            // We can replace all three shuffles with an unpack.
> >> >>            V = DAG.getNode(ISD::BITCAST, DL, VT, D.getOperand(0));
> >> >>            DCI.AddToWorklist(V.getNode());
> >> >>
> >> >> Modified: llvm/trunk/unittests/ADT/ArrayRefTest.cpp
> >> >> URL:
> >> >>
> >> >>
> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/ArrayRefTest.cpp?rev=230907&r1=230906&r2=230907&view=diff
> >> >>
> >> >>
> >> >>
> ==============================================================================
> >> >> --- llvm/trunk/unittests/ADT/ArrayRefTest.cpp (original)
> >> >> +++ llvm/trunk/unittests/ADT/ArrayRefTest.cpp Sun Mar  1 15:05:05
> 2015
> >> >> @@ -57,24 +57,24 @@ TEST(ArrayRefTest, DropBack) {
> >> >>  TEST(ArrayRefTest, Equals) {
> >> >>    static const int A1[] = {1, 2, 3, 4, 5, 6, 7, 8};
> >> >>    ArrayRef<int> AR1(A1);
> >> >> -  EXPECT_TRUE(AR1.equals(1, 2, 3, 4, 5, 6, 7, 8));
> >> >> -  EXPECT_FALSE(AR1.equals(8, 1, 2, 4, 5, 6, 6, 7));
> >> >> -  EXPECT_FALSE(AR1.equals(2, 4, 5, 6, 6, 7, 8, 1));
> >> >> -  EXPECT_FALSE(AR1.equals(0, 1, 2, 4, 5, 6, 6, 7));
> >> >> -  EXPECT_FALSE(AR1.equals(1, 2, 42, 4, 5, 6, 7, 8));
> >> >> -  EXPECT_FALSE(AR1.equals(42, 2, 3, 4, 5, 6, 7, 8));
> >> >> -  EXPECT_FALSE(AR1.equals(1, 2, 3, 4, 5, 6, 7, 42));
> >> >> -  EXPECT_FALSE(AR1.equals(1, 2, 3, 4, 5, 6, 7));
> >> >> -  EXPECT_FALSE(AR1.equals(1, 2, 3, 4, 5, 6, 7, 8, 9));
> >> >> +  EXPECT_TRUE(AR1.equals({1, 2, 3, 4, 5, 6, 7, 8}));
> >> >> +  EXPECT_FALSE(AR1.equals({8, 1, 2, 4, 5, 6, 6, 7}));
> >> >> +  EXPECT_FALSE(AR1.equals({2, 4, 5, 6, 6, 7, 8, 1}));
> >> >> +  EXPECT_FALSE(AR1.equals({0, 1, 2, 4, 5, 6, 6, 7}));
> >> >> +  EXPECT_FALSE(AR1.equals({1, 2, 42, 4, 5, 6, 7, 8}));
> >> >> +  EXPECT_FALSE(AR1.equals({42, 2, 3, 4, 5, 6, 7, 8}));
> >> >> +  EXPECT_FALSE(AR1.equals({1, 2, 3, 4, 5, 6, 7, 42}));
> >> >> +  EXPECT_FALSE(AR1.equals({1, 2, 3, 4, 5, 6, 7}));
> >> >> +  EXPECT_FALSE(AR1.equals({1, 2, 3, 4, 5, 6, 7, 8, 9}));
> >> >>
> >> >>    ArrayRef<int> AR1a = AR1.drop_back();
> >> >> -  EXPECT_TRUE(AR1a.equals(1, 2, 3, 4, 5, 6, 7));
> >> >> -  EXPECT_FALSE(AR1a.equals(1, 2, 3, 4, 5, 6, 7, 8));
> >> >> +  EXPECT_TRUE(AR1a.equals({1, 2, 3, 4, 5, 6, 7}));
> >> >> +  EXPECT_FALSE(AR1a.equals({1, 2, 3, 4, 5, 6, 7, 8}));
> >> >>
> >> >>    ArrayRef<int> AR1b = AR1a.slice(2, 4);
> >> >> -  EXPECT_TRUE(AR1b.equals(3, 4, 5, 6));
> >> >> -  EXPECT_FALSE(AR1b.equals(2, 3, 4, 5, 6));
> >> >> -  EXPECT_FALSE(AR1b.equals(3, 4, 5, 6, 7));
> >> >> +  EXPECT_TRUE(AR1b.equals({3, 4, 5, 6}));
> >> >> +  EXPECT_FALSE(AR1b.equals({2, 3, 4, 5, 6}));
> >> >> +  EXPECT_FALSE(AR1b.equals({3, 4, 5, 6, 7}));
> >> >>  }
> >> >>
> >> >>  TEST(ArrayRefTest, EmptyEquals) {
> >> >>
> >> >>
> >> >> _______________________________________________
> >> >> llvm-commits mailing list
> >> >> llvm-commits at cs.uiuc.edu
> >> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >> >
> >> >
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150302/caf598dd/attachment.html>


More information about the llvm-commits mailing list