[PATCH] InstCombine: constant comparison involving ashr is wrongly simplified (PR20945).

[Duncan P. N. Exon Smith]

> On 2014-Sep-16, at 11:07, Andrea Di Biagio <andrea.dibiagio at gmail.com> wrote:
> 
>>    if (IsAShr ? AP1.sgt(AP2) : AP1.ugt(AP2)) {
>>      // Right-shifting will not increase the value.
>>      return getConstant(false);
>>    }
> 
> Unfortunately this would not work for the case where 'IsNegative &&
> IsAShr' is true.
> If 'IsNegative' is true, then we have to invert the role of AP1 and
> AP2 in the equation.
> 
> Something like...
> if (IsAShr) {
>   if (IsNegative ? AP2.sgt(AP1) : AP1.sgt(AP2))
>     return getConstant(false);
> }
> 
> For logical shifts, if both AP1 and AP2 are negative numbers then
> (under the assumption that AP1 is different than AP2), we can never
> find a 'Shift' value that solves the equation. That is because a
> logical shift by a non-zero quantity would affect the sign.

I'd just move this case up into the logic above then.

    if (IsAShr) {
      if (AP1.isNegative() != AP2.isNegative()) {
        // Arithmetic shift will never change the sign.
        return getConstant(false);
      }
      // Both the constants are negative, take their positive to calculate
      // log.
      if (AP1.isNegative()) {
        if (AP1.slt(AP2))
          // Right-shifting won't increase the magnitude.
          return getConstant(false);
        IsNegative = true;
      }
    }

Then change this to:

    if (!IsNegative && AP1.ugt(AP2))
      // Right-shifting won't increase the magnitude.
      return getConstant(false);