[PATCH] [LoopDist/LoopVer] Move LoopVersioning to a new module, NFC

[Adam Nemet]

================
Comment at: include/llvm/Transforms/Utils/LoopVersioning.h:44
@@ +43,3 @@
+  /// \brief Performs the CFG manipulation part of versioning the loop including
+  /// the DominatorTree and LoopInfo updates.
+  void versionLoop(Pass *P);
----------------
hfinkel wrote:
> anemet wrote:
> > hfinkel wrote:
> > > From this interface, I find it very unclear how you get out the new versioned loop after the CFG mutation is complete.
> > The loop that was used to construct the class will be the "versioned" loop i.e. the loop that will get control if all the memchecks pass.
> > 
> > This is the loop that typically, the client transform will be interested in modifying.  E.g. this is what LoopDistribute does.
> > 
> > This is sort of implicit because the versioning is optional.  I think this is a typical algorithm:
> > 
> >   for each loop L:
> >     analyze L
> >     if versioning is necessary version L
> >     transform L
> > 
> > The versioning itself is an optional step and you don't change the loop underlying the transformation if versioning was necessary.
> > 
> > Both loops (versioned, unversioned) are attributes of the class, so we can add APIs to query them as we need them.
> > 
> > Let me know if you'd be OK just commenting the above or you're requesting an actual API change.
> > 
> > Thanks very much for the review!
> > 
> > Adam
> In that case, I think the current API is okay, but needs comments.
OK, will add.

================
Comment at: include/llvm/Transforms/Utils/LoopVersioning.h:49
@@ +48,3 @@
+  /// loop-defined values used outside of the loop.
+  void addPHINodes(const SmallVectorImpl<Instruction *> &DefsUsedOutside);
+
----------------
hfinkel wrote:
> This function too needs comments on when/how it should be used.
Sure, I'll explain it.

Just FYI, as I was mentioning in the summary, I'd like to make this a private member function in a follow-on patch, so clients would not have to worry about when/where to call it.  This can be called at the end of versionLoop, internally.

Where I'd like to end up is something like this:

* move findDefsUsedOutside to LoopUtils.h or something like that
* have LoopVersioning take an optional DefsUsedOutside vector in the ctor.  Optional, because like in the case of LoopDistribution the pass itself needs this piece of information so it's readily available
* compute DefsUsedOutside using findDefsUsedOutside if it wasn't passed
* call addPHINodes if there are DefsUsedOutside

http://reviews.llvm.org/D10577

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/