[llvm-commits] SETCC InstructionCombining.cpp [3/3]

[Reid Spencer]

On Fri, 2006-12-22 at 13:31 -0800, Chris Lattner wrote:

> 
> +    // We're performing a signed comparison.
> +    if (isSignedExt) {
> +      // Signed extend and signed comparison.
> +      if (cast<ConstantInt>(CI)->getSExtValue() < 0)// X < (small)
> --> false
> +        Result = ConstantBool::getFalse();
> +      else
> +        Result = ConstantBool::getTrue();           // X < (large)
> --> true
> +    } else {
> +      // Zero extend and signed comparison.
> +      if (cast<ConstantInt>(CI)->getSExtValue() < 0)
> +        Result = ConstantBool::getFalse();
> +      else
> +        Result = ConstantBool::getTrue();
>      }
> 
> 
> My reading of this code says that you can eliminate the 'if
> issignedext' test, as the if/then cases are the same.  Is this a bug
> or just code duplication?

I'm not Sure. The original (before any signless types changes, rev
1.525) looks like this:

      if (DestTy->isSigned()) {
        // We're performing a signed comparison.
        if (isSignSrc) {
          // Signed extend and signed comparison.
          if (cast<ConstantSInt>(CI)->getValue() < 0) // X < (small) -->
false
            Result = ConstantBool::getFalse();
          else
            Result = ConstantBool::getTrue();         // X < (large) -->
true
        } else {
          // Unsigned extend and signed comparison.
          if (cast<ConstantSInt>(CI)->getValue() < 0)
            Result = ConstantBool::getFalse();
          else
            Result = ConstantBool::getTrue();
        }

Clearly getting the ZExt value and comparing with < 0 is just silly
(unsigned is never less than zero).  I'm inclined to think this is
either a long-standing bug or just some code duplication.  I'll
eliminate the redundancy because it is clearly generating the same
results in either case. However, I'm not convinced this isn't a bug.

Reid.

>