[PATCH] D55373: [LSR] Generate formulae to enable more indexed accesses

Mon Feb 4 03:37:28 PST 2019

samparker marked 2 inline comments as done.
samparker added inline comments.

================
Comment at: lib/Transforms/Scalar/LoopStrengthReduce.cpp:159
+static cl::opt<bool> CollapseUnrolledCode(
+  "lsr-collapse-unrolled", cl::Hidden, cl::init(false),
+  cl::desc("Narrow LSR search space by collapsing unrolled code"));
----------------
gilr wrote:
> samparker wrote:
> > gilr wrote:
> > > False by default?
> > This option has been introduced to force the collapsing, even if the complexity limit hasn't been reached. Which is why it's implemented differently to the other, similar, options.
> Aaargh, sorry, read it backwards ...
> Is it needed since unrolled code doesn't initially go into the same LSRUse? If so, can FavorBackedgeIndex be used to have initial construction put them in the same LSRUse?
I will look into this, thanks.

================
Comment at: lib/Transforms/Scalar/LoopStrengthReduce.cpp:3121
     assert(UseI != Inc.UserInst->op_end() && "cannot find IV operand");
+    if (FavorBackedgeIndex && UseI->getUser() == Chain.lastNonPHI())
+      continue;
----------------
gilr wrote:
> I'm a bit confused here. IIUC a profitable complete chain prvides an efficient solution using post-increments. Why break the last increment out?
This was so that the last access could also be a target for optimisation. I'll remove the change and do some testing again to see the difference.

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

https://reviews.llvm.org/D55373