his block has live-ins; I just didn't copy-paste them in because listing them didn't seem useful.

Thanks for looking again!
Good catch, feedback addressed.

LGTM

LGTM

Thanks, and will fix your suggestions before committing.

Thanks for fixing.

Since this is user documentation, we should only add it here once it is true.

thanks for the suggestions; comments addressed.

Sorry, should have done my std::string, StringRef, and Twine homework a lot better!

Thanks for your help! And I will wait a few days.

thanks @Meinersbur and @fhahn for the discussions on this, also on the dev list.
It looks like we are happy with this behaviour?

this is not important at all, but might be nice clean up, so friendly ping :-)

Ah sorry, ignore me! I messed that up.

In D65884#1621209, @SjoerdMeijer wrote:

Thinking out loud what the strategy could be so that we properly test the whole flow.....

I think it will be okay to land this when we can, it's disabled and the vectorizer won't be generating suitable input anyway. The finalisation can happen last and with no real rush - isel should still generate a valid > loop, the vctp instructions will be more efficient that doing the arithmetic with vectors in the loop. So really, isel for masked load/stores and the vctp is the next most important bit because then we at least run llc. > Then we can prod the vectorizer into generating masked loops and we can find all the bugs :)

OK. It's time. Lets do this.

The overall plan is as follows:

• Enable vectorizer.
• Teach the vectorizer to produce a predicated loop, using a pragma and a target profitability hook. Think we also need to specify that we support masked load/store intrinsics too.
• This pass converts some vector icmps to vctp intrinsics.
• A bit of isel for vctp and final bits for other MVE instructions.
• The loop is finalised in the LowOverheadLoop pass where we check for validity and remove the VCTP and VPST instructions, as well as their VPT blocks.
• Beer.

Hi Sam, nice one!

I do not know whether/how "setting a transformation option implicitly enables the transformation" should be implemented, maybe we should discuss this.

Hi Florian, thanks for your input!

Thanks for pointing this all out!

Forgot that this requires a doc change too. Will add that once we're happy with the proposed behaviour.

Okay, cheers, looks reasonable to me.

Looks reasonable

Still think regressions tests are missing, e.g. for code-size (see a previous comments).

Fixed the string problems.

Many thanks for all your help and reviews!

Sorry for asking, @hsaito , but I was just wondering and wanted to check if your last comment was in fact a LGTM with a minor nit.

Thanks for taking another look!

Friendly ping :-)

Looks like a straightforward fix to me

ran clang-format.

LGTM

Yep, sounds good, let's go for it.

I've moved the test case to the X86 subfolder and removed the ASSERT.

Looks like a sensible fix to me, just one question/nit inline.

I think I've addressed all comments, the main ones are:

• I've moved the loop hint handling to LoopVectorizationLegality.cpp
• I've renamed ScalarEpilogueNotNeededPredicatePragma
• and finally created a helper function to avoid some code duplication that has been bothering me for a while

Thanks for taking a look at this!

We probably need to discuss whether vectorize_predicate(enable) should (or should not) implicitly turns on vectorize(enable) or not. I guess the current behavior is "does not", right? We don't have to discuss that in this review, but we still want to make a conscious decision one way or the other, or did I miss that discussion?

Many thanks for reviewing!

Many thanks for reviewing, very much appreciated!

Ha, that's funny, because before noticing these comments here, I was just doing a test commit (366904) with the github monorepo workflow.
With the discussion on the dev list that the transition date is near, and just following the public documentation in https://llvm.org/docs/GettingStarted.html, it really looks like I committed to the clang and llvm repo at the same time using the git llvm push script from a local git monorepo. I was of course aware of separating clang and llvm patches, but again, thought that this new workflow is fully accepted/supported.

Hi Michael, thanks for taking a look again!

Looks like a good fix to me

Apologies for the early ping! Bu I'm off next weeks, so it would be nice to get this in before that if there are no further comments.
Tomorrow, I will upload another diff that builds on top D64916, which enables code-generation with tail loops folded, thus also demonstrating an end to end test.

Looks reasonable to me.

looks good to me to, just some nits inline.

This all looks very reasonable to me. I am happy with this if @samparker is happy too.

Just a first scan, I will look more closely tomorrow.

More doc changes added to AttrDocs.td and LangRef.rst

Thanks for the suggestion: I have generalized isScalarEpilogueAllowed by introducing an enum to model different reasons why it might be allowed or not.

Is it intentional that this review has no reviewers listed (like, is this a work in progress you don't expect review on yet)?

LGTM

• Moved the codegen test to a separate file
• Added a langref description for this new metadata node.

looks okay to me.

LGTM

Looks like a good optimisation to me.

looks reasonable

Thanks for fixing this.

Removed the separate function that created the loop.llvm.vectorize.predicate metadata. This is now just part of function createLoopVectorizeMetadata, that creates all other vectorize metadata.

Feedback addressed. Thanks for taking a look!

Hi Michael, thanks for taking a look again!

I agree with this: