[PATCH] [LoopDist/LoopVer] Move LoopVersioning to a new module, NFC
anemet at apple.com
Mon Jun 22 23:00:33 PDT 2015
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);
> 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);
> 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
More information about the llvm-commits