[PATCH] D13242: [SCEV] Factor out common visiting patterns for SCEV rewriters. NFC.

[Sanjoy Das via llvm-commits]

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

lgtm with comments addressed (addressing them in a separate change is fine).


================
Comment at: include/llvm/Analysis/ScalarEvolutionExpressions.h:556
@@ -555,7 +555,3 @@
 
-  typedef DenseMap<const Value*, Value*> ValueToValueMap;
-
-  /// The SCEVParameterRewriter takes a scalar evolution expression and updates
-  /// the SCEVUnknown components following the Map (Value -> Value).
-  struct SCEVParameterRewriter
-    : public SCEVVisitor<SCEVParameterRewriter, const SCEV*> {
+  /// SCEVRewriteVisitor - Recursively visits a SCEV expression
+  /// and re-writes it.
----------------
We're moving away from `Entity Name - description` style comments.

================
Comment at: include/llvm/Analysis/ScalarEvolutionExpressions.h:670
@@ -656,4 +669,3 @@
   /// the Map (Loop -> SCEV) to all AddRecExprs.
-  struct SCEVApplyRewriter
-    : public SCEVVisitor<SCEVApplyRewriter, const SCEV*> {
+  class SCEVApplyRewriter : public SCEVRewriteVisitor<SCEVApplyRewriter> {
   public:
----------------
I see you didn't add this code; but a name more descriptive than `SCEVApplyRewriter` may be better (unless "apply" has some special meaning I'm not aware of).

================
Comment at: include/llvm/Analysis/ScalarEvolutionExpressions.h:692
@@ -716,3 +691,3 @@
 
       const SCEVAddRecExpr *Rec = (const SCEVAddRecExpr *) Res;
       return Rec->evaluateAtIteration(Map[L], SE);
----------------
Looks like this code was pre-existing; but can you please change this to do a `cast<>` (in this change or a later change).


http://reviews.llvm.org/D13242