[PATCH] D111300: [VPlan] Keep induction recipes in header.

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked 2 inline comments as done.
fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9333
 
         RecipeBuilder.setRecipe(Instr, Recipe);
+        if (isa<VPWidenIntOrFpInductionRecipe>(Recipe)) {
----------------
Ayal wrote:
> Worth a comment why VPWidenIntOrFpInductionRecipe deserves special displacement.
> 
> (Easiest to handle here; just raising some related thoughts:
> This is somewhat reminiscent of SinkAfter, in that recipes created next to their triggering ingredient need to be placed elsewhere; but SinkAfter is quite cumbersome. Other conceptual approaches may be to search for Trunc's when processing an IV Phi and produce a recipe for each (but current scheme produces a single recipe per triggering ingredient), and first hoisting the Trunc to appear in the header (i.e., cleaning up SinkAfter - perhaps once VPlan starts by representing the original scalar loop).)
Thanks, added a comment.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9339
+          else
+            Header->appendRecipe(Recipe);
+        } else
----------------
Ayal wrote:
> Suffice to have  `Header->insert(Recipe, Header->getFirstNonPhi())` for both cases?
> 
> May be worth wrapping within an insertPhi() method.
Updated to use `Header->insert()`, thanks! 

> May be worth wrapping within an insertPhi() method.

Not sure if it's worth adding that helper just now. I think it would be good to add once we have multiple users.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111300