[llvm] r248073 - [InstCombine] FoldICmpCstShrCst didn't handle icmps of -1 in the ashr case correctly

[Sanjoy Das via llvm-commits]

Not objecting to this fix, I don't quite understand the
AP2.isPowerOf2() caveat.

David Majnemer via llvm-commits wrote:
 >         if (Shift > 0) {
 >     -    if (IsAShr ? AP1 == AP2.ashr(Shift) : AP1 == AP2.lshr(Shift))
 >     +    if (IsAShr && AP1 == AP2.ashr(Shift)) {
 >     +      // There are multiple solutions if we are comparing against
 >     -1 and the LHS
 >     +      // of the ashr is not a power of two..
 >     +      if (AP1.isAllOnesValue() && !AP2.isPowerOf2())

Firstly, for AP1 == -1 && IsAShr, AP2.isPowerOf2() will be true iff
AP2 is INT_MIN, right (because of the (AP2.isNegative() !=
AP1.isNegative())) above?

And if I have (in i8): "(128 >> x) == -1" then all x >= 7 are
solutions: x == 7 is clearly a solution and for everything greater
than 7, the comparison is "undef == -1" == "true".

Of course, you could also evaluate "undef == -1" to "false" (and so
emitting an EQ is correct), but we don't really need the "&&
!AP2.isPowerOf2()" for correctness, IIUC.

-- Sanjoy