Unrelated diffs got uploaded?

Reverted at rL349056 because:

Sounds like everyone is in agreement about the overall direction. There are just a few inline comments/questions to answer, and then we should be good to go.

Patch updated:
Added check and test for non-power-of-2 type.

Patch updated:

1. Add hasOneUse() checks and test, so we're not converting to funnel shift if there's potential for perf regression.
2. I went ahead and committed the PhaseOrdering test since it shows a bug independent of this patch (rL348980).

LGTM

Unsetting 'Approved' until we have a fix for the bug.

Reopening - this patch was reverted at rL348844 because it broke an ARM test.

Patch updated:
Add tests for vsetcc simplification edge cases. These are currently folded early in generic DAGCombiner via SimplifySetCC().

Patch updated:
Rebased after cleanup in rL348845 and rL348851. This should be functionally equivalent, but with less cruft. There's a 'TODO' comment for 32-bit target based on the comments here.

Patch updated:
Fix code comment to better explain what we're doing.

In D55515#1326183, @nikic wrote:

Are there any tests for the 0/max cases? (I guess they're probably just constant folded away anyway?)

I don't have a good understanding of the potential edge cases with aggregates, so adding some other potential reviewers.

LGTM - but I think there's a bug in cheapToScalarize() that I didn't notice before. Please put a FIXME note in there.

Patch updated:
Determine op size based on the opcode parameter (no need to change the function signature).

In D55263#1325283, @davezarzycki wrote:

In D55263#1320043, @spatel wrote:

I just looked over the codegen changes so far, but I want to add some more knowledgeable x86 hackers to have a look too. There are 2 concerns:

1. Are there any known uarch problems with overlapping loads?
2. Are there any known uarch problems with unaligned accesses (either scalar or SSE)?
   
   If there's any perf data (either nano-benchmarks or full apps) to support the changes, that would be nice to see. This reminds me of PR33329: https://bugs.llvm.org/show_bug.cgi?id=33329 (can we close that now?)

Let's not lose sight of the big picture here. If uarch problems exist, are they *worse* than the cost of calling memcmp()? In other words, is the likely register spills, function call overhead, and dynamic algorithm selection (given the constantness of the size parameter is lost) worth it?

LGTM - the code change itself is just removing a hack.

(Adding more potential reviewers for MI changes)

In D55182#1316746, @spatel wrote:

2. There should be a test that shows the benefit of using 'isCheapToScalarize()'. IIUC, that would involve a sequence with multiple binops and/or compares between an insertelement and the trailing extract.

In D55263#1320172, @courbet wrote:

The change makes the impacted values 2.5 - 3x as fast.

I just looked over the codegen changes so far, but I want to add some more knowledgeable x86 hackers to have a look too. There are 2 concerns:

1. Are there any known uarch problems with overlapping loads?
2. Are there any known uarch problems with unaligned accesses (either scalar or SSE)?

In D54710#1319436, @aemerson wrote:

This seems to me to be a bug in ProcessImplicitDefs, for some reasoning killing the IMPLICIT_DEF even though there's a subreg use. GlobalISel MIR output looks fine to me.

Ping * 2.

LGTM. We can handle any of the TODO items as follow-ups.

I just see a few inline nits. Adding Fabian in case he has any feedback.

I think this is the right change for IR (I'm trying to do something similar with D50992 but that's been held up while we try to get the backend in shape). .

I've been looking at this:
rL347868
rL347896
rL348088
...and I'm still not exactly sure how we should fix it, but I'm confident that what this patch is proposing is not the right answer. We don't want to create a dummy instruction only to analyze and delete it.
My best guess is that we can handle this case using a small enhancement to ValueTracking plus a minimal change to the callers in InstCombine and/or SimplifyCFG. These are hacks vs. a proper solution that uses a dominator tree and a more significant fix to CVP, but it's what we already do in both InstCombine and SimplifyCFG, so there's really no additional cost for optimizing simple cases like this.