[PATCH] D44338: [LV][VPlan] Build plain CFG with simple recipes for outer loops.

[Diego Caballero via Phabricator via llvm-commits]

dcaballe added inline comments.


================
Comment at: lib/Transforms/Vectorize/VPlan.h:457
   }
 
+  /// Set a given VPBlockBase \p Successor as the single successor of this
----------------
rengolin wrote:
> dcaballe wrote:
> > This is the rationale behind the changes below: I wouldn't like VPBlockBase to end up with a large list of interfaces to set/insert successors/predecessors, all of them doing almost the same with very subtle differences. For that reason, I introduced the class VPBlockUtility to keep there all the VPBlockBase manipulation interfaces and keep VPBlockBase class cleaner. Of course, that doesn't mean that we want to add unnecessary utilities to VPBlockUtility class.
> > 
> > In VPBlockBase class, I decided to just keep the very very basic interfaces, which can be used directly or can be a building block of a more complex utility in VPBlockUtility. In this regard, I'm simplifying the logic of setOneSuccessor and setTwoSuccessors and replacing their existing calls with a more generic utility function in VPBlockUtility. The previous implementation was very ad-hoc for the patch where they were introduced and I couldn't reuse them as is.
> > 
> > Of course, I'm open to other suggestions.
> I like this idea and I like the methods, as they give a better impression as to what is truly happening behind the scenes.
> 
> I agree we shouldn't bloat this class, but it's a good buffer class to have during the prototype phase of the outer loop implementation, so we can gauge the BPBasicBlock usability.
Thanks, Renato!

>I agree we shouldn't bloat this class, but it's a good buffer class to have during the prototype phase of the outer loop implementation, so we can gauge the BPBasicBlock usability.

Agreed. Any particular suggestion on moving some of the current utilities in VPBlockUtils back to VPBlockBase? I think the current ones are OK there but let me know if you have any comments in that regard.



https://reviews.llvm.org/D44338