[PATCH] D74695: [VPlan] Replace VPWidenRecipe with VPInstruction (WIP).

[Florian Hahn via Phabricator via llvm-commits]

fhahn updated this revision to Diff 250439.
fhahn added a comment.

Rebased & updated to only use VPWidenInstruction in VPIRBuilder if there is an underlying value.

In D74695#1916542 <https://reviews.llvm.org/D74695#1916542>, @Ayal wrote:

> Breaking the recipes into VPInstructions is indeed on the roadmap for evolving VPlan to support transformations, cost modeling, and more.
>
> The approach we’ve taken so far is to first migrate the def-use dependencies between recipes, which originally rely on the underlying immutable def-use dependencies among their IR ingredient Instructions and their Operands, to VPlan VPValue-VPUser dependencies. Once two recipes feed each other via a VPValue-VPUser pair, new VPInstructions can be introduced freely between them. The primary motivation for this approach is to facilitate VPlan def-use analysis and transformations, including reordering recipes, replacing them, and introducing new VPInstructions. Once the uses of all recipes migrate to VPUsers, a rather mechanical process following D70865 <https://reviews.llvm.org/D70865>, any single-def recipe can be converted independently into a VPInstruction. VPWidenRecipe is indeed an easy first candidate, which would break up naturally at this time.
>
> An approach that first breaks recipes into VPInstructions, will migrate all def-use dependencies when the *last* recipe is broken. Other recipes are however more challenging to break than VPWidenRecipe, because they have multiple defs, entail SCEV expansions, un/pack predicated scalars along a to-be-unrolled loop of VF*UF iterations, and more. These design challenges require careful attention, which should preferably be devoted in parallel to facilitating VPlan transformation rather than blocking it.
>
> Breaking VPWidenRecipe into a VPInstruction at this time, by letting dependent recipes continue to rely on their underlying IR Operands, via a lookup for potential VPValues (VPInstructions) that cover their corresponding IR Value def, presumably builds a VPlan def-use graph but in effect does not support introducing a new VPInstruction between a VPWidenInstruction and its users. It seems preferable to first focus on migrating away from the original IR def-use relations, gradually and consistently.


Thanks for the detailed response! I agree we should focus on migrating the def-use dependencies of the existing recipes. I think doing so for VPWidenRecipe might be more tricky than other recipes, as VPWidenRecipe currently contains a list of instructions. To me it seems breaking VPWidenRecipe up into a new VPInstructions makes modeling the def-use chain easier (the operands are explicit per VPInstruction) and allows us to make progress on 2 fronts: migrating def-uses and getting rid of a recipe. It also should not hinder migrating the def-use dependencies for other recipes, but rather help because we can directly use the VPWidenInstruction as VPValue inputs for other recipes.

I thought a little bit about how to migrate the def-use dependencies for VPWidenRecipe without breaking it up, but couldn't come up with a more straight-forward approach. I might be missing something though and would also be happy to explore other directions. Given that the existing approach in this patch seems to work well in practice (and it makes progress in 2 directions), I think it would allow us to continue moving in the right direction, while we should continue to focus on migrating the def-use dependencies for recipes that cannot be broken up easily.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74695

Files:
  llvm/lib/Transforms/Vectorize/LoopVectorizationPlanner.h
  llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
  llvm/lib/Transforms/Vectorize/VPRecipeBuilder.h
  llvm/lib/Transforms/Vectorize/VPlan.cpp
  llvm/lib/Transforms/Vectorize/VPlan.h
  llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
  llvm/lib/Transforms/Vectorize/VPlanValue.h
  llvm/unittests/Transforms/Vectorize/VPlanHCFGTest.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D74695.250439.patch
Type: text/x-patch
Size: 27393 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200315/9c65759d/attachment.bin>