[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#973699, @sanjoy wrote:

> In https://reviews.llvm.org/D41578#973016, @skatkov wrote:
>
> > What I see can be done here (any of):
> >
> > 1. Try to analyze why SCEV lost nuw/nsw information and possible fix/extend that place to preserve nuw/nsw in SCEV. In this case matching will still in place and new mul instruction will not be generated.
> > 2. Potentially "%mul86.i = mul nuw nsw i32 %phitmp280.i, %phitmp.i" can be replaced with "%16 = mul i32 %phitmp280.i, %phitmp.i". It can be done by some other pass but I'm not sure who should be responsible for that. I guess GVN can do this but not sure that it works after LSR in your pipeline.
> > 3. We can update this code to clean poison flags from instruction if built SCEV lost information about these flags. To my understanding cleaning of poison flags is a safe operation from correctness point of view but it can prohibit some optimization opportunities because we loose some valuable information.
> >
> >   To be honest I'm not sure what approach is a best here. I'd like to get a feedback from Sanjoy and others...
>
>
> Given that this pessimization is happening in LSR which runs pretty late, I would be okay with (3) -- most of the nsw/nuw flag specific optimizations would have already happened by now so there is no big reason to keep the flags anymore.


Hi Sanjoy, I guess this piece of code is common one and will be executed SCEV is created each time whenever it is a LSR or earlier passes. Do you propose to introduce some flag and use it in LSR?
This seems to me very intrusive fix.

I also thought about clearing of nuw/nsw at moment of expansion. Specifically when we check whether there is a instruction for SCEV, first we can give a priority for the instruction without nuw/nsw and second even if we decide to use instruction with nuw/nsw we can clean these flags at that moment. At least it should reduce the cases when we clean up these flags (only when expansion happens). I think the better fix would be really to understand why SCEV looses these flags and fix/extend that place and as I understand Evgeny looks in this direction.


Repository:
  rL LLVM

https://reviews.llvm.org/D41578