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

[Kewen Lin via Phabricator via llvm-commits]

jedilyn added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:2919
   // 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())) {
----------------
jedilyn wrote:
> shchenz wrote:
> > inouehrs wrote:
> > > ikulagin wrote:
> > > > lebedev.ri wrote:
> > > > > This seems to originate from @jmolloy from rL237423.
> > > > maybe it was supposed like this:
> > > > 
> > > > ```
> > > > if (std::equal(ValueVTs.begin(), ValueVTs.end(), ValueVTs.rbegin())) { 
> > > > ```
> > > > iterate from both ends
> > > Comparing from both side may return true for sequences like 1, 2, 1 or 1, 2, 2, 1, although they are not all the same. 
> > Yes, iterating from both ends can only get a palindrome pattern like {1,2,3,4,5,4,3,2,1}. Here we need to check all values in container are the same I think.
> It looks "ValueVTs.size() == 1" is redundant, even if the input vector only holds 1-element, the std::equal can still work and return true, what is this special size checking expects. Please feel free to correct me if I missed something we have to specify that here.
OK, it looks due to empty range related operation is an undefined behavior https://stackoverflow.com/questions/2270138/does-passing-an-empty-range-identical-iterators-to-an-stl-algorithm-result-in. Please ignore my above comment.


https://reviews.llvm.org/D49958