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

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

Just some random thoughts after glancing at this for a few moments.  Don't take them too seriously.

You mention the alternate urem/udiv form and reject it as non-profitable.  While I see your point, I also see code that is structured in a way which ties legality and profitability.  I'd suggest considering whether you could separate cost model and mechanism.  As one for instance, could you have a transform which produced the urem/udiv form and then simplified them away if possible?  And then a default cost model which only allowed the transform in that case?  The advantage of structuring it that way would be ease of testing, and later extension.

Another alternative to the urem/udiv form would be to introduce 3 IVs - one overall, one current outer, and one current inner.  Instead of using an explicit udiv/urem, you could generate code which looks something like:
int inner = 0, outer = 0;
for (int i = 0; i < M * N; i++) {

  { /* body of inner */}
  inner++;
  if (inner == N) {
    inner = 0;
    outer++;
  }

}

This is, of course, just a weird way of writing a nested loop, and LoopSimplify will probably try to turn it back into a nested loop.  However, if you knew part of inner was going to simplify, the transformation might be profitable.    The advantage to this form is that since it just a weirdly written nested loop, the cost modelling could be slightly fuzzier as there's little danger of getting horrible performance either way.

Of course, an ideal world, LSR would reliably produce the later form from the former.  So, that might be worth exploring too.  :)

If you can, please make sure that at least the inner loop allows multiple exiting blocks.  Multiple exits are rather common in some workloads, so supporting them is appreciated if it's straight forward.  (For this transform, it should be, at least in theory.)

Finally, make sure you consider the nest formation code in LoopSimplify.  We need to make sure that the two places agree on what profitable nests are.  We certainly wouldn't want them fighting each other!


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

https://reviews.llvm.org/D42365