[PATCH] D47922: unsigned foo(unsigned x, unsigned y) { return x > y && x != 0; } should fold to x > y

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

There was no need to close https://reviews.llvm.org/D47972 and open a new https://reviews.llvm.org/D48000.
But regardless, please do update this differential,
rebase it ontop of the updated tests, to show how it affects them.



================
Comment at: lib/Analysis/InstructionSimplify.cpp:1312-1316
   if (match(UnsignedICmp, m_ICmp(UnsignedPred, m_Value(X), m_Specific(Y))) &&
       ICmpInst::isUnsigned(UnsignedPred))
     ;
   else if (match(UnsignedICmp,
-                 m_ICmp(UnsignedPred, m_Value(Y), m_Specific(X))) &&
----------------
HLJ2009 wrote:
> lebedev.ri wrote:
> > After staring at this a bit more, the original code looks like just a dead code.
> > We are first trying to match `match(UnsignedICmp, m_ICmp(UnsignedPred, m_Value(X), m_Specific(Y))`.
> > I.e. we take whatever `X`, and the specific value of `Y`. This could match.
> > And then we check that `ICmpInst::isUnsigned(UnsignedPred)` is true. This could fail.
> > Then we fall back to `else if`.
> > And use `match(UnsignedICmp, m_ICmp(UnsignedPred, m_Value(Y), m_Specific(X))`.
> > But this could not possibly match, because the first time we took `X` from the first operand,
> > and now we are looking for this very `X` in the second operand.
> > And the `m_ICmp()` is not commutable.
> > So i think this is simply a typo.
> Yes, your analysis is the same as mine.
OK


Repository:
  rL LLVM

https://reviews.llvm.org/D47922