[PATCH] D42365: [LoopFlatten] Add a loop-flattening pass

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

Hello.

So when this comes up, it helps to simplify loops a little. The fourinarow test in the testsuite is helped out by this, decreasing the total time at -Os on a M23 by 1.5% (at -O3 the whole loop is just unrolled either way). It also helps knock off 32bytes off the total filesize. Both these results are a little higher than I would expect, considering it's only flattening a single loop.

For an M33, something like this loop takes 880 cycles when flattened, compared to 1350 cycles from before. Both LSR and the registry allocator seem to have a better time when they don't have to deal with outer loops, along with the obvious benefits of removing an outer level of compare/branches/IV etc.

  for (int i=0; i<N; i++)
    for (int j=0; j<N; j++)
      V[i*N+j] += val;

I don't have compile time numbers yet, but I don't expect this pass to be very heavy with the checks we are doing. I'll look into it to be sure.

When it comes up, it helps a little. Not much, but hopefully not an expensive pass to do so :)



================
Comment at: lib/Transforms/Scalar/LoopFlatten.cpp:114
+  if (!Compare ||
+      (Compare->getUnsignedPredicate() != CmpInst::ICMP_NE &&
+       Compare->getUnsignedPredicate() != CmpInst::ICMP_ULT) ||
----------------
samparker wrote:
> Why are these predicates valid and not any others?
My understanding is that indvars simplify will canonicalise this check to NE. I'll make sure this is actually correct and update things if I can get other forms to come up.


================
Comment at: lib/Transforms/Scalar/LoopFlatten.cpp:422
+  // (non-instructions are all inherently invariant).
+  auto InnerLimitInst = dyn_cast<Instruction>(InnerLimit);
+  if (InnerLimitInst &&
----------------
samparker wrote:
> Couldn't you just use 'OuterLoop->isLoopInvariant(InnerLimit)?
Nice. Like it.


https://reviews.llvm.org/D42365