[llvm] VPlan: introduce worklist in simplifyRecipes (PR #105699)

Ramkumar Ramachandra via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 26 03:27:09 PDT 2024


================
@@ -1009,8 +1015,16 @@ static void simplifyRecipes(VPlan &Plan, LLVMContext &Ctx) {
       Plan.getEntry());
   VPTypeAnalysis TypeInfo(Plan.getCanonicalIV()->getScalarType(), Ctx);
   for (VPBasicBlock *VPBB : VPBlockUtils::blocksOnly<VPBasicBlock>(RPOT)) {
-    for (VPRecipeBase &R : make_early_inc_range(*VPBB)) {
-      simplifyRecipe(R, TypeInfo);
+    // Populate a Worklist, as simplifyRecipe might return a new recipe that we
----------------
artagnon wrote:

> to be replaced by a small vector of recipes in the subsequent patch along with evident use for it (i.e., simplifying (X && Y) || (X && Z) into (X && (Y || Z)). BTW, this pattern could introduce a single new recipe - by modifying the RAUW'd OR to use Y and Z instead of (X && Y) and (X && Z) - but recipes modified during simplification should also be (re)visited, along with new ones.

Sorry, I'm having difficulty understanding this comment. How can the pattern introduce a single new recipe? If we return the recipe `X && (Y || Z)`, how will a pattern like `X && true` or `X && false` suffice? In other words, how can we ensure that `Y || Z` is re-queued for simplification? If we return the recipe `Y || Z`, that recipe might be simplfied, but then how does the root recipe involving X get re-queued for simplification?

> This patch is missing tests, demonstrating the simplification of new recipes introduced when simplifying others, i.e., its non-NFC-ness.

While I agree that this patch is technically not an NFC, we only return the `VPWidenCastRecipe` as a candidate for further simplification, and other patterns are not broken up. In practice, is it possible to cook up a test where the newly-created `VPWidenCastRecipe` actually gets simplified further?

https://github.com/llvm/llvm-project/pull/105699


More information about the llvm-commits mailing list