[PATCH] D68814: [LV] Allow assume calls in predicated blocks.

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added a comment.

In D68814#1717282 <https://reviews.llvm.org/D68814#1717282>, @Ayal wrote:

> > [snip]
>
> I vote to simply drop conditional assumes as a first step, at this time. [snip] Following D68577 <https://reviews.llvm.org/D68577> such assumes could be dropped as a VPlan-to-VPlan transformation.


The alternative mentioned above of following D68577 <https://reviews.llvm.org/D68577> would result in something like

  + auto &ConditionalAssumes = Legal->getConditionalAssumes();
  ...
    // Mark instructions we'll need to discard later as
    // ingredients whose recipe we'll need to record.
  +  for (auto ConditionalAssume : ConditionalAssumes)
  +    RecipeBuilder.recordRecipeOf(ConditionalAssume);
  +
  ...
  +  // Discard conditional assumes.
  +  for (auto ConditionalAssume : ConditionalAssumes)
  +    RecipeBuilder.getRecipe(ConditionalAssume)->eraseFromParent();

where Legal->getConditionalAssumes() is analogous to the proposed Legal->mustDropAssumes(), returning the set of conditional assumes rather than boolean.

Hence the vote to simply include all ConditionalAssumes in DeadInstructions instead.

Having a VPInstruction-based VPlan-to-VPlan transformation would be fine, of course, which drops assumes in if-converted blocks and keeps those under (retained, if any) uniform branches. For buildVPlanWithVPRecipes, however, it would be better to make progress on refactoring Recipes into VPInstructions, wrt assume intrinsics in this case, than to start befriending Recipes and examine their ingredients. As for the VPInstruction-based transformation, should VPInstruction provide an adequate interface (isAssume(), or perhaps an Assume Opcode?) rather than befriending it and examining its UnderlyingInstr(?).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68814