[PATCH] D32871: [LV] Using VPlan to model the vectorized code and drive its transformation

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added a comment.

In https://reviews.llvm.org/D32871#760712, @rengolin wrote:

> Hi Ayal, sorry for the delay. Can you commit the plan doc separately?
>
> We have a `docs/Proposals` sub-directory that is perfect for that kind of document. It should be fine to commit there directly, as none of what's in there is "an official LLVM document" and can change with time.
>
> It would also be good to get a link to that document from the existing vectoriser documents (as an FYI on what we're working), but once VPlans are fully in, we should move the core part of the VPlan doc into the vectorisers (or just move them outside of `Proposals`), so that they become official.
>
> I'll start looking at the code changes this week.
>
> cheers,
> --renato


Sure, done (r304161), thanks!



================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:470
 
+public:
   /// A helper function that computes the predicate of the block BB, assuming
----------------
rengolin wrote:
> The public protected swap is confusing. I'd create a "protected bubble" and move the methods inside it.
Sorry for this confusion. The idea was to avoid moving methods around to simplify the diff:

> Protected methods made public to enable their reuse are annotated as such in-place to simplify the diff. They will eventually be moved and grouped together.

An alternative is to use "friend" instead.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:497
+  /// inclusive..
+  void scalarizeInstruction(Instruction *Instr, const VPIteration &Instance,
+                            bool IfPredicateInstr);
----------------
rengolin wrote:
> Not virtual anymore? Should it be marked "final"?
This should have been part of r297580 originally.

In general ILV's methods are marked virtual iff they're overridden by Unroller; otherwise they may be marked "final".

This is unrelated to VPlan. Sorry for the confusion.


https://reviews.llvm.org/D32871