[PATCH] D35931: [SCEV] Do not visit nodes twice in containsConstantSomewhere

[Sanjoy Das via Phabricator via llvm-commits]

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

This LGTM, but I think the use site is using too large of a hammer.  It only cares about binary add instructions in which one of the operands is a multiplication with a constant or a constant.  We should write a check for just that, instead of traversing the whole expression like this.

For instance, I don't think the user should apply the transform in this case `(C0 + X) * Y + Z` (going by the comments) but it will apply this transform today.

Do you mind fixing that in a separate change?



================
Comment at: lib/Analysis/ScalarEvolution.cpp:2685
+      if (isa<SCEVConstant>(S)) {
+        FoundConstant = true;
+        return false;
----------------
This is optional and stylistic, but you could have been briefer as:

```
bool follow(const SCEV *S) {
  FoundConstant = isa<SCEVConstant>(S);
  return isa<SCEVAddExpr>(S) || isa<SCEVMulExpr>(S);
}
```


https://reviews.llvm.org/D35931