In D66416#1635141, @ABataev wrote:

There is more complex D57059 to support non-power-of-2 vectorization. It should be split into several small patches + it must be very carefully tested, I just don't have time to work on this. I have a patch for this updated to the latest version, it would be good if somebody else could take it, split it, etc.

LGTM

LGTM - in D63233, I added a TODO that I've marked here for reference, but this appears to be a minimal/safe fix.

LGTM

In D66050#1633020, @spatel wrote:

@qiucf - please rebase and update for test changes after rL369106.

@qiucf - please rebase and update for test changes after rL369106.

In D66095#1631547, @ABataev wrote:

In D66095#1631524, @spatel wrote:

In D66095#1631501, @ABataev wrote:

In D66095#1631495, @spatel wrote:

In D66095#1630408, @ABataev wrote:

What about very long vectors that do no fit into single register? Is it cost effective for such vectors too?

We would split the long vectors into values that fit the target registers in the backend. At that point, the target can decide if N vector selects are better or worse than a transfer to scalar compare and branch. As @lebedev.ri mentioned, we're missing that logic in SDAG, but given that this transform produces the better code for the default case, I don't think we need to make this patch dependent on backend fixups.

But it adds extra cost for vectors splitting. Maybe limit the size of the vectors in the patch?

How would we do that? This is canonicalization, so we are not using any target-dependent information here. Presumably, whoever or whatever created the illegal vectors in the first place knows that codegen will have to alter the those ops to create legal code, so that will be handled by a pass that has a cost model.

It means that you can make a transformation that may be less cost-effective than the original code.

In D66095#1631501, @ABataev wrote:

In D66095#1631495, @spatel wrote:

In D66095#1630408, @ABataev wrote:

What about very long vectors that do no fit into single register? Is it cost effective for such vectors too?

We would split the long vectors into values that fit the target registers in the backend. At that point, the target can decide if N vector selects are better or worse than a transfer to scalar compare and branch. As @lebedev.ri mentioned, we're missing that logic in SDAG, but given that this transform produces the better code for the default case, I don't think we need to make this patch dependent on backend fixups.

But it adds extra cost for vectors splitting. Maybe limit the size of the vectors in the patch?

In D66095#1630408, @ABataev wrote:

What about very long vectors that do no fit into single register? Is it cost effective for such vectors too?

Not sure if clang / C source has any impact on their targets, but subscribing @mcberg2017 @escha in case this could make a difference for out-of-tree (GPU) hardware.

Patch updated:

1. Switch flag requirements based on opcode (only FADD handled for now).
2. Added partial reduction tests (rL368913).
3. Added negative test for mismatch on fast-math-flags.

x86 is moving away from reciprocal estimate code because recent hardware implements a real fdiv at about the same speed as reciprocal estimate+refinement. So x86 perf probably isn't that big of a concern, but it would be good to see the regression test diffs (use the auto-generation script to update those files).

In D66095#1627455, @lebedev.ri wrote:

I'm honestly not sure here if i would consider splat-of-i1 or i1 more canonical,
i would kind-of guessed i1 since it is a single bit while vector isn't.

LGTM

LGTM

In D65765#1625870, @lebedev.ri wrote:

Thanks for taking a look.

I'm not sure how to test uge/sge predicates - i can't come up with any pattern that still contains them after -instcombine.
https://godbolt.org/z/7EAcfy
I'm not sure if that is guaranteed (and thus i should just drop that handling), or i'm simply failing at test-case production.

In D65530#1625331, @lebedev.ri wrote:

In D65530#1625321, @spatel wrote:

Make sure I didn't typo this translation, but I think I'm seeing extra instructions for vectors with this transform on x86 and aarch64:
...

Sure, but that is irrelevant. D65765 improves upon both of them:
https://godbolt.org/z/-UIddL

Sorry for the delay in looking at this. What do the motivating examples look like for codegen? Are we getting the optimal codegen for these clamps, or would we better off trying to create min/max and/or saturating intrinsics?

In D65898#1625141, @xbolva00 wrote:

Thanks

LGTM

LGTM

LGTM

I stepped through this example, and wouldInstructionBeTriviallyDead() returns true because the call has neither uses nor side effects. So we mark it as dead, and then it gets deleted. I'm not sure if that explains the other bug reports mentioned in D65336 though.

In D65954#1621623, @mcberg2017 wrote:

We have some stir(some regressions - a few instructions here and there), however the change is overall beneficial for us.

LGTM.

Patch updated:

1. Added documentation comment for vectorizeLoads().
2. Use isSimple() to filter out 'volatile' and other loads that we don't want to alter.
3. Moved vectorization of loads to end of processing per block (not sure if that answers the request for "end of vectorization" though).
4. Removed check for load larger than vector register size (that was an attempt to not create something harmful, but now we are using the cost model).

LGTM

LGTM