[PATCH] D51286: [IndVars] Fix usage of SCEVExpander to not mess with SCEVConstant. PR38674

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added a comment.

In https://reviews.llvm.org/D51286#1222501, @mkazantsev wrote:

> In https://reviews.llvm.org/D51286#1222271, @etherzhhb wrote:
>
> > Is it possible to fix the problem by directly codegening the constant from SCEVConstant in expandSCEVIfNeeded?
>
>
> What's the point of that? I generally don't see any profit in going manually into rewriting internals and trying to find something in its map. Expand methods are designed to handle such cases.






================
Comment at: test/Transforms/IndVarSimplify/pr38674.ll:103
+; CHECK:       return:
+; CHECK-NEXT:    ret i32 [[INC11_LCSSA]]
+;
----------------
greened wrote:
> Is it really necessary to check all of the generated IR like this?  Could the test be written to only check the instructions participating in the LCSSA value flow?  If so, it would make the test more robust.
I'm having a crawling suspicion that this algorithm is generally wrong in terms of preserving SSA form. I would rather keep the tests verbose as possible just to make sure that we don't break it anyhow else.



https://reviews.llvm.org/D51286