[PATCH] D25810: [SCEV] Memoize visitMulExpr results in SCEVRewriteVisitor. Fix PR18606

[Sanjoy Das via llvm-commits]

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

lgtm with nits



================
Comment at: include/llvm/Analysis/ScalarEvolutionExpressions.h:540
 
   /// Recursively visits a SCEV expression and re-writes it.
   template<typename SC>
----------------
Please document the fact that this only implements "pure" transforms.


================
Comment at: include/llvm/Analysis/ScalarEvolutionExpressions.h:560
+      auto *Result = SCEVVisitor<SC, const SCEV *>::visit(S);
+      RewriteResults.insert({S, Result});
+      return Result;
----------------
Just to be pedantic, I'd assert here that you did in fact insert a new entry.


================
Comment at: test/Analysis/ScalarEvolution/pr18606.ll:3
+
+; CHECK: main
+
----------------
Please add a basic output sanity check.


https://reviews.llvm.org/D25810