[PATCH] D12284: [LoopVectorize] Extract InductionInfo into a helper class

James Molloy via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 27 02:54:41 PDT 2015


jmolloy added a comment.

I've committed this with changes suggested by Adam in r246145.

> Also please upload patches with full context.


Ach, sorry!

James


================
Comment at: include/llvm/Transforms/Utils/LoopUtils.h:215
@@ +214,3 @@
+public:
+  /// Default constructor made public for convenience.
+  InductionDescriptor()
----------------
anemet wrote:
> That's a pretty strange comment.  It's made public so that isInductionPHI can do the actual initialization.
> 
> I actually don't think that's a very nice interface.  If you want something like a factory method to create InductionDescriptors then I would name it something like createFromPHI.  Then have an isInduction() member to check that the descriptor is an induction variable (!IK_NoInduction).
I fully agree - this interface is terrible. The reason I did it like this was twofold:
  1) Reduce code churn in one commit
  2) It matches the equally horrible isReductionPHI API.

I fully intend to go on a redesign here and make a much better API. It's just a nicer starting point if the induction code is already hoisted into Utils.


Repository:
  rL LLVM

http://reviews.llvm.org/D12284





More information about the llvm-commits mailing list