[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