[PATCH] D41578: [SCEV] Do not cache S -> V if S is not equivalent of V

[Serguei Katkov via Phabricator via llvm-commits]

skatkov added a comment.

In https://reviews.llvm.org/D41578#978975, @eastig wrote:

> Hi Sergey and Sanjoy,
>
> To set flags ScalarEvolution::createSCEV calls:
>
>   SCEV::NoWrapFlags ScalarEvolution::getNoWrapFlagsFromUB(const Value *V) {
>     if (isa<ConstantExpr>(V)) return SCEV::FlagAnyWrap;
>     const BinaryOperator *BinOp = cast<BinaryOperator>(V);
>  
>     // Return early if there are no flags to propagate to the SCEV.
>     SCEV::NoWrapFlags Flags = SCEV::FlagAnyWrap;
>     if (BinOp->hasNoUnsignedWrap())
>       Flags = ScalarEvolution::setFlags(Flags, SCEV::FlagNUW);
>     if (BinOp->hasNoSignedWrap())
>       Flags = ScalarEvolution::setFlags(Flags, SCEV::FlagNSW);
>     if (Flags == SCEV::FlagAnyWrap)
>       return SCEV::FlagAnyWrap;
>  
>     return isSCEVExprNeverPoison(BinOp) ? Flags : SCEV::FlagAnyWrap;
>   }
>
>
> For "%mul86.i = mul nuw nsw i32 %p280, %p281" isSCEVExprNeverPoison return false which causes the SCEV '(%p280 * %p281)' to have SCEV::FlagAnyWrap instead of a combination of  SCEV::FlagNUW and Flags, SCEV::FlagNSW.
>
> Am I correct SCEV::FlagAnyWrap means no poisoned values? If so, this looks strange. Shouldn't it be:


As I understand this flag means have no idea, in other words may overflow.

> 
> 
>   return isSCEVExprNeverPoison(BinOp) ? SCEV::FlagAnyWrap : Flags;

No, I read this code as if isSCEVExprNeverPoison(BinOp) is true then we can trust the flags otherwise we cannot...

But it would be better to hear Sanjoy's opinion here.


Repository:
  rL LLVM

https://reviews.llvm.org/D41578