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

Sanjoy Das via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 20 01:12:29 PDT 2016

sanjoy requested changes to this revision.
sanjoy added inline comments.
This revision now requires changes to proceed.

Comment at: include/llvm/Analysis/ScalarEvolutionExpressions.h:552
+    // Currently only memoize results of visitMulExpr, because other
+    // calculations are much cheaper in the worst case.
+    DenseMap<const SCEV *, const SCEV *> MulRewriteResults;
Design-wise I'm not too happy to do see this happen only for multiplications -- have you considered over-riding the `visit` function and implementing the general logic there?

Comment at: include/llvm/Analysis/ScalarEvolutionExpressions.h:585
     const SCEV *visitMulExpr(const SCEVMulExpr *Expr) {
+      auto it = MulRewriteResults.find(Expr);
+      if (it != MulRewriteResults.end())

Comment at: include/llvm/Analysis/ScalarEvolutionExpressions.h:592
+      auto *MulExpr = SE.getMulExpr(Operands);
+      MulRewriteResults[Expr] = MulExpr;
+      return MulExpr;
I'd use `.insert`.


More information about the llvm-commits mailing list