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

[Sanjoy Das via llvm-commits]

sanjoy added inline comments.


================
Comment at: include/llvm/Analysis/ScalarEvolutionExpressions.h:540
 
   /// Recursively visits a SCEV expression and re-writes it.
   template<typename SC>
----------------
lihuang wrote:
> sanjoy wrote:
> > lihuang wrote:
> > > sanjoy wrote:
> > > > Please document the fact that this only implements "pure" transforms.
> > > Sorry, but by "pure" transform do you mean identity transform? So what about:
> > > 
> > > /// Recursively visits a SCEV expression and re-writes an identical SCEV.
> > I meant a transform that maps equal inputs to equal outputs.  For instance, I cannot use this framework to implement a transform that maps all `SCEVConstant` s to `getConstant(Counter++)` since then the caching scheme won't work.
> I see, thanks. What about:
> 
> /// This visitor recursively visits a SCEV expression and re-writes it. The result from each visit is cached, so it will return the same SCEV for the same input.
sgtm


https://reviews.llvm.org/D25810