[llvm] Add enum iteration to Sequence

[gchatelet]

Summary

This patch allows iterating typed enum via the ADT/Sequence utility.

[• Quuxplusone]

llvm/include/llvm/ADT/Sequence.h

210–223

I don't think it makes sense to force the caller to decide between seq and enum_seq spellings. If I were implementing this facility, I'd just give iota_range a second template type parameter:

template<class V, class U>
struct iota_range_iterator {
    using value_type = V;
    V operator*() const { return static_cast<V>(value_); }
    ~~~
private:
    U value_;
};

and then here instantiate iota_range<EnumType, std::underlying_type_t<EnumType>>.
On line 250 below, you make an iota_range_iterator<IterableEnum<ValueE>>, whose operator* gives back values of type IterableEnum<ValueE> instead of ValueE — that's going to be really confusing to the caller. I wouldn't do that.

template <class T, std::enable_if_t<std::is_integral<T>::value, int> = 0>
auto seq(T Begin, T End) {
  return iota_range_iterator<T, T>(Begin, End);
}

template <class T, std::enable_if_t<std::is_enum<T>::value, int> = 0>
auto seq(T Begin, T End) {
  using U = std::underlying_type_t<T>;
  return iota_range_iterator<T, U>(U(Begin), U(End));
}

Notice you don't need std::move because these types are integral.

llvm/include/llvm/CodeGen/ValueTypes.h

43 ↗

(On Diff #350610)

Get rid of IterableEnum as I propose above, and then you can get rid of this new overload. Ditto throughout.

[gchatelet]

llvm/include/llvm/ADT/Sequence.h

210–223

Thx for helping me taking a step back.
I originally wanted to separate concerns (iteration and typing) but the fact that IterableEnum<T> in not T is problematic indeed. The abstraction is leaking.

Now conflating the two orthogonal aspect make the implementation a bit more complex but I think it's worth it.

There is an additional design point that you may have missed. seq has the C++ iterator semantic, i.e. the end value is exclusive.
When iterating enums we want end to be included.
That's a problem with typed enums as user can't write seq(Enum::First, Enum::Last + 1). We are not allowed to do arithmetic on typed enums (and TBH it's cumbersome to write).

I've fixed this by adding another method seq_inclusive but I'm not too happy about the naming. Suggestions welcome.
Adding a different method does not compose well with overloading seq based on is_integral/is_enum so I moved the type resolution inside iota_range directly. Let me know what you think.

[gchatelet]

• Trying a different approach

[• Quuxplusone]

llvm/include/llvm/ADT/Sequence.h

210–223

There is an additional design point [...] When iterating enums we want end to be included.

Yuck. I had missed that. seq_inclusive seems like a decent solution to me. If you really wanted to be explicit everywhere, you could have a llvm::halfopen_seq and an llvm::closed_seq... but I assume that's a bad idea because the half-open version is used all over the place and we like the shortness of its name?
Another option would be a "fluent interface" via member function:

for (int i : llvm::seq<int>(0, 10)) { /* loop from 0 to 9 */ }
for (int i : llvm::seq<int>(1, 10).inclusive()) { /* loop from 1 to 10 */ }

Watch out, though: the trivial implementation of .inclusive() would lead to integer overflow in llvm::seq<int>(INT_MIN, INT_MAX).inclusive() and so you'd have to think about what you wanted to do there: make it work, at the cost of complicated implementation? make it runtime-assert-fail? just ignore the potential overflow? find a simple implementation I'm not thinking of off the top of my head? ;)

I definitely agree that we do not want seq<E>(x, y) to iterate over one more element than seq<underlying_type_t<E>>(x, y); the enum-ness of the type parameter should affect only the enum-ness of the value_type, definitely not the number of things iterated over.

Btw, some enumeration types do have "one-past-the-end" values, as in

enum E { Apple, Orange, Banana, MaxFruit = Banana+1 };
for (E fruit : llvm::halfopen_seq<E>(Apple, MaxFruit)) { /* is correct */ }

and of course all integer types do have "closed" range forms as well:

for (int i : llvm::closed_seq<int>(INT_MIN, INT_MAX)) { /* is correct */ }
for (int i : llvm::seq<int>(INT_MIN, INT_MAX).inclusive()) { /* is correct */ }

[gchatelet]

• rebase and use seq_inclusive in a few places

[gchatelet]

llvm/include/llvm/ADT/Sequence.h

210–223

There is an additional design point [...] When iterating enums we want end to be included.

Yuck. I had missed that. seq_inclusive seems like a decent solution to me. If you really wanted to be explicit everywhere, you could have a llvm::halfopen_seq and an llvm::closed_seq... but I assume that's a bad idea because the half-open version is used all over the place and we like the shortness of its name?

I count about 150 call sites most of them are exclusive so yeah, I'd prefer to keep the most used version short.

Another option would be a "fluent interface" via member function:

for (int i : llvm::seq<int>(0, 10)) { /* loop from 0 to 9 */ }
for (int i : llvm::seq<int>(1, 10).inclusive()) { /* loop from 1 to 10 */ }

I usually like fluent interfaces but it has to be omnipotent (setting a value). The way I implemented it would lead to bugs if client calls llvm::seq<int>(1, 10).inclusive().inclusive().
There are ways to prevent it (adding more state to iota_range) but I feel like handling it as a separate function works quite well.

Watch out, though: the trivial implementation of .inclusive() would lead to integer overflow in llvm::seq<int>(INT_MIN, INT_MAX).inclusive() and so you'd have to think about what you wanted to do there: make it work, at the cost of complicated implementation? make it runtime-assert-fail? just ignore the potential overflow? find a simple implementation I'm not thinking of off the top of my head? ;)

I'll go with runtime assert.

I definitely agree that we do not want seq<E>(x, y) to iterate over one more element than seq<underlying_type_t<E>>(x, y); the enum-ness of the type parameter should affect only the enum-ness of the value_type, definitely not the number of things iterated over.

Yeah they ought to be orthogonal.

Btw, some enumeration types do have "one-past-the-end" values, as in

enum E { Apple, Orange, Banana, MaxFruit = Banana+1 };
for (E fruit : llvm::halfopen_seq<E>(Apple, MaxFruit)) { /* is correct */ }

and of course all integer types do have "closed" range forms as well:

for (int i : llvm::closed_seq<int>(INT_MIN, INT_MAX)) { /* is correct */ }
for (int i : llvm::seq<int>(INT_MIN, INT_MAX).inclusive()) { /* is correct */ }

Let me add more tests to make sure these cases are handled.

[• Quuxplusone]

llvm/include/llvm/ADT/Sequence.h

123–129

llvm::detail::StorageType is too generic of a name to land-grab, IMHO. This can/should be an implementation detail of iota_range_iterator — maybe it doesn't even need to be a template parameter.

template<class T>
struct iota_range_iterator {
    using storage_type = typename std::conditional_t<std::is_enum<T>::value,
        type_identity<T>, std::underlying_type<T>>::type;

Technically, instantiating underlying_type<T> for non-enum T was UB before C++20.
In practice I'd hope it works on all implementations we care about.
I don't know where to get a C++14-friendly type_identity but hopefully you can figure it out.
https://en.cppreference.com/w/cpp/types/type_identity

[• Quuxplusone]

llvm/include/llvm/ADT/Sequence.h

195

I've just noticed that this is unidiomatic: on normal iterators, --begin() would be UB. (std::reverse_iterator always stores one-past-where-it's-"pointing"-to, for this reason.) I guess it's OK here, but if you've got the time, maybe think about some way to avoid the hiccup here.

219

Phab somehow ate my comment here.
This should be just return seq(Begin, static_cast<T>(End + 1));
and then in the constructor of iota_range, please assert(Begin <= End) just as a sanity check.

[gchatelet]

llvm/include/llvm/ADT/Sequence.h

123–129

llvm::detail::StorageType is too generic of a name to land-grab, IMHO. This can/should be an implementation detail of iota_range_iterator — maybe it doesn't even need to be a template parameter.
Technically, instantiating underlying_type<T> for non-enum T was UB before C++20.
In practice I'd hope it works on all implementations we care about.

Unfortunately no, std::conditional must evaluated both types, and std::underlying_type<T> does not provide the type member when T is not an enum. The standard seems to specify this use case explicitly.
https://en.cppreference.com/w/cpp/types/underlying_type

I had to provide type resolution via template specialization.
Because of how nested template specialization works, I can't make StorageType part of iota_range_iterator either so it has to live outside of iota_range_iterator.

I tried many different solutions but I couldn't make it much better.

I renamed StorageType to iota_range_iterator_storage_type so at least it's less susceptible to accidental hijacking.

I don't know where to get a C++14-friendly type_identity but hopefully you can figure it out.
https://en.cppreference.com/w/cpp/types/type_identity

type_identity doesn't help in that case.

219

Phab somehow ate my comment here.
This should be just return seq(Begin, static_cast<T>(End + 1));

Because End may be a scoped enum End + 1 is not always a valid statement.
Trying to do the arithmetic here will yield the same problems that IterableEnum was supposed to solve.
The best we could do here would be something along those lines:

return seq(Begin, static_cast<T>(static_cast<detail::StorageType<T>::type>(End) + 1));

Which, IMHO, is redoing most of the work that has been moved to iota_range_iterator (hence the use of iterators to bump the values)

[gchatelet]

• rename StorageType to iota_range_iterator_storage_type

[• Quuxplusone]

llvm/include/llvm/ADT/Sequence.h

123–129

I'd mixed up the arguments to conditional_t. Try this:
https://godbolt.org/z/q9fxznja7

[gchatelet]

• Use std::conditional instead of custom specialization

llvm/include/llvm/ADT/Sequence.h

123–129

oh... my apologies. I should have checked as well. I had convinced myself that the error was std::conditional_t needing to resolve both types ...

So I had to provide a C++14 type_identity but it works.

[gchatelet]

-rebase and address comments

[gchatelet]

Please take another look @Quuxplusone .

llvm/include/llvm/ADT/Sequence.h

195

I've added a note as I couldn't find a better implementation.

[• Quuxplusone]

llvm/include/llvm/ADT/Sequence.h

211–222

I would say "from Begin to End, exclusive of End" — or more colloquially but clearer IMO, "from Begin up to but not including End."

[• Quuxplusone]

llvm/include/llvm/ADT/Sequence.h

219

Yeah, I guess since End + 1 doesn't work for scoped enums, I'm willing to live with *++result.end().

Or... Maybe iota_range should just take three arguments? Does this work?

explicit iota_range_iterator(T Value, bool plus1)
    : Value(static_cast<U>(Value) + plus1) {}

explicit iota_range(T Begin, T End, bool inclusive)
    : Begin(Begin, false), End(End, inclusive) {
  assert(Begin <= End);
}

// seq
return iota_range<T>(Begin, End, false);

// seq_inclusive
return iota_range<T>(Begin, End, true);

llvm/unittests/ADT/SequenceTest.cpp

77–84

I'd also test reverse(seq_inclusive(min, min)).
You shouldn't need the <T> anywhere on lines 72-84.

llvm/unittests/IR/ConstantRangeTest.cpp

1554–1556

You shouldn't need the explicit template argument here.

[gchatelet]

• Address comments

llvm/include/llvm/ADT/Sequence.h

219

Begin and End are not iterators but values (BTW I renamed them to avoid confusion), this is needed because of rbegin()/rend() must be constructed from values.

Now this can be handled by adding an additional member but then it gets really confusing when creating the reverse iterators:

value_type Begin;
value_type End;
bool Inclusive;

explicit iota_range(T Begin, T End, bool Inclusive)
    : Begin(Begin), End(End), Inclusive(Inclusive) {
  assert(Begin <= End);
}

auto begin() const { return const_iterator(Begin, false); }
auto end() const { return const_iterator(End, Inclusive); }

auto rbegin() const { return const_reverse_iterator(*--end(), false); }
auto rend() const { return const_reverse_iterator(*--begin(), false); } // <- false here is confusing

llvm/unittests/ADT/SequenceTest.cpp

77–84

Unfortunately I do because + 1 and - 1 trigger integer promotion which makes both arguments of different types.
I can introduce variables though.

[gchatelet]

llvm/include/llvm/ADT/Sequence.h

182–223

@Quuxplusone when using seq_inclusive with End == std::numeric_limits<T>::max() iteration works fine but iterator comparison is broken.
There are two possibilities here:

1. Leave it as-is and document the broken behavior while allowing inclusive End being INT_MAX
2. assert when using seq_inclusive and End == std::numeric_limits<T>::max(), effectively disabling iteration up to - and including - INT_MAX.

The current implementation is for 1. but I think 2. is more reasonable.
What do you think?

[• Quuxplusone]

llvm/include/llvm/ADT/Sequence.h

188–197

Hmm. I suggest

{
  assert(Begin <= End);
  // This assertion forbids iota_range(0, UINT_MAX, true)
  // which would otherwise be treated as an empty range,
  // and iota_range(1, UINT_MAX, true) which would otherwise
  // report that s.begin() > s.end().
  // iota_range(INT_MIN, INT_MAX, true) will have already caused
  // undefined overflow, but this assertion will likely catch it too.
  if (Inclusive)
    assert(End < PastEndValue);
}

FWIW, I like your renaming of Begin to BeginValue etc.

llvm/unittests/ADT/SequenceTest.cpp

77–84

Ah, I see. (FWIW, I'd bikeshed the names to minp1 and maxm1, but whatever.)

[gchatelet]

• rebase + prevent overflow of PastEndValue + tests

[gchatelet]

llvm/include/llvm/ADT/Sequence.h

188–197

Hmm. I suggest

{
  assert(Begin <= End);
  // This assertion forbids iota_range(0, UINT_MAX, true)
  // which would otherwise be treated as an empty range,
  // and iota_range(1, UINT_MAX, true) which would otherwise
  // report that s.begin() > s.end().
  // iota_range(INT_MIN, INT_MAX, true) will have already caused
  // undefined overflow, but this assertion will likely catch it too.
  if (Inclusive)
    assert(End < PastEndValue);
}

The assert didn't trigger in the case of iota_range(INT_MIN, INT_MAX, true) so I used numeric_limits.
Tests are passing.

[• Quuxplusone]

llvm/include/llvm/ADT/Sequence.h

191–192

I believe teeechnically this is still UB, because if the enumeration has no specified underlying type, and its enumerator values are e.g. enum E { A=1, B=2, C=3 }, then it's UB to evaluate E(4). (It basically works like a bit-field: you can evaluate E(mask) for any mask that doesn't use bits beyond the highest enumerator's bits.) But in practice I think this is fine.
For readability, I would still prefer to see it with an if:

if (Inclusive)
  assert(End != T(std::numeric_limits<typename iterator::underlying_type>::max()));

Although now you've got me confused again about why we're storing BeginValue and PastEndValue as value_type (i.e. E) instead of underlying_type. How does your size() method work when value_type is an enum type? I think data "at rest" should always be stored in the underlying integer type; the only time it should ever be converted to value_type (i.e. E) is when the iterator is being dereferenced.

[gchatelet]

• Address comments
• Fix clang-tidy warning

llvm/include/llvm/ADT/Sequence.h

191–192

I believe teeechnically this is still UB, because if the enumeration has no specified underlying type, and its enumerator values are e.g. enum E { A=1, B=2, C=3 }, then it's UB to evaluate E(4). (It basically works like a bit-field: you can evaluate E(mask) for any mask that doesn't use bits beyond the highest enumerator's bits.) But in practice I think this is fine.

Thx! So I'm now moving all the logic in the underlying type world.
Technically End + 1 is implementation defined for signed types but it's guarded by the assertion so it's a logical error before being implementation defined.

Although now you've got me confused again about why we're storing BeginValue and PastEndValue as value_type (i.e. E) instead of underlying_type. How does your size() method work when value_type is an enum type? I think data "at rest" should always be stored in the underlying integer type; the only time it should ever be converted to value_type (i.e. E) is when the iterator is being dereferenced.

Thx a lot for challenging me on this one. You are correct that we can use underlying_type instead of value_type directly into iota_range.
I've moved things around: I think it's much better now. Let me know if you think this can be improved further.

[gchatelet]

• Fix another clang-tidy warning

[• Quuxplusone]

I probably shouldn't be the only person to accept this PR, but it looks fine to me at this point.

llvm/include/llvm/ADT/Sequence.h

28

Nits: Please add a newline before struct; and consider renaming/reordering the parameters so that T comes before U. I think that should be easy and mechanical (if tedious), because neither T nor U is deduced nor defaulted.

195

Note that BeginValue-1 might be UB; e.g. seq(INT_MIN, 0).rend(). But I think it's fine to ignore this corner case.

218

"End is not allowed to be"... what?

llvm/unittests/ADT/SequenceTest.cpp

54

Nit: I don't see why you bother defining LAST when you could just say E below.

[gchatelet]

• Trying a different approach
• rebase and use seq_inclusive in a few places
• rename StorageType to iota_range_iterator_storage_type
• Use std::conditional instead of custom specialization
• Address comments
• Address comments
• Prevent overflow of PastEndValue + tests
• Address comments
• Fix clang-tidy warning
• Fix another clang-tidy warning
• Fix UB in reverse iteration with signed type

[gchatelet]

llvm/include/llvm/ADT/Sequence.h

28

Nits: Please add a newline before struct;

clang-format puts it back to the same line :-/

and consider renaming/reordering the parameters so that T comes before U. I think that should be easy and mechanical (if tedious), because neither T nor U is deduced nor defaulted.

Done.

195

Note that BeginValue-1 might be UB; e.g. seq(INT_MIN, 0).rend(). But I think it's fine to ignore this corner case.

Fixed it.

[courbet]

llvm/include/llvm/ADT/Sequence.h

159

So you can't iota_range and enum with uint8_t underlying type and 256 values ? That seems a bit artificial.
Maybe we could always use intmax_t (resp. uintmax_t for unsigned underlying types) as raw_type ?

[courbet]

llvm/include/llvm/ADT/Sequence.h

220

Maybe also note that all enum values are generated, even those that do not appear in the enumerator list ?

[gchatelet]

• Added warning on validity of enum range iteration.
• Use uintmax_t/intmax_t types for storage.

[gchatelet]

llvm/include/llvm/ADT/Sequence.h

159

So you can't iota_range and enum with uint8_t underlying type and 256 values ? That seems a bit artificial.
Maybe we could always use intmax_t (resp. uintmax_t for unsigned underlying types) as raw_type ?

Fair enough. I'm slightly concerned about integer promotion/conversion rules but it should be safe (famous last words).

[gchatelet]

• rebase and squash commits

[gchatelet]

@Quuxplusone are you still OK with the last changes? If so I think I can submit.

[• Quuxplusone]

llvm/include/llvm/ADT/Sequence.h

159

Well, now I'm wondering why we don't just use uintptr_t all the time. Why does the storage type ever have to be signed intptr_t?
I think the rationale has to do with ensuring that r.begin() < r.end(); but maybe that just indicates that Op::difference is computing its return value via the wrong expression.
But anyway, I continue to be uninterested in blocking this at this point. :)

Please do verify that the test failures in the latest run aren't your fault (e.g. by rebasing and letting the tests run again) before committing.

213–215

Maybe "seq will generate each consecutive value, even if no enumerator with that value exists."

[gchatelet]

• Updated enum documentation

[gchatelet]

llvm/include/llvm/ADT/Sequence.h

159

Well, now I'm wondering why we don't just use uintptr_t all the time. Why does the storage type ever have to be signed intptr_t?

So there are enums that explicitly use negative values (e.g. FloatingPointMode.h:44) so I think it's reasonable to allow negative values.
Also, I think it's a valid use case to use seq with negative values as well (e.g. seq<int>(-10, 0)).

I think the rationale has to do with ensuring that r.begin() < r.end(); but maybe that just indicates that Op::difference is computing its return value via the wrong expression.
But anyway, I continue to be uninterested in blocking this at this point. :)

Thx a lot for the time you spent on the review. Highly appreciate it.

Please do verify that the test failures in the latest run aren't your fault (e.g. by rebasing and letting the tests run again) before committing.

Yes.

[• Quuxplusone]

llvm/include/llvm/ADT/Sequence.h

159

For the record, I was talking about the storage type (raw_type) being unsigned, not the value type (value_type). I certainly agree that the value_type needs to be able to be signed or unsigned, enum or integral. But the reason you ever need anything besides uintptr_t as the raw_type is much subtler and harder to see.

[gchatelet]

This is currently blocked on failing tests see https://reviews.llvm.org/D83351#2857797 for reference.

[gchatelet]

• Fix iteration over AttributeList

[• Quuxplusone]

llvm/tools/llvm-reduce/deltas/ReduceAttributes.cpp

87–91 ↗

(On Diff #356499)

LGTM, but per @lebedev.ri 's comment in D83351, I think this would benefit from a follow-up patch that turns index_begin and index_end into signed ints.
Style nit: Personally I'd rather see "unsigned(-1)" and "-1, 0, 1...", than "unsigned(~0)" and "~0, 0, 1...".

llvm/unittests/ADT/SequenceTest.cpp

118–119

Pre-existing consistency issue: I see all of the following in llvm/unittests/, and they can't all be right:

#if !defined(NDEBUG) && GTEST_HAS_DEATH_TEST  // 1
#if defined(GTEST_HAS_DEATH_TEST) && !defined(NDEBUG)  // 2
#ifndef NDEBUG  // 3

and as a two-liner:

#ifdef GTEST_HAS_DEATH_TEST
#ifndef NDEBUG  // 4

In a vacuum, I'd recommend (2) above, because once the #if condition mentions DEATH, then your comment on line 118 becomes redundant and can be removed.

[lebedev.ri]

llvm/tools/llvm-reduce/deltas/ReduceAttributes.cpp

87–91 ↗

(On Diff #356499)

This change NACK.
It should probably be something like

for (unsigned SetIdx : seq<int>(AL.index_begin(), AL.index_end())) {

or

for (unsigned SetIdx : seq((int)AL.index_begin(), (int)AL.index_end())) {

[• Quuxplusone]

llvm/tools/llvm-reduce/deltas/ReduceAttributes.cpp

87–91 ↗

(On Diff #356499)

I approve of seq((int)AL.index_begin(), (int)AL.index_end()) but not seq<int>(AL.index_begin(), AL.index_end()) — I don't want people writing explicit template arguments all over the place.
Regardless, this would still benefit from a followup patch to actually change the type of index_begin so that we don't have to cast it on every use.

[gchatelet]

• Fix DEATH_TEST guard

llvm/tools/llvm-reduce/deltas/ReduceAttributes.cpp

87–91 ↗

(On Diff #356499)

Ideally, AttributeList should be changed to preserve iterator semantic, this was the intention of one of the previous author (Read summary of D32811).
Unfortunately it seems a bit more work than just renaming the enum.

I went for the very explicit version because using int involves two casts:

• two from unsigned to signed which are implementation defined and thus non standard.
• one from signed to unsigned which is well defined and uses modulo 2 arithmetic.

See Integer conversions in https://en.cppreference.com/w/c/language/conversion

I think it's important to make sure the code is crystal clear and the double cast makes it non trivial to reason about.

We can:

1. Use for (unsigned SetIdx : seq<int>(AL.index_begin(), AL.index_end())) although I think it's implementation defined and a fix for something that is broken elsewhere.
2. Write the loop explicitly explaining why we have to do it this way, allegedly not fixing the real problem either.
3. Invest extra time to fix the issue in AttributeList so it respects begin<=end.

WDYT?

[gchatelet]

FYI I'm fixing AttributeList indexing in D105485.

[gchatelet]

• rebase and use the same iterating scheme as in D105485.

[gchatelet]

llvm/tools/llvm-reduce/deltas/ReduceAttributes.cpp

87–91 ↗

(On Diff #356499)

Since I'm blocked on D105485, I'll submit this one using the idiomatic way of iterating through AttributeList. This is consistent with what I did in D105485 as well.

 % git grep \.index_begin\(
include/llvm/IR/Attributes.h:764:  unsigned index_begin() const { return AttributeList::FunctionIndex; }
lib/Bitcode/Writer/BitcodeWriter.cpp:836:    for (unsigned i = AL.index_begin(), e = AL.index_end(); i != e; ++i) {
lib/Bitcode/Writer/ValueEnumerator.cpp:1039:  for (unsigned i = PAL.index_begin(), e = PAL.index_end(); i != e; ++i) {
lib/IR/Attributes.cpp:1550:  for (unsigned i = index_begin(), e = index_end(); i != e; ++i) {
lib/Transforms/Utils/FunctionComparator.cpp:113:  for (unsigned i = L.index_begin(), e = L.index_end(); i != e; ++i) {

[thakis]

This breaks check-llvm on arm macs: http://45.33.8.238/macm1/13855/step_11.txt

[gchatelet]

In D103900#2874384, @thakis wrote:

This breaks check-llvm on arm macs: http://45.33.8.238/macm1/13855/step_11.txt

Thx for reporting. I'll revert the patch asap.

llvm/include/llvm/ADT/Sequence.h

 /// values. The names are intentionally kept very short as they tend to occur
 /// in common and widely used contexts.
 ///
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_ADT_SEQUENCE_H
 #define LLVM_ADT_SEQUENCE_H
 
-#include <cstddef>  //std::ptrdiff_t
-#include <iterator> //std::random_access_iterator_tag
+#include <cassert>     // assert
+#include <cstddef>     // std::ptrdiff_t
+#include <iterator>    // std::random_access_iterator_tag
+#include <limits>      // std::numeric_limits
+#include <type_traits> // std::underlying_type, std::is_enum
 
 namespace llvm {
 
 namespace detail {
 
-template <typename T, bool IsReversed> struct iota_range_iterator {
+template <typename T, typename U, 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 &;
 
-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>;
-
-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) {}
+  explicit iota_range_iterator(U Value) : Value(Value) {}
   // copy-assignable
   iota_range_iterator &operator=(const iota_range_iterator &) = default;
   // destructible
   ~iota_range_iterator() = default;
 
   // Can be compared for equivalence using the equality/inequality operators,
   bool operator!=(const iota_range_iterator &RHS) const {
     return Value != RHS.Value;
template <typename T, typename U, bool IsReversed> struct iota_range_iterator {
   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;
   }
 
   // Dereference
-  T operator*() const { return Value; }
-  T operator[](difference_type Offset) const { return Op::add(Value, Offset); }
+  T operator*() const { return static_cast<T>(Value); }
+  T operator[](difference_type Offset) const {
+    return static_cast<T>(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)};
   }
iota_range_iterator &operator+=(difference_type Offset) {
     return *this;
   }
   iota_range_iterator &operator-=(difference_type Offset) {
     Op::offset(Value, -Offset);
     return *this;
   }
 
 private:
-  T Value;
+  struct Forward {
+    static void increment(U &V) { ++V; }
+    static void decrement(U &V) { --V; }
+    static void offset(U &V, difference_type Offset) { V += Offset; }
+    static U add(const U &V, difference_type Offset) { return V + Offset; }
+    static difference_type difference(const U &A, const U &B) {
+      return difference_type(A) - difference_type(B);
+    }
   };
 
+  struct Reverse {
+    static void increment(U &V) { --V; }
+    static void decrement(U &V) { ++V; }
+    static void offset(U &V, difference_type Offset) { V -= Offset; }
+    static U add(const U &V, difference_type Offset) { return V - Offset; }
+    static difference_type difference(const U &A, const U &B) {
+      return difference_type(B) - difference_type(A);
+    }
+  };
+
+  using Op = std::conditional_t<!IsReversed, Forward, Reverse>;
+
+  U Value;
+};
+
+// Providing std::type_identity for C++14.
+template <class T> struct type_identity { using type = T; };
+
 } // 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>;
+template <typename T> struct iota_range {
+private:
+  using raw_type = typename std::conditional_t<std::is_enum<T>::value,
+                                               std::underlying_type<T>,
+                                               detail::type_identity<T>>::type;
+
+  static raw_type compute_past_end(raw_type End, bool Inclusive) {
+    if (Inclusive) {
+      // This assertion forbids overflow of `PastEndValue`.
+      assert(End != std::numeric_limits<raw_type>::max() &&
+             "Forbidden End value for seq_inclusive.");
+      return End + 1;
+    }
+    return End;
+  }
+  static raw_type raw(T Value) { return static_cast<raw_type>(Value); }
+
+  raw_type BeginValue;
+  raw_type PastEndValue;
+
+public:
+  using value_type = T;
+  using reference = T &;
+  using const_reference = const T &;
+  using iterator = detail::iota_range_iterator<value_type, raw_type, false>;
   using const_iterator = iterator;
-  using reverse_iterator = detail::iota_range_iterator<value_type, true>;
+  using reverse_iterator =
+      detail::iota_range_iterator<value_type, raw_type, true>;
   using const_reverse_iterator = reverse_iterator;
   using difference_type = std::ptrdiff_t;
   using size_type = std::size_t;
 
-  value_type Begin;
-  value_type End;
-
-  explicit iota_range(ValueT Begin, ValueT End) : Begin(Begin), End(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); }
+  explicit iota_range(T Begin, T End, bool Inclusive)
+      : BeginValue(raw(Begin)),
+        PastEndValue(compute_past_end(raw(End), Inclusive)) {
+    assert(Begin <= End && "Begin must be less or equal to End.");
+  }
+
+  size_t size() const { return PastEndValue - BeginValue; }
+  bool empty() const { return BeginValue == PastEndValue; }
+
+  auto begin() const { return const_iterator(BeginValue); }
+  auto end() const { return const_iterator(PastEndValue); }
+
+  auto rbegin() const { return const_reverse_iterator(PastEndValue - 1); }
+  auto rend() const {
+    assert(std::is_unsigned<raw_type>::value ||
+           BeginValue != std::numeric_limits<raw_type>::min() &&
+               "Forbidden Begin value for reverse iteration");
+    return const_reverse_iterator(BeginValue - 1);
+  }
 
 private:
-  static_assert(std::is_same<ValueT, std::remove_cv_t<ValueT>>::value,
-                "ValueT must not be const nor volatile");
+  static_assert(std::is_integral<T>::value || std::is_enum<T>::value,
+                "T must be an integral or enum type");
+  static_assert(std::is_same<T, std::remove_cv_t<T>>::value,
+                "T must not be const nor volatile");
+  static_assert(std::is_integral<raw_type>::value,
+                "raw_type must be an integral type");
 };
 
-template <typename ValueT> auto seq(ValueT Begin, ValueT End) {
-  return iota_range<ValueT>(Begin, End);
+/// Iterate over an integral/enum type from Begin up to - but not including -
+/// End.
+template <typename T> auto seq(T Begin, T End) {
+  return iota_range<T>(Begin, End, false);
+}
+
+/// Iterate over an integral/enum type from Begin to End inclusive.
+/// To prevent overflow, `End` must be different from the maximum value of the
+/// storage type for `T`. That is, in case of `T` being an enum, `End` must be
+/// different from the maximum value of `std::underlying_type<T>::type`.
+template <typename T> auto seq_inclusive(T Begin, T End) {
+  return iota_range<T>(Begin, End, true);
 }
 
 } // end namespace llvm
 
 #endif // LLVM_ADT_SEQUENCE_H

llvm/include/llvm/Support/MachineValueType.h

 //===- Support/MachineValueType.h - Machine-Level types ---------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file defines the set of machine-level target independent types which
 // legal values in the code generator use.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_SUPPORT_MACHINEVALUETYPE_H
 #define LLVM_SUPPORT_MACHINEVALUETYPE_H
 
+#include "llvm/ADT/Sequence.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/TypeSize.h"
 #include <cassert>
 
 namespace llvm {
 
static MVT getVectorVT(MVT VT, ElementCount EC) {
       return getVectorVT(VT, EC.getKnownMinValue());
     }
 
     /// Return the value type corresponding to the specified type.  This returns
     /// all pointers as iPTR.  If HandleUnknown is true, unknown types are
     /// returned as Other, otherwise they are invalid.
     static MVT getVT(Type *Ty, bool HandleUnknown = false);
 
-  private:
-    /// A simple iterator over the MVT::SimpleValueType enum.
-    struct mvt_iterator {
-      SimpleValueType VT;
-
-      mvt_iterator(SimpleValueType VT) : VT(VT) {}
-
-      MVT operator*() const { return VT; }
-      bool operator!=(const mvt_iterator &LHS) const { return VT != LHS.VT; }
-
-      mvt_iterator& operator++() {
-        VT = (MVT::SimpleValueType)((int)VT + 1);
-        assert((int)VT <= MVT::MAX_ALLOWED_VALUETYPE &&
-               "MVT iterator overflowed.");
-        return *this;
-      }
-    };
-
-    /// A range of the MVT::SimpleValueType enum.
-    using mvt_range = iterator_range<mvt_iterator>;
-
   public:
     /// SimpleValueType Iteration
     /// @{
-    static mvt_range all_valuetypes() {
-      return mvt_range(MVT::FIRST_VALUETYPE,
-                       (MVT::SimpleValueType)(MVT::LAST_VALUETYPE + 1));
+    static auto all_valuetypes() {
+      return seq_inclusive(MVT::FIRST_VALUETYPE, MVT::LAST_VALUETYPE);
     }
 
-    static mvt_range integer_valuetypes() {
-      return mvt_range(MVT::FIRST_INTEGER_VALUETYPE,
-                       (MVT::SimpleValueType)(MVT::LAST_INTEGER_VALUETYPE + 1));
+    static auto integer_valuetypes() {
+      return seq_inclusive(MVT::FIRST_INTEGER_VALUETYPE,
+                           MVT::LAST_INTEGER_VALUETYPE);
     }
 
-    static mvt_range fp_valuetypes() {
-      return mvt_range(MVT::FIRST_FP_VALUETYPE,
-                       (MVT::SimpleValueType)(MVT::LAST_FP_VALUETYPE + 1));
+    static auto fp_valuetypes() {
+      return seq_inclusive(MVT::FIRST_FP_VALUETYPE, MVT::LAST_FP_VALUETYPE);
     }
 
-    static mvt_range vector_valuetypes() {
-      return mvt_range(MVT::FIRST_VECTOR_VALUETYPE,
-                       (MVT::SimpleValueType)(MVT::LAST_VECTOR_VALUETYPE + 1));
+    static auto vector_valuetypes() {
+      return seq_inclusive(MVT::FIRST_VECTOR_VALUETYPE,
+                           MVT::LAST_VECTOR_VALUETYPE);
     }
 
-    static mvt_range fixedlen_vector_valuetypes() {
-      return mvt_range(
-               MVT::FIRST_FIXEDLEN_VECTOR_VALUETYPE,
-               (MVT::SimpleValueType)(MVT::LAST_FIXEDLEN_VECTOR_VALUETYPE + 1));
+    static auto fixedlen_vector_valuetypes() {
+      return seq_inclusive(MVT::FIRST_FIXEDLEN_VECTOR_VALUETYPE,
+                           MVT::LAST_FIXEDLEN_VECTOR_VALUETYPE);
     }
 
-    static mvt_range scalable_vector_valuetypes() {
-      return mvt_range(
-               MVT::FIRST_SCALABLE_VECTOR_VALUETYPE,
-               (MVT::SimpleValueType)(MVT::LAST_SCALABLE_VECTOR_VALUETYPE + 1));
+    static auto scalable_vector_valuetypes() {
+      return seq_inclusive(MVT::FIRST_SCALABLE_VECTOR_VALUETYPE,
+                           MVT::LAST_SCALABLE_VECTOR_VALUETYPE);
     }
 
-    static mvt_range integer_fixedlen_vector_valuetypes() {
-      return mvt_range(
-       MVT::FIRST_INTEGER_FIXEDLEN_VECTOR_VALUETYPE,
-       (MVT::SimpleValueType)(MVT::LAST_INTEGER_FIXEDLEN_VECTOR_VALUETYPE + 1));
+    static auto integer_fixedlen_vector_valuetypes() {
+      return seq_inclusive(MVT::FIRST_INTEGER_FIXEDLEN_VECTOR_VALUETYPE,
+                           MVT::LAST_INTEGER_FIXEDLEN_VECTOR_VALUETYPE);
     }
 
-    static mvt_range fp_fixedlen_vector_valuetypes() {
-      return mvt_range(
-          MVT::FIRST_FP_FIXEDLEN_VECTOR_VALUETYPE,
-          (MVT::SimpleValueType)(MVT::LAST_FP_FIXEDLEN_VECTOR_VALUETYPE + 1));
+    static auto fp_fixedlen_vector_valuetypes() {
+      return seq_inclusive(MVT::FIRST_FP_FIXEDLEN_VECTOR_VALUETYPE,
+                           MVT::LAST_FP_FIXEDLEN_VECTOR_VALUETYPE);
     }
 
-    static mvt_range integer_scalable_vector_valuetypes() {
-      return mvt_range(
-       MVT::FIRST_INTEGER_SCALABLE_VECTOR_VALUETYPE,
-       (MVT::SimpleValueType)(MVT::LAST_INTEGER_SCALABLE_VECTOR_VALUETYPE + 1));
+    static auto integer_scalable_vector_valuetypes() {
+      return seq_inclusive(MVT::FIRST_INTEGER_SCALABLE_VECTOR_VALUETYPE,
+                           MVT::LAST_INTEGER_SCALABLE_VECTOR_VALUETYPE);
     }
 
-    static mvt_range fp_scalable_vector_valuetypes() {
-      return mvt_range(
-            MVT::FIRST_FP_SCALABLE_VECTOR_VALUETYPE,
-            (MVT::SimpleValueType)(MVT::LAST_FP_SCALABLE_VECTOR_VALUETYPE + 1));
+    static auto fp_scalable_vector_valuetypes() {
+      return seq_inclusive(MVT::FIRST_FP_SCALABLE_VECTOR_VALUETYPE,
+                           MVT::LAST_FP_SCALABLE_VECTOR_VALUETYPE);
     }
     /// @}
   };
 
 } // end namespace llvm
 
 #endif // LLVM_SUPPORT_MACHINEVALUETYPE_H

llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp

assert(VT.getVectorNumElements() <
              InOp.getValueType().getVectorNumElements() &&
          "Input wasn't widened!");
 
   // We may need to further widen the operand until it has the same total
   // vector size as the result.
   EVT InVT = InOp.getValueType();
   if (InVT.getSizeInBits() != VT.getSizeInBits()) {
     EVT InEltVT = InVT.getVectorElementType();
-    for (int i = MVT::FIRST_VECTOR_VALUETYPE, e = MVT::LAST_VECTOR_VALUETYPE; i < e; ++i) {
-      EVT FixedVT = (MVT::SimpleValueType)i;
+    for (EVT FixedVT : MVT::vector_valuetypes()) {
       EVT FixedEltVT = FixedVT.getVectorElementType();
       if (TLI.isTypeLegal(FixedVT) &&
           FixedVT.getSizeInBits() == VT.getSizeInBits() &&
           FixedEltVT == InEltVT) {
         assert(FixedVT.getVectorNumElements() >= VT.getVectorNumElements() &&
                "Not enough elements in the fixed type for the operand!");
         assert(FixedVT.getVectorNumElements() != InVT.getVectorNumElements() &&
                "We can't have the same type as we started with!");
static EVT FindMemType(SelectionDAG& DAG, const TargetLowering &TLI,
   unsigned WidenEltWidth = WidenEltVT.getSizeInBits();
   unsigned AlignInBits = Align*8;
 
   // If we have one element to load/store, return it.
   EVT RetVT = WidenEltVT;
   if (!Scalable && Width == WidenEltWidth)
     return RetVT;
 
-  // See if there is larger legal integer than the element type to load/store.
-  unsigned VT;
   // Don't bother looking for an integer type if the vector is scalable, skip
   // to vector types.
   if (!Scalable) {
-    for (VT = (unsigned)MVT::LAST_INTEGER_VALUETYPE;
-         VT >= (unsigned)MVT::FIRST_INTEGER_VALUETYPE; --VT) {
-      EVT MemVT((MVT::SimpleValueType) VT);
+    // See if there is larger legal integer than the element type to load/store.
+    for (EVT MemVT : reverse(MVT::integer_valuetypes())) {
       unsigned MemVTWidth = MemVT.getSizeInBits();
       if (MemVT.getSizeInBits() <= WidenEltWidth)
         break;
       auto Action = TLI.getTypeAction(*DAG.getContext(), MemVT);
       if ((Action == TargetLowering::TypeLegal ||
            Action == TargetLowering::TypePromoteInteger) &&
           (WidenWidth % MemVTWidth) == 0 &&
           isPowerOf2_32(WidenWidth / MemVTWidth) &&
           (MemVTWidth <= Width ||
            (Align!=0 && MemVTWidth<=AlignInBits && MemVTWidth<=Width+WidenEx))) {
         if (MemVTWidth == WidenWidth)
           return MemVT;
         RetVT = MemVT;
         break;
       }
     }
   }
 
   // See if there is a larger vector type to load/store that has the same vector
   // element type and is evenly divisible with the WidenVT.
-  for (VT = (unsigned)MVT::LAST_VECTOR_VALUETYPE;
-       VT >= (unsigned)MVT::FIRST_VECTOR_VALUETYPE; --VT) {
-    EVT MemVT = (MVT::SimpleValueType) VT;
+  for (EVT MemVT : reverse(MVT::vector_valuetypes())) {
     // Skip vector MVTs which don't match the scalable property of WidenVT.
     if (Scalable != MemVT.isScalableVector())
       continue;
     unsigned MemVTWidth = MemVT.getSizeInBits().getKnownMinSize();
     auto Action = TLI.getTypeAction(*DAG.getContext(), MemVT);
     if ((Action == TargetLowering::TypeLegal ||
          Action == TargetLowering::TypePromoteInteger) &&
         WidenEltVT == MemVT.getVectorElementType() &&

llvm/tools/llvm-exegesis/lib/X86/Target.cpp

for (auto I : llvm::zip(Instr.Variables, VariableChoices)) {
 
     switch (Instr.getPrimaryOperand(Var).getExplicitOperandInfo().OperandType) {
     default:
       // We don't wish to explicitly explore this variable.
       Choices.emplace_back(); // But add invalid MCOperand to simplify logic.
       continue;
     case X86::OperandType::OPERAND_COND_CODE: {
       Exploration = true;
-      auto CondCodes = seq((int)X86::CondCode::COND_O,
-                           1 + (int)X86::CondCode::LAST_VALID_COND);
-      Choices.reserve(std::distance(CondCodes.begin(), CondCodes.end()));
+      auto CondCodes =
+          seq_inclusive(X86::CondCode::COND_O, X86::CondCode::LAST_VALID_COND);
+      Choices.reserve(CondCodes.size());
       for (int CondCode : CondCodes)
         Choices.emplace_back(MCOperand::createImm(CondCode));
       break;
     }
     }
   }
 
   // If we don't wish to explore any variables, defer to the baseline method.

llvm/unittests/ADT/SequenceTest.cpp

 //===- SequenceTest.cpp - Unit tests for a sequence abstraciton -----------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/ADT/Sequence.h"
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 #include <list>
 
 using namespace llvm;
 
+using testing::ElementsAre;
+
 namespace {
 
 TEST(SequenceTest, Forward) {
   int X = 0;
   for (int I : seq(0, 10)) {
     EXPECT_EQ(X, I);
     ++X;
   }
TEST(SequenceTest, Dereference) {
   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);
 }
 
+enum class CharEnum : char { A = 1, B, C, D, E };
+
+TEST(SequenceTest, ForwardIteration) {
+  EXPECT_THAT(seq_inclusive(CharEnum::C, CharEnum::E),
+              ElementsAre(CharEnum::C, CharEnum::D, CharEnum::E));
+}
+
+TEST(SequenceTest, BackwardIteration) {
+  EXPECT_THAT(reverse(seq_inclusive(CharEnum::B, CharEnum::D)),
+              ElementsAre(CharEnum::D, CharEnum::C, CharEnum::B));
+}
+
+using IntegralTypes = testing::Types<uint8_t, uint16_t, uint32_t, uint64_t,
+                                     int8_t, int16_t, int32_t, int64_t>;
+
+template <class T> class SequenceTest : public testing::Test {
+public:
+  const T min = std::numeric_limits<T>::min();
+  const T minp1 = min + 1;
+  const T max = std::numeric_limits<T>::max();
+  const T maxm1 = max - 1;
+
+  void checkIteration() const {
+    // Forward
+    EXPECT_THAT(seq(min, min), ElementsAre());
+    EXPECT_THAT(seq(min, minp1), ElementsAre(min));
+    EXPECT_THAT(seq(maxm1, max), ElementsAre(maxm1));
+    EXPECT_THAT(seq(max, max), ElementsAre());
+    // Reverse
+    if (std::is_unsigned<T>::value) {
+      EXPECT_THAT(reverse(seq(min, min)), ElementsAre());
+      EXPECT_THAT(reverse(seq(min, minp1)), ElementsAre(min));
+    }
+    EXPECT_THAT(reverse(seq(maxm1, max)), ElementsAre(maxm1));
+    EXPECT_THAT(reverse(seq(max, max)), ElementsAre());
+    // Inclusive
+    EXPECT_THAT(seq_inclusive(min, min), ElementsAre(min));
+    EXPECT_THAT(seq_inclusive(min, minp1), ElementsAre(min, minp1));
+    EXPECT_THAT(seq_inclusive(maxm1, maxm1), ElementsAre(maxm1));
+    // Inclusive Reverse
+    if (std::is_unsigned<T>::value) {
+      EXPECT_THAT(reverse(seq_inclusive(min, min)), ElementsAre(min));
+      EXPECT_THAT(reverse(seq_inclusive(min, minp1)), ElementsAre(minp1, min));
+    }
+    EXPECT_THAT(reverse(seq_inclusive(maxm1, maxm1)), ElementsAre(maxm1));
+  }
+
+  void checkIterators() const {
+    auto checkValidIterators = [](auto sequence) {
+      EXPECT_LE(sequence.begin(), sequence.end());
+    };
+    checkValidIterators(seq(min, min));
+    checkValidIterators(seq(max, max));
+    checkValidIterators(seq_inclusive(min, min));
+    checkValidIterators(seq_inclusive(maxm1, maxm1));
+  }
+};
+TYPED_TEST_SUITE(SequenceTest, IntegralTypes);
+TYPED_TEST(SequenceTest, Boundaries) {
+  this->checkIteration();
+  this->checkIterators();
+}
+
+// DEATH tests
+#if !defined(NDEBUG)
+template <class T> class SequenceDeathTest : public SequenceTest<T> {
+public:
+  using SequenceTest<T>::min;
+  using SequenceTest<T>::minp1;
+  using SequenceTest<T>::max;
+  using SequenceTest<T>::maxm1;
+
+  void checkInvalidOrder() const {
+    EXPECT_DEATH(seq(max, min), "Begin must be less or equal to End.");
+    EXPECT_DEATH(seq(minp1, min), "Begin must be less or equal to End.");
+    EXPECT_DEATH(seq_inclusive(maxm1, min),
+                 "Begin must be less or equal to End.");
+    EXPECT_DEATH(seq_inclusive(minp1, min),
+                 "Begin must be less or equal to End.");
+  }
+  void checkInvalidValues() const {
+    EXPECT_DEATH(seq_inclusive(min, max),
+                 "Forbidden End value for seq_inclusive.");
+    EXPECT_DEATH(seq_inclusive(minp1, max),
+                 "Forbidden End value for seq_inclusive.");
+    if (std::is_signed<T>::value) {
+      EXPECT_DEATH(reverse(seq(min, min)),
+                   "Forbidden Begin value for reverse iteration");
+      EXPECT_DEATH(reverse(seq_inclusive(min, min)),
+                   "Forbidden Begin value for reverse iteration");
+    }
+  }
+};
+TYPED_TEST_SUITE(SequenceDeathTest, IntegralTypes);
+TYPED_TEST(SequenceDeathTest, DeathTests) {
+  this->checkInvalidOrder();
+  this->checkInvalidValues();
+}
+#endif // !defined(NDEBUG)
+
 } // anonymous namespace

llvm/unittests/CodeGen/ScalableVectorMVTsTest.cpp

 #include "llvm/Support/TypeSize.h"
 #include "gtest/gtest.h"
 
 using namespace llvm;
 
 namespace {
 
 TEST(ScalableVectorMVTsTest, IntegerMVTs) {
-  for (auto VecTy : MVT::integer_scalable_vector_valuetypes()) {
+  for (MVT VecTy : MVT::integer_scalable_vector_valuetypes()) {
     ASSERT_TRUE(VecTy.isValid());
     ASSERT_TRUE(VecTy.isInteger());
     ASSERT_TRUE(VecTy.isVector());
     ASSERT_TRUE(VecTy.isScalableVector());
     ASSERT_TRUE(VecTy.getScalarType().isValid());
 
     ASSERT_FALSE(VecTy.isFloatingPoint());
   }
 }
 
 TEST(ScalableVectorMVTsTest, FloatMVTs) {
-  for (auto VecTy : MVT::fp_scalable_vector_valuetypes()) {
+  for (MVT VecTy : MVT::fp_scalable_vector_valuetypes()) {
     ASSERT_TRUE(VecTy.isValid());
     ASSERT_TRUE(VecTy.isFloatingPoint());
     ASSERT_TRUE(VecTy.isVector());
     ASSERT_TRUE(VecTy.isScalableVector());
     ASSERT_TRUE(VecTy.getScalarType().isValid());
 
     ASSERT_FALSE(VecTy.isInteger());
   }

llvm/unittests/IR/ConstantRangeTest.cpp

EnumerateTwoConstantRanges(
           ForeachNumInConstantRange(
               CR2, [&](const APInt &N2) { Exhaustive &= icmp(Pred, N1, N2); });
         });
         EXPECT_EQ(CR1.icmp(Pred, CR2), Exhaustive);
       });
 }
 
 TEST(ConstantRange, ICmp) {
-  for (auto Pred : seq<unsigned>(CmpInst::Predicate::FIRST_ICMP_PREDICATE,
-                                 1 + CmpInst::Predicate::LAST_ICMP_PREDICATE))
-    ICmpTestImpl((CmpInst::Predicate)Pred);
+  for (auto Pred : seq_inclusive(CmpInst::Predicate::FIRST_ICMP_PREDICATE,
+                                 CmpInst::Predicate::LAST_ICMP_PREDICATE))
+    ICmpTestImpl(Pred);
 }
 
 TEST(ConstantRange, MakeGuaranteedNoWrapRegion) {
   const int IntMin4Bits = 8;
   const int IntMax4Bits = 7;
   typedef OverflowingBinaryOperator OBO;
 
   for (int Const : {0, -1, -2, 1, 2, IntMin4Bits, IntMax4Bits}) {

mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp

getCollapsedOutputDimFromInputShape(OpBuilder &builder, Location loc,
                                     int64_t dimIndex, Value src,
                                     ArrayRef<AffineMap> reassociationMap) {
   AffineMap map = reassociationMap[dimIndex];
   unsigned startPos =
       map.getResults().front().cast<AffineDimExpr>().getPosition();
   unsigned endPos = map.getResults().back().cast<AffineDimExpr>().getPosition();
   AffineExpr expr;
   SmallVector<Value, 2> dynamicDims;
-  for (auto dim : llvm::seq(startPos, endPos + 1)) {
+  for (auto dim : llvm::seq_inclusive(startPos, endPos)) {
     dynamicDims.push_back(builder.createOrFold<memref::DimOp>(loc, src, dim));
     AffineExpr currExpr = builder.getAffineSymbolExpr(dim - startPos);
     expr = (expr ? expr * currExpr : currExpr);
   }
   return applyMapToValues(builder, loc,
                           AffineMap::get(0, endPos - startPos + 1, expr),
                           dynamicDims)[0];
 }

 static llvm::DenseMap<int64_t, int64_t>
 getExpandedDimToCollapsedDimMap(ArrayRef<AffineMap> reassociation) {
   llvm::DenseMap<int64_t, int64_t> expandedDimToCollapsedDim;
   for (auto map : enumerate(reassociation)) {
     unsigned startPos =
         map.value().getResults().front().cast<AffineDimExpr>().getPosition();
     unsigned endPos =
         map.value().getResults().back().cast<AffineDimExpr>().getPosition();
-    for (auto dim : llvm::seq(startPos, endPos + 1)) {
+    for (auto dim : llvm::seq_inclusive(startPos, endPos)) {
       expandedDimToCollapsedDim[dim] = map.index();
     }
   }
   return expandedDimToCollapsedDim;
 }
 
 /// For an expanding reshape op, compute the value for a dimension of the output
 /// from the shape of the input.