In D63382#1547090, @spatel wrote:

The only problem that I see is that the backend (DAGCombiner) isn't prepared to generically reverse this pattern yet (x86 might have some code that can be lifted):

combineSelectOfTwoConstants seems to be the handler for X86 you mentioned . I like your idea to migrate it to a target-independent combine.

Fix typo in comment pointed out by @xbolva00

Following suggestion by @lebedev.ri to support non-splat vector shift amount

Addressed comments by @lebedev.ri
Thanks!

LGTM. Regarding the SSE2->SSSE3 test change, i think it's fine. Can you update the --check-prefix to SSSE3 in a follow-up commit? I think it's convenient to review as-is, but in the longer term it would be misleading to leave it as-is.

LGTM. Thanks!

Sorry for not being responsive. Won't have time to work on this, so thanks @RKSimon for taking charge.

Is this patch pending for review? If yes, can you please rebase it on ToT?

LGTM

LGTM

Minor improvement: No need for the select if there are no negatives

• Rebase
• Simplify the creation of the shuffle by only modifying the widened mask to take all zeros from the all-zero operand.

• Remove deprecated comment
• Fix variable names
• N1C can be passed to matchUnaryPredicate also when the divisor is a constant-splat. Makes check more efficient as we avoid re-checking the splatted element for every vector element. (why doesn't matchUnaryPredicate check for splat and do the same?)

In D42479#1011497, @RKSimon wrote:

I still think it'd be better if you treated splat vectors as a vector instead of a scalar - your change to matchUnaryPredicate means that we're accepting UNDEF elements where we weren't before, which for DIV/REM opcodes is supposed to be a big no-no.

I think that at the point this combine runs there should not be any undef elements because there is an earlier combine that handles division by undef (or vector with any undef elements)., see SelectionDAG::isUndef.
Having said that, there is room for improvement by visiting only the source splatted value
instead of every BUILD_VECTOR operand. Will upload an improved patch.

The previous revision was a rebase on a dated revision. This is a rebase on the latest.

Rebase

I apologize about the delay. Got some other priority work. Will make an effort to continue this week. If not, please go ahead and complete the work, Simon.

LGTM

Addressing Simon's comments

LGTM after fixing the signed/unsigned mismatches.

1. matchBinaryPredicate -> matchUnryPredicate
2. Use Simon's uniform scalar/vector code suggestion for computing INEXACT

Rebase + ping

Following Simon's suggestions, dropping the TLI hook seems to improve all cases except for v2i64 on SSE/AVX1.

In D42479#986607, @RKSimon wrote:

How bad does the codegen get if we don't limit this to targets with vector shifts? Again, thinking AVX1 (Jaguar) here., but combine_vec_sdiv_by_pow2b_v4i64 looks like a missed opportunity

I think you are right. Probably all cases will profit except for v2i64. Will try to drop the TLI hook.

LGTM

LGTM with a minor request:

In D42485#986599, @spatel wrote:

LGTM. Just curious - do we have vector intrinsics or any passes that create vector integer division?

Rebase

LGTM

Thanks for the fix.

Add some basic encoding tests?

In D42171#978790, @rnk wrote:

Isn't this MI buggy? We're adjusting SP down by 40 bytes and storing to SP+48, which could overwrite data. I think the assert is valid.

I would appreciate suggestions for alternative solutions.

In D41944#973480, @MatzeB wrote:

• With the way you are modeling the new flag, means that existing bitcode/.ll files will change semantics when read with newer compilers. I'm not sure that is a good idea for this, in any way at the very least you have to provide AutoUpgrade logic for that.

This seems like a real issue. With no version info in the module, how can AutoUpgrade tell if a divide with no 'nof' attribute is of the old form or new form? This is really a performance issue, because AutoUpgrade can always pessimistically not add 'nof' if the version of the incoming module is unknown. Possible solutions:

Generalize to account for commutativity of add and mul

Check both BUILD_VECTOR nodes together if one is composed of odd indexed extracts and the other composed of even idexed extracts.

Rebase

Rebase after adding the missing zext cases

Add asserions for type sizes and fix typo in comment

Reabase on top D41925

There are some occurrences of calls to getMaskedGather in DAGCombine.cpp which i do not see being addressed by this patch. I guess they are not being covered by tests?

Fix issue identified by Simon: use original vector type for the insert_vector

In D41865#972023, @RKSimon wrote:

In D41865#971307, @zvi wrote:

Sure, but looking at your example the return type should have the same number of elements as the indices vector, right?

Yup, sorry for the typo. Are you intending to support cases like this?

Average lowering fully using the refactored type-splitting code.

1. Following Simon's suggestion, refactored out the code that splits the vector to legal-types to 'LowerBinTo' (the function name probably needs revision)) and applied to PMADDWD.
2. Added a missing DAGCombine to let a truncate negate a sext through an EXTRACT_SUBVECTOR.

Fixes for Craig's comments

Added test with source vector larger than indices vector

Sure, but looking at your example the return type should have the same number of elements as the indices vector, right?

LGTM

Rebase + apply fixes for Simon's comments. Will commit this change right away to avoid conflicts.