[llvm-commits] CVS: llvm/lib/Transforms/Scalar/InstructionCombining.cpp

[Chris Lattner]

On Dec 4, 2006, at 10:33 AM, Reid Spencer wrote:

> On Mon, 2006-12-04 at 10:22 -0800, Chris Lattner wrote:
>>>    // If this is another cast that can be eliminated, it isn't
>>> codegen either.
>>>    if (const CastInst *CI = dyn_cast<CastInst>(V))
>>> -    if (isEliminableCastPair(CI, CastInst::getCastOpcode(V, Ty),
>>> Ty, TD))
>>> +    if (isEliminableCastPair(CI, CastInst::getCastOpcode(
>>> +            V, V->getType()->isSigned(), Ty, Ty->isSigned()), Ty,
>>> TD))
>>>        return false;
>>>    return true;
>>>  }
>>
>> Won't this miscompile stuff like:
>>
>> %B = zext sbyte %A to ushort
>> %C = zext ushort %B to uint
>>
>> into a sext?
>
> No. This patch doesn't change anything.

I never claimed it wasn't broken before.  We have *known* breakage in  
the tree still from your cast patch that we're still trying to track  
down.

> The only reason for this change
> is the change in signature for getCastOpcode. See the corresponding
> change in getCastOpcode.  This passed everything on Linux last night
> before I committed it.

Okay, this code isn't miscompiled because it isn't called in this  
case.  After looking at it closer, I still don't see why it's safe.   
Specifically, why do you think it is safe for the sign of Ty or V to  
determine whether the cast is a sext or zext?

The only way to fix this is for clients to pass in the kind of cast  
it is interested in.  Looking at the callers, the only possibilities  
are sext/zext/trunc/bitcast.  Treating zext and sext incorrectly  
*can* miscompile code.

-Chris