[PATCH] InstCombineCompare with constant return false if we know it is never going to be equal

[Duncan Sands]

Hi Suyog,
> +  // If the KnownBits of LHS and RHS at same bit position differ,
> +  // LHS and RHS are not equal.

I think this comment is misleading.  If KnownBitsLHS != KnownBitsRHS it doesn't 
mean that LHS != RHS, but the comment seems to be saying that.

> +  if (ConstantInt *CI = dyn_cast<ConstantInt>(RHS)) {
> +      uint32_t BitWidth = CI->getBitWidth();
> +      APInt LHSKnownZero(BitWidth, 0, 1);

Why not just: APInt LHSKnownZero(BitWidth);

> +      APInt LHSKnownOne(BitWidth, 0, 1);

Likewise here and below.

> +      computeKnownBits(LHS, LHSKnownZero, LHSKnownOne);
> +      APInt RHSKnownZero(BitWidth, 0, 1);
> +      APInt RHSKnownOne(BitWidth, 0, 1);
> +      computeKnownBits(RHS, RHSKnownZero, RHSKnownOne);
> +      switch (Pred) {
> +      // TODO: Handle for other icmp instructions.
> +      default:
> +        break;
> +      case ICmpInst::ICMP_EQ: {
> +        if (((LHSKnownOne & RHSKnownZero) != 0) ||
> +            ((LHSKnownZero & RHSKnownOne) != 0))
> +          return ConstantInt::getFalse(CI->getContext());
> +        break;
> +      }
> +      case ICmpInst::ICMP_NE:
> +        if (((LHSKnownOne & RHSKnownZero) != 0) ||
> +            ((LHSKnownZero & RHSKnownOne) != 0))
> +          return ConstantInt::getTrue(CI->getContext());
> +        break;

These two cases could be unified:
   case ICmpInst::ICMP_EQ:
   case ICmpInst::ICMP_NE:
     if (((LHSKnownOne & RHSKnownZero) != 0) ||
         ((LHSKnownZero & RHSKnownOne) != 0))
       return Pred == ICmpInst::ICMP_EQ ?
         ConstantInt::getFalse(CI->getContext()) :
         ConstantInt::getTrue(CI->getContext());

Best wishes, Duncan.

http://reviews.llvm.org/D3868