[PATCH] D58770: [LSR] Attempt to increase the accuracy of LSR's setup cost

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: lib/Transforms/Scalar/LoopStrengthReduce.cpp:1224
+    return getSetupCost(S->getStart());
+  if (!EnableRecursiveSetupCost)
+    return 0;
----------------
samparker wrote:
> dmgreen wrote:
> > samparker wrote:
> > > Shouldn't we have this before the first recursive call?
> > It's was trying to replicate the behaviour from before this patch, so allowing the recursion into the addrec. EnableRecursiveSetupCost may not have been the best name for that exactly. I'll update the code to match the name better.
> Fair enough, then maybe we should still check for a constant start for the AddRec even when recursion isn't enabled?
Not sure. This version here is probably cleanest going forward. The earlier version is more like the old behaviour, but not identical.

I don't think it's worth replicating the old behaviour exactly, if it's going to look ugly with explicitly checking AddRec start's, as opposed to just relying on the recursion. Unless we see regressions come up from this, which I don't think we should.

But I don't have a strong opinion. Let me know what you think.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58770/new/

https://reviews.llvm.org/D58770