[llvm-commits] CVS: llvm/lib/Analysis/ScalarEvolution.cpp

[Reid Spencer]

On Tue, 2006-12-05 at 12:07 -0800, Chris Lattner wrote:
> On Dec 5, 2006, at 11:58 AM, Reid Spencer wrote:
> > On Tue, 2006-12-05 at 11:52 -0800, Chris Lattner wrote:
> >>>  SCEVHandle SCEVTruncateExpr::get(const SCEVHandle &Op, const Type
> >>> *Ty) {
> >>>    if (SCEVConstant *SC = dyn_cast<SCEVConstant>(Op))
> >>> -    return SCEVUnknown::get(ConstantExpr::getCast(SC->getValue(),
> >>> Ty));
> >>> +    return SCEVUnknown::get(
> >>> +        ConstantExpr::getTruncOrBitCast(SC->getValue(), Ty));
> >>
> >> Shouldn't this one always bit a truncate?
> >
> > No, they could be the same size.
> 
> How?  The two call sites are:
> 
>    if (SrcTy->getPrimitiveSize() > Ty->getPrimitiveSize())
>      return SCEVTruncateExpr::get(V, Ty);
> 
> and:
> 
>      case Instruction::Trunc:
>        if (I->getType()->isInteger() && I->getOperand(0)->getType()- 
>  >isInteger())
>          return SCEVTruncateExpr::get(getSCEV(I->getOperand(0)),
>                                       I->getType()->getUnsignedVersion 
> ());
> 
> The first can't be the same size, obviously.  The second can't be the  
> same size because it's coming from a trunc instruction.  Further, the  
> 'I->getOperand(0)->getType()->isInteger()' in the second code is also  
> redundant with 'truncness'.

Okay, this one appears to be correct as just Trunc. The issue I had was
not related to these but to another recommendation you made. See below.

> 
> >>
> >>>  SCEVHandle SCEVZeroExtendExpr::get(const SCEVHandle &Op, const
> >>> Type *Ty) {
> >>>    if (SCEVConstant *SC = dyn_cast<SCEVConstant>(Op))
> >>> -    return SCEVUnknown::get(ConstantExpr::getCast(SC->getValue(),
> >>> Ty));
> >>> +    return SCEVUnknown::get(
> >>> +        ConstantExpr::getZExtOrBitCast(SC->getValue(), Ty));
> >>
> >> Likewise, always a zext?
> >
> > No, they could be the same size.
> 
> This is called in two places very similar to trunc.

Same deal.

> 
> If you believe that these can be called with srcsize=destsize, please  
> explain how.

After debugging it, the problem that arose is that the constructor for
SCEVTruncExpr or SCEVZeroExtendExpr would fail because the operand to be
casted was SCEVUnknown. This situation resulted from removing the
integer checks in createSCEV for Trunc and ZExt which can validly cast
BoolTy (e.g. setne). However, for setne and friends, createSCEV creates
an SCEVUnknown which would then be passed down to the SCEV*Expr
constructor and assert out. 
Long and short of it, I put the checks for integer back in and changed
the casts to use getTrunc or getZExt.  Seems to work.

> 
> >>
> >>> @@ -998,11 +1000,10 @@
> >>>        Constant *LHSCV = LHSC->getValue();
> >>>        Constant *RHSCV = RHSC->getValue();
> >>>        if (LHSCV->getType()->isUnsigned())
> >>> -        LHSCV = ConstantExpr::getInferredCast(
> >>> -            LHSCV, false, LHSCV->getType()->getSignedVersion(),
> >>> true);
> >>> +        LHSCV = ConstantExpr::getBitCast(LHSCV,
> >>> +                                         LHSCV->getType()-
> >>>> getSignedVersion());
> >>>        if (RHSCV->getType()->isUnsigned())
> >>> -        RHSCV = ConstantExpr::getInferredCast(
> >>> -            RHSCV, false, LHSCV->getType(), true);
> >>> +        RHSCV = ConstantExpr::getBitCast(RHSCV, LHSCV->getType());
> >>>        return SCEVUnknown::get(ConstantExpr::getSDiv(LHSCV, RHSCV));
> >>>      }
> >>>    }
> >>
> >> As I mentioned before, these casts can all go away.  sdiv is signless
> >> now.
> >
> > One thing at a time.  This patch is getting rid of getInferredCast and
> > no more.
> 
> Fair enough, please make sure it happens at some point though.

Okay, these are done now.  Once stuff passes tests, I'll commit it.

Reid.

> 
> -Chris
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20061205/1a070edf/attachment.sig>