[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 inline comments.


================
Comment at: lib/Analysis/InstructionSimplify.cpp:1312
   else if (match(UnsignedICmp,
-                 m_ICmp(UnsignedPred, m_Value(Y), m_Specific(X))) &&
            ICmpInst::isUnsigned(UnsignedPred))
----------------
HLJ2009 wrote:
> lebedev.ri wrote:
> > HLJ2009 wrote:
> > > HLJ2009 wrote:
> > > > lebedev.ri wrote:
> > > > > So was the old one a harmless typo that rendered that `if` useless, was that a miscompile (needs test then),
> > > > > or is the RHS of the diff is a new case (so the old one is a standalone case, too, and needs a test)?
> > > > Yes, you are right. For the optimization of this file, I did not find the corresponding test case file.Do I need to add all the remaining test cases now? Or submit a change individually.
> > > If we add a test file named simplifyUnsignedRangeCheck.ll and write the optimizations in this function as test cases, would it be better ?
> > Hm?
> > It looks like `test/Transforms/InstCombine/range-check.ll` is the testfile with tests for this function, no?
> > But that wasn't the question.
> > Is this change fixing a code that was *supposed* to handle your case as-is [but did nothing, or miscompiled something],
> > or does it change the code that was properly handling some other case?
> I looked at the range-check.ll file, and its test point does not seem to be in the function that I modified.My changes will not affect other cases and will not cause compilation errors.
We are talking past each other.



Repository:
  rL LLVM

https://reviews.llvm.org/D47922