[PATCH] D11530: RFC: LoopEditor, a high-level loop transform toolkit

[James Molloy via llvm-commits]

jmolloy added a comment.

Hi Adam,

I've been working on this for the past few days and wanted to give an update. The most interesting/annoying thing about this all is exactly how to split out/share code with the loopvectorizer.

My ideal would be to take the LoopVectorizer code and refactor it piece by piece until it's in a modular enough shape to encapsulate in a utility class. This would mean we'd maintain 100% test coverage throughout the process.

Unfortunately the code in question is a bit of a monolith and has resisted all of my attempts at chiselling into better shape. The main problem is that any sane composition model would perform a sequence of operations on a loop, mutating it in place. This is the model I'd like to get to. But the current model is to do one pass over the source loop, injecting any cloned instructions into a single IRBuilder.

This has meant that everything we do in the loop vectorizer happens in one place. If-conversion is done on-the-fly, as are induction PHI updates and remaps. Because of this it's very difficult to isolate one piece of functionality and refactor it - it's all or nothing.

So what I'd like to do is to incrementally create the helper classes LoopWidening/LoopInterleaving/LoopVectorizing separately. In order to get test coverage and move towards an end goal, I'd hook the new helper classes into LoopVectorize and make LoopVectorize use the new classes when possible. Something like this:

   if (!VectorizeLoop) {
    assert(IC > 1 && "interleave count should not be 1 or 0");
    // If we decided that it is not legal to vectorize the loop then
    // interleave it.
    if (canBeHandledByLoopInterleaving()) {
      LoopVersioning LVer(L);
      LVer.versionLoop();
      LoopInterleaving LInt(LVer.getVersionedLoop());
      LInt.widenLoop();
    } else {
      InnerLoopUnroller Unroller(L, SE, LI, DT, TLI, TTI, IC);
      Unroller.vectorize(&LVL);
    }
    emitOptimizationRemark(F->getContext(), DEBUG_TYPE, *F, L->getStartLoc(),
                           Twine("interleaved loop (interleaved count: ") +
                               Twine(IC) + ")");
  } else {
    ...

Initially the new codepath would be taken only when the loop is very simple; it has only a single block, for example (or a debug option forced it).

I'm still not 100% happy that I haven't found a way to refactor the code already there :( What do you think? I don't want to spend a massive amount of time going in the wrong direction.

Cheers,

James


Repository:
  rL LLVM

http://reviews.llvm.org/D11530