I will try to summarize the current changes done in the below C code and also try to answer some of the common questions raised.

ping!

In D88731#2328076, @dmgreen wrote:

The pre-commit check is not being helpful here... :-/

Seems like a useful optimisation to me though.

Can you add test for i16 and i8. As far as I understand they will not fold because we will have legalized types and the return type will not match the vector element type?

Can you also run the update_llc_test_checks.py script on the file and pre-commit the tests, just showing the changes here.

Sure. I have created new patch : https://reviews.llvm.org/D89365

ping!

Adding support for SABD patterns..

Add support for all integer types.

In D88742#2318989, @dmgreen wrote:

Does this still only handle v16i8?

Fixed review comments

In D88742#2316222, @dmgreen wrote:

In D88742#2314735, @mivnay wrote:

What part of sadb are you worried about? I thought they could be treated the same, given you are extended from enough extra bits. But I may be mistaken, they can be somewhat difficult.

I am worried about the semantics of sabd instruction especially about v16i32 (sub(sext(v16i8), sext(v16i8))) to v16i32 sext(sub(v16i8, v16i8)) part. Can this be looked at later once my other patches are through?

The return type of the abd is positive unsigned, so I think if turning it into v16i32 zext(sabd(v16i8, v16i8)) should be valid? But it's worth testing that, I may be mistaken..

Moved back to https://reviews.llvm.org/D88742

In D88897#2316214, @dmgreen wrote:

Right. Because of Global ISel? I don't know that very well and if there's a similar place to do the same thing, converting an intrinsics to the kind of node that would be matched.

My hope was that we would be able to make something that I could later lift to being target-independent and share across AArch64 and MVE.

Replicating the patterns doesn't feel great though, and disabling the test feels a bit of a step backwards. Perhaps make the pattern match either the node or the intrinsic:

def AArch64uabd_n   : SDNode<"AArch64ISD::UABD", SDT_AArch64binvec>;
def AArch64sabd_n   : SDNode<"AArch64ISD::SABD", SDT_AArch64binvec>;
def AArch64uabd     : PatFrags<(ops node:$lhs, node:$rhs),
                                   [(AArch64uabd_n node:$lhs, node:$rhs),
                                    (int_aarch64_neon_uabd node:$lhs, node:$rhs)]>;
def AArch64sabd     : PatFrags<(ops node:$lhs, node:$rhs),
                                   [(AArch64sabd_n node:$lhs, node:$rhs),
                                    (int_aarch64_neon_sabd node:$lhs, node:$rhs)]>;

In D88819#2314545, @fhahn wrote:

In D88819#2311917, @mivnay wrote:

In D88819#2311678, @fhahn wrote:

Did you consider supporting this naturally by just having LV re-visit the newly created remainder loops, i.e. remember the created remainder loops and add them to the top-level worklist https://github.com/llvm/llvm-project/blob/master/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp#L8587 ? We would need to make sure we do not visit them repeatedly, but overall we should be able to achieve the same goal, but without adding extra complexity to the vectorizer.

Blindly calling the vectorizer for the loop again is not optimal. The current change does that but at lower abstraction level. The majority of the changes are about setting the right overall CFG structure. Example: It is unnecessary to execute runtime checks twice, etc. "struct EpilogVectorLoopHelper" is just the carrier of information from the original vector loop generation to the epilog vector loop generation. Also, InnerLoopVectorizer doesn't expose the vector loop CFG structure to it's users. Fixing the CFG structure at the higher abstraction level exposes this class completely.

In D88819#2314457, @bmahjour wrote:

Thanks for working on epilogue vectorization. Incidentally I've also looked into this recently. There has been a long and detailed discussion on the mailing list from back in 2017 about this transformation here http://llvm.1065342.n5.nabble.com/llvm-dev-Proposal-RFC-Epilog-loop-vectorization-td106322.html. Your patch is able to vectorize epilogue loops with fairly small changes to the LV, however the generated CFG is not optimal. For example, while the SCEV and memory checks are not redundantly executed, they are statically duplicated in the code and increase code size unnecessarily. The trip count checks can also be generated in a way that shortens the critical path from the checks to the scalar loop, which is critical for loops that have a small trip count. Based on the followup discussions from the mentioned RFC, the optimal CFG should look more like what I've attached below.

What part of sadb are you worried about? I thought they could be treated the same, given you are extended from enough extra bits. But I may be mistaken, they can be somewhat difficult.

In D88897#2314408, @dmgreen wrote:

There's some code in performExtendCombine from the look of it, still looking at the intrinsics ID's.

This change seems to be generating a different pattern (UABDLv4i32_v2i64 vs UABDLv2i32_v2i64) for one of the test cases in arm64-vabs.ll. I am not sure whether the new pattern generated is correct and better.

In D88742#2312075, @dmgreen wrote:

So this converts v16i32 abs(sub(zext(v16i8), zext(v16i8)))) to v16i32 zext(abd(v16i8, v16i8))? And doesn't start with a trunk? That sounds like it should work OK.

Yes.

In D88819#2311678, @fhahn wrote:

Did you consider supporting this naturally by just having LV re-visit the newly created remainder loops, i.e. remember the created remainder loops and add them to the top-level worklist https://github.com/llvm/llvm-project/blob/master/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp#L8587 ? We would need to make sure we do not visit them repeatedly, but overall we should be able to achieve the same goal, but without adding extra complexity to the vectorizer.

The CFG after the optimization of a typical loop will be as follows:

I can certainly commit this. I just need an author string to attribute it correctly. Is "Vinay Madhusudan <vinay@compilertree.com>" OK for that?

The build failure is unrelated to this patch and happening in other patches as well (Example: https://reviews.llvm.org/D88471). What should I do? Also, I do not have commit access. Can you please help in committing this patch?

Fixed context and review comments

Added support for SDOT

In D88577#2305658, @dmgreen wrote:

Hello

Can you update with full context? -U999999. It makes phabriactor reviews easier to follow.

I had thought about this somewhat in reference to inloop reductions. I had presumed that it would need some form of partial reduction though, as you would want part of the reduction would then happen outside the loop (I think)

Improving codegen on it's own is good, but I'm interested in seeing how this fits with the other patches.

In D88577#2305592, @SjoerdMeijer wrote:

I haven't looked in much detail at this patch, but this looks like some straightforward lowering of llvm.experimental.vector.reduce.add. Absolutely nothing wrong with that, but I am curious who's going to produce this intrinsic? The vectoriser, the matrix pass? In other words, any ideas on the bigger picture?