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

[Renato Golin via Phabricator via llvm-commits]

rengolin added inline comments.


================
Comment at: lib/Transforms/Vectorize/VPlan.h:457
   }
 
+  /// Set a given VPBlockBase \p Successor as the single successor of this
----------------
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.


https://reviews.llvm.org/D44338