LGTM. See inline for a nit.

LGTM - although the test file was not updated here in the uploaded patch?

LGTM. Don't forget to remove the 'TODO' comments from the test file.

For those following the progress for x86, I enabled the smallest sizes with:
https://reviews.llvm.org/rL305802

In D33342#784553, @efriedma wrote:

Sorry about the delayed review.

LGTM.

Ping * 3.

LGTM.

Patch updated (no functional changes):

1. Use APInt::isOneValue().
2. Change comment about potential zero-latency implementation of pcmpeq - although that does seem possible for some uarch to implement if it wanted to. :)

Here's a test loop:

In D34336#783833, @RKSimon wrote:

In D34336#783558, @craig.topper wrote:

vpternlog does not have any idiom recognition.

For pcmpeq I think intel only avoids the dependency but still executes it. What does AMD do?

Confirmed with Agner's docs - Jaguar/Bulldozer/Ryzen all avoid input register dependencies for PCMPEQ/PCMPGT/PSUB/XOR/ANDN simd instructions when the two inputs are the same. They still create+execute uops (unlike move elimination) - but as integer ops can often go down most simd pipes and are low latency they are very unlikely to cause a performance regression.

In D34242#783298, @davide wrote:

If the goal of this is to expose more opportunities for congruent expressions maybe GVN itself should do this canonicalization, maybe? Some algorithms perform, e.g., reassociation to catch more cases.
See, e.g.
http://www.sable.mcgill.ca/~hendren/621/karthikhandouts.pdf

LGTM.

In D34162#782594, @anna wrote:

Should we go ahead with this fix, and come up with cost model for vector constants?

I don't think we even need the cmp:

The code change looks right, but the test is not minimized, and it's also not checking the more complicated case of a phi with >2 incoming values.

Ping.

@a.elovikov - I didn't find your name in bugzilla; you might be interested in:
https://bugs.llvm.org/show_bug.cgi?id=33467

LGTM.

LGTM.

Ping * 3.

In D34244#781501, @craig.topper wrote:

Will do.

Can I change foldOrOfIcmps to take CxtI by reference so we discourage anyone from passing nullptr.

In D34184#781467, @craig.topper wrote:

I think you're right. The 'icmp' and the 'or' pieces aren't checked for single use. In the existing code we do reuse the 'and'. In my new code i did make sure the trunc only had one use so that putting back an 'and' doesn't increase anything. So in the existing code we potentially create an 'or' and possibly a 'shift' and only delete a 'select'. And the really bad case we create an 'xor', an 'or', and a 'shift' while only removing a 'select'. This is kinda weird to handle because we don't always create a shift and don't always create an xor.

Nice! I was wondering if that was missing.

The enhancement looks right, but I'm worried that the existing transform can increase the instruction count if any of the pieces in the pattern have more than one use. Are we missing m_OneUse() in some/all of the matchers?

Adding more potential reviewers.

LGTM. See inline for a possible follow-up and nit.

Functionally, I think this is correct and complete now. See inline for some nits.

If there are no objections, I would add the regression tests with the current output as a preliminary step, so we document the current behavior.

LGTM.

In D34174#779996, @chandlerc wrote:

This makes lots of sense to me. Honestly, fixing the multiplies might be worth it -- if you can benchmark >1% improvement, I might do it. I'm thinking about the architectures where integer multiplies are *very* slow (atom at least, also some ARM chips)

The analysis looks right to me - this won't work for non-power-of-2 widths, and there's nothing restricting the possible widths coming into here AFAICT.

In D33185#779080, @craig.topper wrote:

@spatel Yes he does. He sent me a mail asking for me to commit it for him. Do you want to do it or do you want me to?

Thanks! Do you need someone to commit this for you?

In D33185#778060, @a.elovikov wrote:

In D33185#778032, @spatel wrote:

Sorry for the delay. Can this bug be exposed with an IR transform or a codegen test?

I am not aware of such cases. I've faced that when working on D33186 but later decided to work with fast math only there. But I believe that this is still useful to fix. At least someone else won't be surprised by it.

Sorry for the delay. Can this bug be exposed with an IR transform or a codegen test?

The patch was approved with the requirement to use "getChain()", but IIUC, we can't do that here. Can someone confirm if I understood that correctly?

A couple of notes for reference:

1. There's another potential case for trying harder to recognize a splat mask in PR32007:

https://bugs.llvm.org/show_bug.cgi?id=32007

Ping * 2.

Ping * 2.