Patch updated:

1. Added TODO comment about handling non-zero extract index.
2. Reformatted switch.

In D47735#1406249, @kparzysz wrote:

Are you opposed to having this done in the DAG combiner?

In D47735#1404861, @kparzysz wrote:

One goal was to be able to generate rol-and-accumulate instruction (on Hexagon), specifically for the accumulate operation being | (see f11 in rotate.ll). For the C code we still don't generate it:

unsigned blah(unsigned s, unsigned x) {
  return s | (x << 27) | (x >> 5);
}

Ping.

In D58412#1403230, @craig.topper wrote:

In D58412#1403190, @spatel wrote:

Should we change the generic model too then since most CPUs prefer inc/dec?
I'm guessing that's a lot more test diffs, so no problem to make it another patch, but just want to know if that's the way to think about the generic model.

Related question to that. Should "pentium4" which is clang's Linux default CPU for 32-bit builds have more tuning flags on it?

Should we change the generic model too then since most CPUs prefer inc/dec?
I'm guessing that's a lot more test diffs, so no problem to make it another patch, but just want to know if that's the way to think about the generic model.

Abandoning. Added a sentence to the LangRef here:
rL354394

In D58359#1402730, @efriedma wrote:

Yes, we fold loads from alloca to undef. (In GVN like you mention, but also in mem2reg.) LangRef should state memory allocated with alloca is uninitialized, and that loading from uninitialized memory produces undef; if either of those is missing, patch welcome to fix it.

That said, this seems like the wrong direction, in terms of where we want to perform this sort of optimization. Most functions have more than one basic block, and you won't catch any of those cases. EarlyCSE/GVN/NewGVN should be able to handle this case, and similar cases where there's more than one basic block. Also, FindAvailablePtrLoadStore is basically only used from two places: JumpThreading, and InstCombine. InstCombine really shouldn't be doing this sort of scan, and JumpThreading obviously only triggers in functions with more than one BB.

Thanks, Peter. 'HasFastVariableShuffle' is a heuristic, so we're never going to get it right all the time, but it sounds like we should leave this particular bit of logic as-is and try to chisel out smaller patterns/transforms for sure wins.

Ping.

LGTM

The change looks ok, but can you describe here or point to a discussion about how this fits in the overall plan for overflow op codegen? Ie, what exactly is enabled by moving this code?

Independent of this patch, but just so everyone's aware - there's currently no consistency in the way we transform to the overflow intrinsics. We may transform to sadd.with.overflow as an IR canonicalization (no target checks):
https://godbolt.org/z/2ajU23

Patch updated:
Restrict the change to targets without fast-variable-shuffle.

Better to stage this in 2 parts?
I can add a check for fast-variable-shuffle, so we get the clear improvements. Then, a follow-up can remove that check and see if that results in any real-world fallout.

Patch updated:
Add llvm_unreachable so we don't accidentally return without a value.

@jonpa - IIRC, some targets infinite loop if we always allow the generic trunc fold in DAGCombiner after legalization, but you might be able to constrain it using the existing TLI hooks:
IsDesirableToPromoteOp()
isTypeDesirableForOp()

Can you describe what's happening within SimplifySetCC for the affected test?

Adding more potential reviewers for legalization changes. I'm not sure how much combining/optimization we want to do in here.

LGTM - have you requested commit access?

Some IR tests added here:
rL353992
I think that's better than IRBuilder tests. Feel free to add other predicates if you think they are needed, but I think those 4 give us good coverage.

LGTM

This took some work, but I have an IR test case that should show the bug:
rL353883