[PATCH] D100751: [VPlan] Properly handle sinking of replicate regions.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8966
     VPRecipeBase *Sink = RecipeBuilder.getRecipe(Entry.first);
     VPRecipeBase *Target = RecipeBuilder.getRecipe(Entry.second);
     // If the target is in a replication region, make sure to move Sink to the
----------------
fhahn wrote:
> Ayal wrote:
> > fhahn wrote:
> > > fhahn wrote:
> > > > Ayal wrote:
> > > > > > I think we also need to deal with the case when the target is a
> > > > > > replicate region also. I'll put up a patch once I constructed a test
> > > > > > for that scenario.
> > > > > 
> > > > > Agreed. Worth adding an assert in the meanwhile?
> > > > That's a great idea, I added an assert.
> > > Actually this case is already handled, not sure if I was thinking about something else. I also updated the logic to check directly whether `sink` is in a replicate region, same as for `target`.
> > Worth mentioning where the case having both sink and target predicated is handled(?)
> > 
> > Better rename `Region` >> `TargetRegion`.
> Ah right, I missed the *also* when re-reading. So the unhandled case is when both sink and target are in separate regions! Let me look into adding an assert for that and update the naming.
I pushed 01c26d4e048c & 337d7652823f to rename the variable and add an extra assert.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100751/new/

https://reviews.llvm.org/D100751