Trying to avoid regressions with improved undef propagation:
rL343940
rL343942
rL343945
rL344525
rL344528
rL344534

Is the motivating case integer or FP?
I'm asking because we have a canonicalization for integer cmp+sel for the IR in these tests, but we're missing the corresponding FP transform.
If we add the FP canonicalization in IR, would there still be a need for this backend patch? Ie, is something generating this select code in the DAG itself?

I think this is the right way to go. It's a blob of patterns, but they're stamped out in a regular form.
When the logic ops are promoted, we might have to deal with them separately as noted in D51553.

In D52177#1262757, @dmgreen wrote:

Um, in terms of fixes, I guess our alternatives are make this instcombine dependant on the target, or try to undo it somehow in the backend. My understanding is that the first option is not generally done in instcombine. Can someone give me some history there? Was it something that was decided as a rule, or just something that never came up. (I guess also in this case, not supporting this fold because it happens to cause the extra subs in a random test isn't a great reason to not do it).

In D53201#1264513, @craig.topper wrote:

FYI, the post legalize types DAG combine doesn’t run if all the types in the DAG were legal and nothing was changed by legalize types. So gating a combine on LegalTypes can really mean a transform doesn’t run until after vector op legalization(again the DAG combine after this doesn’t run if nothing changed) or LegalizeDAG.

I suspect that none of these changes are actually 'NFC'.
Example:
rL344458

Patch updated:

1. Add "LegalTypes" as the first predicate for trying this transform to make it less likely that any weird types invalidate the later assumptions.
2. Add an assert that the scalar bitwidth is a multiple of the vector element bitwidth (scalar_to_vector size must be a multiple, and bitcast can't change size).
3. Add a test that at least starts with a weird type in IR.

LGTM

LGTM - see inline for a couple of nits.

LGTM

In D53036#1260230, @lebedev.ri wrote:

In D53036#1260222, @dmgreen wrote:

Luckily, it appears that Alive can do countLeadingOnes too (although I don't see it anywhere in the sources I have).
https://rise4fun.com/Alive/O9i

Huh https://github.com/nunoplopes/alive/search?utf8=%E2%9C%93&q=countLeadingOnes&type=
CC @nlopes

Patch updated:
Added an assert to (hopefully) make this transform clearer.

Nit: it would be nicer to check in the PPC tests with baseline asm as a preliminary step. That way, we'd just see the asm diff there too.

LGTM

In D52997#1259162, @dyung wrote:

In D52997#1258717, @spatel wrote:

In D52997#1258666, @andreadb wrote:

I don't see it as a big problem if we start "regressing" this particular case on Jaguar.

Ok - that's not the impression I got reading PR39195. @dyung - how important is this pattern?

I will defer to Andrea's expertise in this area. I was under the impression that generating the horizontal add/sub instructions was preferred for these cases (especially for btver2), but if that is not the case, I can update our test to not expect it to be generated.

In D44548#1259015, @nemanjai wrote:

In D44548#1258889, @spatel wrote:

How about enabling the hook for PPC before adding to the generic DAG combine...
Right now, you'll get something horrible for normal fabs (and fneg?) without this patch, right?

	xscvdpspn 0, 1
	xxsldwi 0, 0, 0, 3
	mfvsrwz 3, 0
	rlwinm 3, 3, 0, 1, 31
	mtvsrd 0, 3
	xxsldwi 0, 0, 0, 1
	xscvspdpn 1, 0

Yes. Although that is the worst case (since PPC f32 isn't 32 bits in a register).
No doubt, the PPC-specific portion of this needs to go in and can certainly go in first. I can commit this as the PPC portion first (with the fnabs test missing). And then the generic part (adding the fnabs test).
The question is do I need to split this into two patches and post for review or can I just commit it as two separate patches (or maybe just commit the PPC-portion and post the generic portion for review)?

How about enabling the hook for PPC before adding to the generic DAG combine...
Right now, you'll get something horrible for normal fabs (and fneg?) without this patch, right?

In D52747#1258801, @RKSimon wrote:

Now that D52964 has landed - is there anything stopping this?

In D52964#1258721, @RKSimon wrote:

LGTM - the AVX1 regression needs SIGN_EXTEND_VECTOR_INREG support adding to SimplifyDemandedBits which I intend to do as a follow up patch

In D52997#1258666, @andreadb wrote:

I don't see it as a big problem if we start "regressing" this particular case on Jaguar.

Patch updated:
Removed unnecessary depth param (copy-paste remnant).

I'm trying to improve vector-demanded-elements folds in D52912 which means we need to do better at recognizing patterns with undefs.

This patch is not 'NFC'. I added a test that should show an improvement at rL343936.

Patch updated:
Fixed whitespace diff.

In D52747#1257240, @RKSimon wrote:

@craig.topper The final codegen from the updated IR in masked_load_store.ll regresses due to masked stores not making use of only requiring the MSB of the mask vector (lots of SIGN_EXTEND_INREG etc.) - X86ISelLowering's combineMaskedStore only handles the PCMPGT case, how tricky would it be to replace it with a general SimplifyDemandedBits calls?

Patch updated - once again, no code changes, but rebased after:
rL343882
...so now we don't have any more sched test diffs.
I was completely missing the point of those tests (to check both reg-reg and reg-mem variants of the opcodes).

Patch updated - no code changes, but:
Rebased after rL343858, so less noise from the SSE1/2 sched tests.