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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
