[PATCH] D49958: [MISC]Fix wrong usage of std::equal()

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:2918-2920
   // Min/max matching is only viable if all output VTs are the same.
-  if (std::equal(ValueVTs.begin(), ValueVTs.end(), ValueVTs.begin())) {
+  if (ValueVTs.size() == 1 ||
+      std::equal(ValueVTs.begin() + 1, ValueVTs.end(), ValueVTs.begin())) {
----------------
lebedev.ri wrote:
> I don't understand what this code is doing at all.
> All three arguments point to the same array.
> This is `(1)` in https://en.cppreference.com/w/cpp/algorithm/equal:
> > 1,3) Returns true if the range [first1, last1) is equal to the range [first2, first2 + (last1 - first1)), and false otherwise
> 
> So it will equality-compare the `ValueVTs[i]` with `ValueVTs[i]` for all `i`?
> But that is always `true` (well, except weird floats, etc)
> Or am i completely misreading this?
> 
> Assuming the comment is the correct one, shouldn't this be
> ```
>   if (llvm::all_of(ValueVTs, [FirstVT = ValueVTs.front()](EVT VT) -> bool {
>         return VT == FirstVT;
>       })) {
> ```
> ?
The original code does just compare each element to itself and is always true.

The code in the patch compares each element to the one before it. This will only be true if they are all the same. And is an valid idiom for this. It’s one of the comments here https://stackoverflow.com/questions/20287095/checking-if-all-elements-of-a-vector-are-equal-in-c

The lambda you wrote would also work.


https://reviews.llvm.org/D49958