Revert "[llvm] Make Sequence reverse-iterable"

This reverts commit e772216e70
(and fixup 7f6c878a2c).

The build is broken with gcc5 host compiler:

In file included from
                 from mlir/lib/Dialect/Utils/StructuredOpsUtils.cpp:9:
tools/mlir/include/mlir/IR/BuiltinAttributes.h.inc:424:57: error: type/value mismatch at argument 1 in template parameter list for 'template<class ItTy, class FuncTy, class FuncReturnTy> class llvm::mapped_iterator'
                               std::function<T(ptrdiff_t)>>;
                                                         ^
tools/mlir/include/mlir/IR/BuiltinAttributes.h.inc:424:57: note:   expected a type, got 'decltype (seq<ptrdiff_t>(0, 0))::const_iterator'

llvm/include/llvm/ADT/Sequence.h

@@ -15,168 +15,71 @@
 #ifndef LLVM_ADT_SEQUENCE_H
 #define LLVM_ADT_SEQUENCE_H
 
-#include <cstddef>  // std::ptrdiff_t
-#include <iterator> // std::random_access_iterator_tag
+#include "llvm/ADT/iterator.h"
+#include "llvm/ADT/iterator_range.h"
+#include <algorithm>
+#include <iterator>
+#include <utility>
 
 namespace llvm {
 
 namespace detail {
 
-template <typename T, bool IsReversed> struct iota_range_iterator {
-  using iterator_category = std::random_access_iterator_tag;
-  using value_type = T;
-  using difference_type = std::ptrdiff_t;
-  using pointer = T *;
-  using reference = T &;
+template <typename ValueT>
+class value_sequence_iterator
+    : public iterator_facade_base<value_sequence_iterator<ValueT>,
+                                  std::random_access_iterator_tag,
+                                  const ValueT> {
+  using BaseT = typename value_sequence_iterator::iterator_facade_base;
 
-private:
-  struct Forward {
-    static void increment(T &V) { ++V; }
-    static void decrement(T &V) { --V; }
-    static void offset(T &V, difference_type Offset) { V += Offset; }
-    static T add(const T &V, difference_type Offset) { return V + Offset; }
-    static difference_type difference(const T &A, const T &B) { return A - B; }
-  };
-
-  struct Reverse {
-    static void increment(T &V) { --V; }
-    static void decrement(T &V) { ++V; }
-    static void offset(T &V, difference_type Offset) { V -= Offset; }
-    static T add(const T &V, difference_type Offset) { return V - Offset; }
-    static difference_type difference(const T &A, const T &B) { return B - A; }
-  };
-
-  using Op = std::conditional_t<!IsReversed, Forward, Reverse>;
+  ValueT Value;
 
 public:
-  // default-constructible
-  iota_range_iterator() = default;
-  // copy-constructible
-  iota_range_iterator(const iota_range_iterator &) = default;
-  // value constructor
-  explicit iota_range_iterator(T Value) : Value(Value) {}
-  // copy-assignable
-  iota_range_iterator &operator=(const iota_range_iterator &) = default;
-  // destructible
-  ~iota_range_iterator() = default;
+  using difference_type = typename BaseT::difference_type;
+  using reference = typename BaseT::reference;
 
-  // Can be compared for equivalence using the equality/inequality operators,
-  bool operator!=(const iota_range_iterator &RHS) const {
-    return Value != RHS.Value;
+  value_sequence_iterator() = default;
+  value_sequence_iterator(const value_sequence_iterator &) = default;
+  value_sequence_iterator(value_sequence_iterator &&Arg)
+      : Value(std::move(Arg.Value)) {}
+  value_sequence_iterator &operator=(const value_sequence_iterator &Arg) {
+    Value = Arg.Value;
+    return *this;
   }
-  bool operator==(const iota_range_iterator &RHS) const {
+
+  template <typename U, typename Enabler = decltype(ValueT(std::declval<U>()))>
+  value_sequence_iterator(U &&Value) : Value(std::forward<U>(Value)) {}
+
+  value_sequence_iterator &operator+=(difference_type N) {
+    Value += N;
+    return *this;
+  }
+  value_sequence_iterator &operator-=(difference_type N) {
+    Value -= N;
+    return *this;
+  }
+  using BaseT::operator-;
+  difference_type operator-(const value_sequence_iterator &RHS) const {
+    return Value - RHS.Value;
+  }
+
+  bool operator==(const value_sequence_iterator &RHS) const {
     return Value == RHS.Value;
   }
-
-  // Comparison
-  bool operator<(const iota_range_iterator &Other) const {
-    return Op::difference(Value, Other.Value) < 0;
-  }
-  bool operator<=(const iota_range_iterator &Other) const {
-    return Op::difference(Value, Other.Value) <= 0;
-  }
-  bool operator>(const iota_range_iterator &Other) const {
-    return Op::difference(Value, Other.Value) > 0;
-  }
-  bool operator>=(const iota_range_iterator &Other) const {
-    return Op::difference(Value, Other.Value) >= 0;
+  bool operator<(const value_sequence_iterator &RHS) const {
+    return Value < RHS.Value;
   }
 
-  // Dereference
-  T operator*() const { return Value; }
-  T operator[](difference_type Offset) const { return Op::add(Value, Offset); }
-
-  // Arithmetic
-  iota_range_iterator operator+(difference_type Offset) const {
-    return {Op::add(Value, Offset)};
-  }
-  iota_range_iterator operator-(difference_type Offset) const {
-    return {Op::add(Value, -Offset)};
-  }
-
-  // Iterator difference
-  difference_type operator-(const iota_range_iterator &Other) const {
-    return Op::difference(Value, Other.Value);
-  }
-
-  // Pre/Post Increment
-  iota_range_iterator &operator++() {
-    Op::increment(Value);
-    return *this;
-  }
-  iota_range_iterator operator++(int) {
-    iota_range_iterator Tmp = *this;
-    Op::increment(Value);
-    return Tmp;
-  }
-
-  // Pre/Post Decrement
-  iota_range_iterator &operator--() {
-    Op::decrement(Value);
-    return *this;
-  }
-  iota_range_iterator operator--(int) {
-    iota_range_iterator Tmp = *this;
-    Op::decrement(Value);
-    return Tmp;
-  }
-
-  // Compound assignment operators
-  iota_range_iterator &operator+=(difference_type Offset) {
-    Op::offset(Value, Offset);
-    return *this;
-  }
-  iota_range_iterator &operator-=(difference_type Offset) {
-    Op::offset(Value, -Offset);
-    return *this;
-  }
-
-private:
-  T Value;
+  reference operator*() const { return Value; }
 };
 
-} // namespace detail
+} // end namespace detail
 
-template <typename ValueT> struct iota_range {
-  static_assert(std::is_integral<ValueT>::value,
-                "ValueT must be an integral type");
-
-  using value_type = ValueT;
-  using reference = ValueT &;
-  using const_reference = const ValueT &;
-  using iterator = detail::iota_range_iterator<value_type, false>;
-  using const_iterator = iterator;
-  using reverse_iterator = detail::iota_range_iterator<value_type, true>;
-  using const_reverse_iterator = reverse_iterator;
-  using difference_type = typename iterator::difference_type;
-  using size_type = std::size_t;
-
-  value_type Begin;
-  value_type End;
-
-  template <
-      typename BeginT, typename EndT,
-      std::enable_if_t<std::is_convertible<BeginT, ValueT>::value, bool> = true,
-      std::enable_if_t<std::is_convertible<EndT, ValueT>::value, bool> = true>
-  iota_range(BeginT &&Begin, EndT &&End)
-      : Begin(std::forward<BeginT>(Begin)), End(std::forward<EndT>(End)) {}
-
-  size_t size() const { return End - Begin; }
-  bool empty() const { return Begin == End; }
-
-  auto begin() const { return const_iterator(Begin); }
-  auto end() const { return const_iterator(End); }
-
-  auto rbegin() const { return const_reverse_iterator(End - 1); }
-  auto rend() const { return const_reverse_iterator(Begin - 1); }
-
-private:
-  static_assert(std::is_same<ValueT, std::remove_cv_t<ValueT>>::value,
-                "ValueT must not be const nor volatile");
-};
-
-template <typename ValueT> auto seq(ValueT Begin, ValueT End) {
-  return iota_range<ValueT>(std::move(Begin), std::move(End));
+template <typename ValueT>
+iterator_range<detail::value_sequence_iterator<ValueT>> seq(ValueT Begin,
+                                                            ValueT End) {
+  return make_range(detail::value_sequence_iterator<ValueT>(Begin),
+                    detail::value_sequence_iterator<ValueT>(End));
 }
 
 } // end namespace llvm

llvm/unittests/ADT/SequenceTest.cpp

@@ -15,37 +15,26 @@ using namespace llvm;
 
 namespace {
 
-TEST(SequenceTest, Forward) {
-  int X = 0;
-  for (int I : seq(0, 10)) {
-    EXPECT_EQ(X, I);
-    ++X;
+TEST(SequenceTest, Basic) {
+  int x = 0;
+  for (int i : seq(0, 10)) {
+    EXPECT_EQ(x, i);
+    x++;
   }
-  EXPECT_EQ(10, X);
-}
+  EXPECT_EQ(10, x);
 
-TEST(SequenceTest, Backward) {
-  int X = 9;
-  for (int I : reverse(seq(0, 10))) {
-    EXPECT_EQ(X, I);
-    --X;
-  }
-  EXPECT_EQ(-1, X);
-}
+  auto my_seq = seq(0, 4);
+  EXPECT_EQ(4, my_seq.end() - my_seq.begin());
+  for (int i : {0, 1, 2, 3})
+    EXPECT_EQ(i, (int)my_seq.begin()[i]);
 
-TEST(SequenceTest, Distance) {
-  const auto Forward = seq(0, 10);
-  EXPECT_EQ(std::distance(Forward.begin(), Forward.end()), 10);
-  EXPECT_EQ(std::distance(Forward.rbegin(), Forward.rend()), 10);
-}
+  EXPECT_TRUE(my_seq.begin() < my_seq.end());
 
-TEST(SequenceTest, Dereferene) {
-  const auto Forward = seq(0, 10).begin();
-  EXPECT_EQ(Forward[0], 0);
-  EXPECT_EQ(Forward[2], 2);
-  const auto Backward = seq(0, 10).rbegin();
-  EXPECT_EQ(Backward[0], 9);
-  EXPECT_EQ(Backward[2], 7);
+  auto adjusted_begin = my_seq.begin() + 2;
+  auto adjusted_end = my_seq.end() - 2;
+  EXPECT_TRUE(adjusted_begin == adjusted_end);
+  EXPECT_EQ(2, *adjusted_begin);
+  EXPECT_EQ(2, *adjusted_end);
 }
 
 } // anonymous namespace

mlir/include/mlir/IR/BuiltinAttributes.td

@@ -707,7 +707,7 @@ def Builtin_SparseElementsAttr
   let extraClassDeclaration = [{
     template <typename T>
     using iterator =
-        llvm::mapped_iterator<decltype(llvm::seq<ptrdiff_t>(0, 0))::const_iterator,
+        llvm::mapped_iterator<llvm::detail::value_sequence_iterator<ptrdiff_t>,
                               std::function<T(ptrdiff_t)>>;
 
     /// Return the values of this attribute in the form of the given type 'T'.

mlir/lib/Conversion/PDLToPDLInterp/PDLToPDLInterp.cpp

@@ -380,8 +380,8 @@ void PatternLowering::generateSwitch(SwitchNode *switchNode,
   if (kind == Predicates::OperandCountAtLeastQuestion ||
       kind == Predicates::ResultCountAtLeastQuestion) {
     // Order the children such that the cases are in reverse numerical order.
-    auto sequence = llvm::seq<unsigned>(0, switchNode->getChildren().size());
-    SmallVector<unsigned> sortedChildren(sequence.begin(), sequence.end());
+    SmallVector<unsigned> sortedChildren(
+        llvm::seq<unsigned>(0, switchNode->getChildren().size()));
     llvm::sort(sortedChildren, [&](unsigned lhs, unsigned rhs) {
       return cast<UnsignedAnswer>(switchNode->getChild(lhs).first)->getValue() >
              cast<UnsignedAnswer>(switchNode->getChild(rhs).first)->getValue();

mlir/lib/Dialect/Linalg/Transforms/Vectorization.cpp

@@ -302,8 +302,8 @@ static VectorizationResult vectorizeLinalgIndex(OpBuilder &b, Operation *op,
   // Compute the static loop sizes of the index op.
   auto targetShape = linalgOp.computeStaticLoopSizes();
   // Compute a one-dimensional index vector for the index op dimension.
-  auto seq1 = llvm::seq<int64_t>(0, targetShape[indexOp.dim()]);
-  SmallVector<int64_t> constantSeq(seq1.begin(), seq1.end());
+  SmallVector<int64_t> constantSeq(
+      llvm::seq<int64_t>(0, targetShape[indexOp.dim()]));
   ConstantOp constantOp =
       b.create<ConstantOp>(loc, b.getIndexVectorAttr(constantSeq));
   // Return the one-dimensional index vector if it lives in the trailing
@@ -317,8 +317,8 @@ static VectorizationResult vectorizeLinalgIndex(OpBuilder &b, Operation *op,
   std::swap(targetShape[indexOp.dim()], targetShape.back());
   auto broadCastOp = b.create<vector::BroadcastOp>(
       loc, VectorType::get(targetShape, b.getIndexType()), constantOp);
-  auto seq2 = llvm::seq<int64_t>(0, linalgOp.getNumLoops());
-  SmallVector<int64_t> transposition(seq2.begin(), seq2.end());
+  SmallVector<int64_t> transposition(
+      llvm::seq<int64_t>(0, linalgOp.getNumLoops()));
   std::swap(transposition.back(), transposition[indexOp.dim()]);
   auto transposeOp =
       b.create<vector::TransposeOp>(loc, broadCastOp, transposition);