[PATCH] D26555: [LV] Keep predicated instructions in the same block

[Matthew Simpson via llvm-commits]

mssimpso added inline comments.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:7099
 
-  VectorParts Cond;
-  if (IfPredicateInstr)
-    Cond = createBlockInMask(Instr->getParent());
+  // If the instruction requires predication, emit the edge masks for its
+  // predecessor blocks. The masks will be stored in MaskCache and made
----------------
gilr wrote:
> mssimpso wrote:
> > gilr wrote:
> > > Why force the creation of the edge masks here?
> > We have to create the edge masks in program order because they are cached in MaskCache. PHI widening, for example, also calls createEdgeMask. During PHI widening, if a mask isn't already in the cache, we will produce it at that time. But then when we go back to perform the actual predication, we would reuse the masks we created for the PHIs. But these masks may not dominate the branches we create for the predicated blocks, so we have to produce the masks in order. The existing tests in if-pred-non-void.ll require this.
> Ahhh ... right.
> This seems somewhat unclean, though: the patch moves block mask creation logic from scalarization to predication (which makes sense), but must leave some of it behind for caching reasons.
> So as long as we're generating masks in program order for caching reasons, could the original createBlockInMask call be retained if the same caching was added for block masks? This would make it clear that it's the block mask we need generated here, avoid partly-duplicating its code and make the masks caching behavior more consistent (more of a getOrCreate API).
Sure, we can do that. I'll add a cache for the block-in masks and update the patch. Thanks!


https://reviews.llvm.org/D26555