Our local testing with this patch shows that it solves our issue. Thanks very much! Looking forward to it landing.

Whoops, I forgot to reference this revision when I committed the patch. This was bccc9aa81c1c1d212acd3314895731ec4de30e35.

Rebased; fixed pasto in getFMACostSavings(); ensured that getFMACostSavings() returns nonnegative values; and made a slight simplification in adjustForFMAs(). Thanks!

This revision now includes fixes for both cases: a horizontal reduction of fadds fed by a vectorized tree rooted at fmuls, and a vectorized tree of fmuls that feeds arbitrary fadds and/or fsubs. It was misguided to break this up into two patches without showing you both...

In D125987#3525451, @RKSimon wrote:

On what target are you seeing this problem?

In D125987#3525414, @fhahn wrote:

I think this loss is not only relevant for horizontal reductions, but can pessimize things in general, right?

You're welcome! Thanks for the good advice!

In D124867#3522822, @ABataev wrote:

LG with some nits

Thanks for the helpful comments to date! In this version, I've managed to remove the undefs from the original test. I also added a second test that removes the loop structure. For both tests, today we will generate an unprofitable horizontal reduction. With the first test, adding cost modeling to constrain the horizontal reduction allows FMAs to be generated. With the second test, this is insufficient, as we then decide to vectorize the multiplies in an unprofitable way. The two tests demonstrate the need to account for lost FMAs in the cost modeling both when vectorizing for a reduction and when vectorizing a list of multiplies.

In D124867#3506308, @ABataev wrote:

In D124867#3506211, @wjschmidt wrote:

In D124867#3506146, @ABataev wrote:

The undefs still here.

Yes, see above -- I was unable to find a sequence without the undefs that causes the horizontal reduction to kick in.

Use -slp-threshold option to avoid problems with the cost.

In D124867#3506304, @ABataev wrote:

In D124867#3506289, @wjschmidt wrote:

In D124867#3506218, @ABataev wrote:

Also, why these sequences are not optimized by InsrtuctionCombiner to FMA?

Phase ordering -- it seems the FMA combining happens quite late in the pipeline. When we replace the adds with a horizontal reduction, the opportunity is removed.

Why, could you investigate it?

I'll have to refresh my memory, but my recollection is that the FMA combining is done in the MI level instruction combiner.

Why? Are there any target-caused limitations?

In D124867#3506218, @ABataev wrote:

Also, why these sequences are not optimized by InsrtuctionCombiner to FMA?

Phase ordering -- it seems the FMA combining happens quite late in the pipeline. When we replace the adds with a horizontal reduction, the opportunity is removed.

Why, could you investigate it?

In D124867#3506146, @ABataev wrote:

The undefs still here.

Hi! I'd like to ping this revision, please.

I've made all requested changes, with the exception that I can't remove the loop structure or any of the undefs without breaking the test. In both cases, we no longer generate the horizontal reduction. I've made all the reductions Alexey requested, and changed the variable names as Vasileios requested.

In D124867#3489175, @ABataev wrote:

Try to reduce test more. I think you can remove attributes, datalayout, pass triple as an argument, remove comments for incoming branches