[PATCH] D124921: [SCEV] Fold umin_seq to umin using implied poison reasoning

[Philip Reames via Phabricator via llvm-commits]

reames accepted this revision.
reames added a comment.
This revision is now accepted and ready to land.

LGTM w/minor comments



================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:4042
+      }
+      return true;
+    }
----------------
Can you add a small comment reminding the reader that constantexprs are not SCEVConstants?  Save the next person the quick search to confirm.  :)


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:4052
+  visitAll(AssumedPoison, PC1);
+
+  // Collect all SCEVs in S that, if poison, *will* result in S being poison
----------------
Can you add the early return here for when PCI.MayBePoison is empty?  Might as well avoid walking the second SCEV.

Also gives you a good place to put for your comment about the corollary.  


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124921/new/

https://reviews.llvm.org/D124921