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

Hi Evgeny, thank you for reporting the problem.

According to IR you posted the patch did what it expected to do.
Specifically, there is an instruction
%mul86.i = mul nuw nsw i32 %phitmp280.i, %phitmp.i
which most probably used in SCEV creation. Somehow SCEV lost nuw/nsw flags.

Before the patch, the matching S -> %mul86.i has been saved for later re-use.
Within this patch It does not keep this matching because SCEV without nuw/nsw should not correspond to instruction with nuw/nsw.

Later when someone expanded this SCEV S at point 
sub i32 %xtraiter289.i, ...

Before the patch it was able to re-use %mul86.i (as soon matching is present) and after the patch it could do so due to no matching.
Instead it literally expanded this SCEV to new mul instruction.

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


Repository:
  rL LLVM

https://reviews.llvm.org/D41578