This minimal improvement is fine, thanks! Adding some optional nits:

Nice clean-up!
Adding a nit for potential further clean-ups.

This fix is fine, thanks, adding a minor nit.

In D127966#3612813, @fhahn wrote:

In D127966#3612708, @Ayal wrote:

In D127966#3611652, @fhahn wrote:

Add extra check if there are any runtime checks before creating LoopVersioning, move code up.

Great, thanks!
Currently noalias is introduced by emitMemRuntimeChecks() when called from ILV::createVectorizedLoopSkeleton() and from its overriding EpilogueVectorizerMainLoop::createEpilogueVectorizedLoopSkeleton(), but is missed by its overriding EpilogueVectorizerEpilogueLoop::createEpilogueVectorizedLoopSkeleton(). How about placing it in completeLoopSkeleton(), which all three skeleton creators call? It does, after all, seem to belong to ILV* than to LVP?

The follow-up patches move addMetadata to VPTransformState, which requires VPTransformState to have access to LoopVersioning. If we move the code to completeLoopSkeleton, it would have to be updated to return the LoopVersioning. Please let me know if you prefer it that way and I'll updated the patch :)

In D127965#3612002, @fhahn wrote:

Address latest comments, thanks!

Nice move! Would it be reasonable to also have a matching VPlanRecipes.h, excluding VPRecipeBase?

I think this would require a bit more surgery, as VPlan.h contains multiple function definitions that require the full definition of different recipes types. This could be resolved by moving them to VPlan.cpp, but I think that would be best done separately. Also, I think in most file we would have to include both headers in any case?

In D127966#3611652, @fhahn wrote:

Add extra check if there are any runtime checks before creating LoopVersioning, move code up.

Ship it!

Ship it!

Curious if current lack of proper noalias metadata of an epilogue vector loop may result in missed optimization or potentially wrong code?

Nice move! Would it be reasonable to also have a matching VPlanRecipes.h, excluding VPRecipeBase?

Looks good to me!

Ship it!

This looks good to me, thanks!

This looks good to me, thanks!
Adding minor nits.

This looks good to me, ship it!

Should tests check for new BranchOnCond VPInstruction?

Thanks for following up! Minor nits to help keep consistency.

Thanks for following up!

Thanks for following up!

Looks good to me, thanks!

Looks good to me! Adding minor suggestions.

Summary deserves updating.

Looks good! Minor documentation update nits.

Looks good to me, adding a couple of nits.

Ship it!
minor nits.

Looks good to me.
Can this too lead to different - more accurate decisions? If so wonder if that could be tested(?)

Looks good to me, curious if it may lead to any regression...
Adding minor nits.

This looks good to me, thanks for accommodating, ship it!
Adding minor nits.

Note that there are some test changes, because we now can correctly look through instructions like truncates to analyze the actual users.

Nice preparation for expanding VPlan's scope to include exit/middle-block after its expansion to include preheader.
Looks good to me, adding minor nits.

In D122096#3429527, @fhahn wrote:

Ping :)

This looks fine! Adding a couple of nits and future thoughts:

This is fine, thanks!

This is fine; adding a clarifying comment.

Looks fine to me, with a couple of nits.

Looks good to me!

A good step forward in refactoring and simplifying induction variable recipes, raising thoughts for follow-up steps. Ship it!

Nice! Looks good to me.

Nice clean-up, effectively retiring ILV's widenPHIInstruction(), completing its refactoring into more specific recipes.
Adding various nits.

Ship it!

Ship it!
Address clearly uses first lane only, as evident in VPInterleaveGroup::execute(), and the other operands Stored values and Mask clearly use all lanes.
This will be exercised/tested by follow-up patch only?

Ship it!
Would be good to follow-up with cleaning-up of needsScalarIV/needsVectorIV.

Looks good to me, with some last minor nits.
Can drop 'only' also from title.
Specifying VPScalarIVStepsRecipe::onlyFirstLaneUsed() could be committed separately, it belongs to previous patches introducing VPScalarIVStepsRecipe and/or onlyFirstLaneUsed().

Perhaps drop "only" - "usesScalars()" instead of "onlyScalarsUsed()"? As opposed to usesVectors.

This nicely removes from ILV both widenIntOrFpInduction() along with createVectorIntOrFpInductionPHI() which it calls, inlining them into VPlan's VPWidenIntOrFpInductionRecipe::execute(), while postponing buildScalarSteps() for all potential scalar users of the vectorized IV to a VPlan2VPlan transformation/optimization.

How about VPScalarIVStepsRecipe, VPBlendRecipe, VPBranchOnMaskRecipe, VPPredInstPHIRecipe, VPExpandSCEVRecipe?

In D118642#3361875, @fhahn wrote:

In D118642#3361598, @dyung wrote:

@fhahn This change seems to be hitting an assertion failure in one of our internal tests. I have filed GHI#54223 with a repro. Can you take a look?

Thanks, that should be fixed by de8ac485e5b7

The issue was that the order in the map vector wasn't correct. That has been fixed by remove the entry before re-sinking. This ensures it will be sunk after all earlier instructions have been sunk.

Looks good to me, with a last nit.

Looks good to me, thanks!

Looks good to me, thanks!

minor nits

This is fine, couple of final nits; thanks for accommodating!

This is fine, with a last nit

This is fine, remove EntryVal from buildScalarSteps() and update comment?

In D117140#3253999, @fhahn wrote:

Remove unnecessary early exit.

In D117140#3251403, @Ayal wrote:

In D117140#3251026, @fhahn wrote:

In D117140#3250984, @Ayal wrote:

This is fine, thanks for accommodating, one last thought: does Canonical mean starting at zero, or possibly at EPResumeVal?

I think checking for zero a the moment should suffice, as EPResumeVal will only be set as start value just before epilogue VPlan execution. This should also match the current behavior before the change (and D117551 which now contains the addition if isCanonical). Once a single plan contains both vector loops we might need some changes in that direction though.

Resetting start of canonicalIV to EPResumeVal just before epiloque VPlan execution might invalidate passes that relied on this value being zero, including removeRedundantCanonicalIVs()? A test would be good, either confirming or reassuring :-)

Added an assert in 165e36bf180e.

In D117140#3252232, @fhahn wrote:

Address latest comments, thanks!

Resetting start of canonicalIV to EPResumeVal just before epiloque VPlan execution might invalidate passes that relied on this value being zero, including removeRedundantCanonicalIVs()? A test would be good, either confirming or reassuring :-)

I am not sure if it is possible to add a test that has epilogue vectorization and a VPWidenCanonicalIVRecipe. We only add VPWidenCanonicalIVRecipe when folding the tail. In that case, epilogue vectorization is disabled as there's no epilogue. But we should be able to add a relevant test for the stepvector/build-scalar-steps patches I think.

This looks fine, with a minor comment.

In D117140#3251026, @fhahn wrote:

In D117140#3250984, @Ayal wrote:

This is fine, thanks for accommodating, one last thought: does Canonical mean starting at zero, or possibly at EPResumeVal?

I think checking for zero a the moment should suffice, as EPResumeVal will only be set as start value just before epilogue VPlan execution. This should also match the current behavior before the change (and D117551 which now contains the addition if isCanonical). Once a single plan contains both vector loops we might need some changes in that direction though.