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

[Adam Nemet via llvm-commits]

anemet added a comment.

Hi James,

Now, I read the whole patch and I guess I am still more-or-less left with my original questions from the RFC thread.

Can we further decompose the functionality beyond LoopEditor?  One straw-man I was thinking it to focus on the widening functionality.  Both interleaving and loop-vectorization could be considered widening differing only through the means they achieve widening (by vectorizing or duplicating instructions).  Then obviously the hook could provide the instruction-specific widening operation and how reduction values should be recovered at the exit of the loop.

I would also encourage you to use LoopVersioning.  The idea was certainly to convert the vectorizer to be another client of LVer.  If you need to add hooks or what not feel free.  The plan for LoopVersioning has always been to go beyond just alias-checks (we have immediate plans to add dynamic loop-trip count checks) so having to reimplement this at various places would be a mistake now that I finally refactored it.

I would also think that it would be a design mistake to re-implement LoopVersioning on top of something more complex, i.e. LoopEditor.  I rather have as simple classes as possible for things like LoopVersioning, LoopWidening, LoopPeeling, etc and then compose transformations using these classes.  This would make things more explicit and easier to reason about correctness: input, ouput state, required analyses, modified analyses.  (Having AnalysisLevel in the class is an indication to me the class is not properly decomposed.)

It also unclear to me why want to pin down the API by committing it with the implementation stubbed out.  I think it's good to discuss the end goal but why can't we invent and refine the API gradually as you start refactoring and transitioning over the existing functionality?

Thanks,
Adam


Repository:
  rL LLVM

http://reviews.llvm.org/D11530