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

[Evgeny Astigeevich via Phabricator via llvm-commits]

eastig added a comment.

In https://reviews.llvm.org/D41578#979947, @skatkov wrote:

> > 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.


When SCEV::FlagAnyWrap is set SCEVNAryExpr::hasNoUnsignedWrap and SCEVNAryExpr::hasNoSignedWrap return false:

  bool hasNoUnsignedWrap() const {
    return getNoWrapFlags(FlagNUW) != FlagAnyWrap;
  }
  
  bool hasNoSignedWrap() const {
    return getNoWrapFlags(FlagNSW) != FlagAnyWrap;
  }

>From this I can conclude SCEV::FlagAnyWrap mean no poisoned values.

>> 
>> 
>>   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.

Sergey, it looks like your patch does not take into account that nsw and nuw have not been set because of additional analysis, e.g. isSCEVExprNeverPoison. So SCEVLostPoisonFlags thinks flags are lost but they were not set intentionally. Where the issue your patch fixes come from? I don't see any IR test.


Repository:
  rL LLVM

https://reviews.llvm.org/D41578