LGTM! Thanks, Francesco!

Or are you saying this needs to be done explicitly for stress testing?

In D59952#1447695, @fpetrogalli wrote:

In D59952#1447146, @dcaballe wrote:

Should we add a small lit test that covers the new expected behavior? Probably reusing one from D57598 with the stress testing flag would suffice.

Good idea. Something that invokes stress testing and sets VF = 4 when the computed VF is 1. Did I get it right?

Should we add a small lit test that covers the new expected behavior? Probably reusing one from D57598 with the stress testing flag would suffice.

I thought the point was to just have a VF to build a VPlan with and as it currently stands we build the same VPlans with VF = 4 or any other VF I think. With the programmatically determined VF, we would be a little closer to reality in the stress testing. As Hideki mentioned, having a constant factor for stress testing might have other benefits later on though

Yep, I agree on that we should keep the stress testing mechanism. It will be very useful to make sure that the construction and predication (and maybe other transformation) are robust enough since we can run it on loop nests that are not necessarily vectorizable.
Something important, though, is that we shouldn't use this mechanism to bypass legality or pragma simd requirements to vectorize a loop, i.e., we shouldn't use it to generate actual vector code.
What are you trying to achieve, Francesco?

Thanks, Francesco. LGTM!

LGTM (as you could see in my previous message :))

Thanks Francesco for helping us remove some of the constraints we have in VPlan native path!
I agree with the idea of using getSmallestAndWidestTypes() as a starting point whereas we don't have the proper cost model.

Hey Florian! Thanks a lot for taking care of this! I'm leaving a comment below. Please, let me know what you think.

Hi Florian,

From the VPlan point of view, LGTM. I don't have any other comments.

LGTM!

In case you missed it, there is some discussion in D50480 regarding this code. Your feedback would be appreciated.

Reverted to use the original ICmpULE extended opcode instead of detached ICmpInst. This can be revised quite easily once VPInstructions acquire any other form of modeling compares.

Thanks, Ayal! Some comments below.
Do you see any potential issue that could make modeling this in the VPlan native path complicated once we have predication?

Just a few comments.

Thanks for working on this, Florian, and sorry for my delayed response. I added some initial comments. I'll come back soon.

Thanks, Florian! I'll go ahead with the commit!

Thanks for the review, Florian!
I'll commit this once D48815 is approved and committed.

Making VPLoopInfo object a member of VPlan.

Reverting GraphTraits changes and adding VPlanTestBase as friend of VPlanHCFGBuilder.

Thanks for the comments, Florian! Addressing your comments.

Rebasing this patch on top of D49032

Rebasing this patch on top of D49032.

Rebasing this patch on top of D49032.

Addressing Chandler's comments. Thanks, Chandler!
Yes, I have commit access.

Good idea! Thanks! LGTM. Just some minor comments.

Thanks, Florian! I'll commit next week.

Thanks a lot, Florian! LGTM!

Hi Robert,

If no more comments, I'll proceed with the commit.

Thanks for the rework and for your patience, Florian!
I don't see any major issues. Please, wait for Satish's approval, just in case he has any comments regarding how this would interact with his next patch.
In any case, we could always address any issues in a separate commit.

LGTM!

Thanks for the patch! LGTM.
I think it's ok to kept these utilities in the VPRecipeBase class for now. Eventually, if we add too many, we may want to move them to a VPInstructionUtils, same as what we are doing for VPBlockBase.

Thanks for the comments, Ayal! Please, let me know if you have any other concerns.

Thank you both for the review!

In D46827#1123771, @fhahn wrote:

Updated to not use VPRecipeBuilder for recipe construction. It now relies relies on the original VPlan to be vectorizable without masking/interleaving. If that makes sense, I'll move the code enabling the CG to a separate patch and add a unit test for the transformation.

In D47477#1123833, @fhahn wrote:

In D47477#1119199, @dcaballe wrote:

Thanks, Florian! Again, wait for the pending review to commit this one.

as I updated D46827, do you think there is anything left that's pending for this patch?

In D47595#1123827, @fhahn wrote:

In D47595#1119195, @dcaballe wrote:

Thanks, Florian! LGTM!
Please wait until we finish the other two related reviews to commit it, in case we have to change something else.

As I updated D46827 to not rely on VPRecipeBuilder, but instead create a limited subset of recipes directly, do you think this is good to go?

Thanks for this new version, Florian! Pretty excited to see that some inner loop tests are passing!
Satish is working on the support for outer loops in CG and we’ve been discussing your changes. We have to make sure that this patch aligns well with his next patch and also with the constraints that we stablished for this patch series and subsequent ones. Some comments in this regard:

1. We think that VPInstructionsToVPRecipes is still too smart for the vplan native path and bringing too much code (from CM and Legal, for example) an recipes that we would prefer to keep away from the native path until they are properly ported. Otherwise, we’d end up having the same development limitations as we have in the inner loop path. What we had in main for the Recipe to VPInstruction transition in the native path is something as simple as the code in https://reviews.llvm.org/D44338?vs=on&id=140081&whitespace=ignore-most, where createRecipesForVPBB is naively creating only VPWidenMemoryInstructionRecipe, VPWidenRecipe and VPWidenPHIRecipe. Satish will give you more details in this regard so that this patch aligns with his.

Thanks, Florian! Again, wait for the pending review to commit this one.

Thanks, Florian! LGTM!
Please wait until we finish the other two related reviews to commit it, in case we have to change something else.

Thanks for doing this refactoring! In general, moving all this code outside of the planner makes sense to me.
create*Mask and getDecisionAndClampRange are not strictly creating recipes but I understand the dependencies.
Minor comments inline.

Thanks for revisiting this, Florian!
No major issues. Just one question below.

For example, uniform control flow check @dcaballe has written is probably shareable. Ditto for Divergence Analysis RFC by Simon Moll.

Thanks for the patch, Florian! Ok, now I understand what you mean exactly. Some comments inline.

Hi Florian!

Thanks for the patch, Andrei!

Much better with this VPlanHCFGTransforms! Thanks a lot!
Just some minor comments. LGTM. Again, let's wait for D46827 to see if this code is still necessary.