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

[Li Jia He via Phabricator via llvm-commits]

HLJ2009 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))
----------------
lebedev.ri wrote:
> 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.
> 
The "if" in the code should match the below ir 
define i32 @test1(i32 %x, i32 %y) {
  %1 = icmp ult i32 %x, %y
  %2 = icmp ne i32 %y, 0
  %3 = and i1 %2, %1
  %4 = zext i1 %3 to i32
  ret i32 %4
}
The "else if" in the code should match the below ir
define i32 @test1(i32 %x, i32 %y) {
  %1 = icmp ugt i32 %x, %y
  %2 = icmp ne i32 %x, 0
  %3 = and i1 %2, %1
  %4 = zext i1 %3 to i32
  ret i32 %4
}
but "else if" branche always match less than the corresponding ir


Repository:
  rL LLVM

https://reviews.llvm.org/D47922