[PATCH] D39236: [SCEV][NFC] Introduce isSafeToExpandAt function to SCEVExpander

[Philip Reames via Phabricator via llvm-commits]

reames requested changes to this revision.
reames added a comment.
This revision now requires changes to proceed.

relatively minor code comments.



================
Comment at: include/llvm/Analysis/ScalarEvolutionExpander.h:30
   /// Return true if the given expression is safe to expand in the sense that
   /// all materialized values are safe to speculate.
   bool isSafeToExpand(const SCEV *S, ScalarEvolution &SE);
----------------
Adjust this comment to reflect the requirement to check dominance.  

i.e. "are safe to speculate anywhere their operands are defined"


================
Comment at: lib/Analysis/ScalarEvolutionExpander.cpp:2294
+                      ScalarEvolution &SE) {
+  return isSafeToExpand(S, SE) && SE.dominates(S, InsertionPoint->getParent());
+}
----------------
>From the comment, this sounds like it's going to be smarted than isSafeToExpand where it's really more restrictive.

Maybe add a TODO about handling cases which aren't *generally* safe to speculate but are safe to speculate *at this particular location*?


https://reviews.llvm.org/D39236