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

[Reid Spencer]

On Wed, 2006-12-13 at 09:48 -0800, Chris Lattner wrote:
> >>> @@ -5681,7 +5677,7 @@
> >>>  /// evaluate the expression.
> >>>  Value *InstCombiner::EvaluateInDifferentType(Value *V, const Type
> >>> *Ty) {
> >>>    if (Constant *C = dyn_cast<Constant>(V))
> >>> -    return ConstantExpr::getCast(C, Ty);
> >>> +    return ConstantExpr::getIntegerCast(C, Ty, C->getType()-
> >>>> isSigned());
> >>
> >> This looks extremely unsafe.  Why is it ok?
> >
> > The only place EvaluateInDifferentType is called is from
> > commonIntCastTransforms. Those casts are guaranteed to only involve
> > integer operands. Therefore, the only kind of cast that could be
> > involved is an integer cast.
> >
> > What looks "extremely" unsafe about it?
> 
> I'm specifically concerned that there is nothing controlling whether  
> a zext or sext gets generated other than C->getType()->isSigned(),  
> which I'm not convinced is the right thing.  Getting the wrong one is  
> bad :)

Ah, okay, that makes sense. I'll look at getting rid of the isSigned()
call.

> 
> -Chris