[PATCH] D108151: [NFC][clang] Use X86 Features declaration from X86TargetParser

[Andrei Elovikov via Phabricator via cfe-commits]

a.elovikov marked 3 inline comments as done.
a.elovikov added inline comments.


================
Comment at: clang/lib/Basic/Targets/X86.cpp:1071
+    assert(llvm::is_contained(Priorities, Priority) &&
+           "Priorites don't form consequtive range!");
+  }
----------------
erichkeane wrote:
> erichkeane wrote:
> > craig.topper wrote:
> > > erichkeane wrote:
> > > > If all you care about is whether they are a consecutive range, why not just use `std::is_sorted`?
> > > The Priorities array isn't sorted. It's just whatever order the X86_FEATURE_COMPAT lists them.
> > > 
> > > The values need to be unique and in a contiguous range.
> > Then I'd suggest something like: `llvm::sort`, then `assert *(end - 1) - *begin == std::distance(begin, end) && llvm::adjacent_find` or something.
> > 
> > I definitely didn't get that point out of this odd for-loop and is_contained.  There is perhaps at trick with std::min and std::max too.  Though, it looks like this is perhaps trying to prove that the range is 0 to the the array size, right?  In that case, perhaps there is something easier.
> > 
> > Also a nit, it is `consecutive` in that case.
> Actually...
> 
>     std::array<unsigned, array_lengthof(Priorities) -1> HelperList;
>     std::iota(HelperList.begin(), HelperList.end());
>     std::is_permutation(HelperList.begin(), HelperList.end(),  
>             std::begin(Priorities), std::end(Priorities));
I thought about std::sort + std::iota + std::equal but wasn't sure how readable that would be with the extra helper object (range version isn't available in C++14) and hoped my version would be more compact.

Since it wasn't obvious what it does I shamelessly copied your suggestion (and learnt about std::is_permutation and array_lengthof when doing it).

Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108151/new/

https://reviews.llvm.org/D108151