[PATCH] D30446: [IndVars] Do not branch on poison

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

We just ran across this again, with a bit more of an urgent motivation this time.  I'd very much like to see this patch make progress.

If I follow the discussion so far, it basically comes down to:

1. We should factor out the LFTR code such that it can be used by LSR as well as IndVarSimplify.
2. Once done, we should restrict the IndVar version to "do no harm".  Specifically, this means that we do *not* strip nsw/nuw flags, we simply don't convert in that case.
3. The LSR version should use the "strip flags if needed" strategy.

Is everyone comfortable with that direction?

Looking at a few of the test cases Sanjoy mentioned, I think we could also trivially re-infer flags in several of the cases.  We could potentially integrate that logic into the IndVar version of the code if desired - i.e. trigger if either no-nsw, or could trivially infer nsw on IV w/new use added.



================
Comment at: lib/Transforms/Scalar/IndVarSimplify.cpp:2174
+    SmallVector<const Value *, 8> NotPoison;
+    propagateKnownNonPoison(BECond, NotPoison, InLoop);
+
----------------
Ok, seeing the usage here, I think this is probably the wrong interface for the problem even if we want to pursue this approach.  I think we'd be much better off with an analysis which provided an interface for asking whether a given Instruction is known to be non-poison.  Your IV analysis could be used, but you could also walk forward through uses to by-assumption non-poison use as well.  


================
Comment at: lib/Transforms/Scalar/IndVarSimplify.cpp:2193
+
+  if (PoisonGenerator) {
+    PoisonGenerator->dropPoisonGeneratingFlags();
----------------
minor: can't we do this before the more general search since we already have the information?  Or are you trying to check where the new IV itself is known non-poison?


https://reviews.llvm.org/D30446