LGTM.

In D94641#2498991, @paquette wrote:

In D94641#2498944, @aemerson wrote:

It it possible to extend the existing combine in prelegalizer combiner to handle this too? Or will that cause us to still miss some cases?

The problem with that combine is it looks for G_FCONSTANTS which are only used by stores, so as is, it would miss cases.

I think that we could probably replace that combine with something similar to this though.

LGTM.

In D94350#2499061, @paquette wrote:

In D94350#2496310, @aemerson wrote:

We also need to check for the strict-align function attribute before we generate wider loads.

Shouldn't the legalizer check alignment requirements on the load? Or is this something different?

Each pass has to respect the semantics though. At the moment we fall back in the translator for strict-align, but when we do add support places like this will need to correct. Unless you add alignment checks for this combine, this might create unaligned wide loads which are illegal with strict-align, since we’re not allowed to assume that unaligned loads are safe.

It it possible to extend the existing combine in prelegalizer combiner to handle this too? Or will that cause us to still miss some cases?

Why is this in lowering instead of combine?

We also need to check for the strict-align function attribute before we generate wider loads.

LGTM.

Fix clang-format warnings.

Improve the performance of the insertion queue flushing.

In D94350#2493991, @paquette wrote:

• Avg regs/missed combine is the number of registers in RegsToVisit when the combine misses

Does this metric count the misses where there were no registers?

In D94350#2493991, @paquette wrote:

Here's some measurements:

                     O0 compile   O0 code       #Missed   #Missed    Avg regs/      | Os compile   Os code      #Missed   #Missed    Avg regs/
                     time change  size change   combines  (> 1 reg)  missed combine | time change  size change  combines  (> 1 reg)  missed combine
bullet               0.25%         0.00%        22        4          0.5            |  0.56%       0.00%        138       14         0.3
kc                   0.05%         0.00%        2         0          0.0            |  0.76%       0.00%        66        0          0.0
lencod              -0.18%         0.00%        89        20         0.6            |  0.31%       0.00%        508       34         0.2
SPASS                0.09%         0.00%        1         1          3.0            |  0.37%       0.00%        146       4          0.0
consumer-typeset    -0.27%         0.00%        369       52         0.3            |  0.17%       0.00%        625       61         0.2
clamscan            -0.09%         0.00%        179       78         1.3            |  0.21%       0.00%        493       49         0.3
sqlite3             -0.48%         0.00%        185       29         0.4            | -0.18%      -0.50%        514       91         0.6
tramp3d-v4           0.14%         0.00%        0         0          0.0            |  0.52%       0.00%        253       2          0.0
pairlocalalign      -0.04%         0.00%        0         0          0.0            |  0.49%       0.00%        54        48         2.7
7zip-benchmark       0.12%        -0.10%        336       273        2.5            |  0.75%      -0.40%        565       268        1.6
                                                                                    |
Average             -0.04%        -0.01%        118.30    45.7       0.87           |  0.40%      -0.09%        336       57         0.6

• Compile time was measured using a single thread with 3 samples. Comparison is between geomeans of the samples.
• # Missed combines is the total times that the combine failed
• # Missed (>1 reg) is the number of times the combine failed with at least one register in RegsToVisit.

Is this meant to say *greater* than 1?

• Avg regs/missed combine is the number of registers in RegsToVisit when the combine misses

Note that a lot of the time, the combine does miss with 0 registers (because you just don't find anything with a single use, for example).

LGTM with the getFPImm nit fixed.

LGTM. Having looked into this recently we should definitely from the MIRBuilder's observer.

Thanks for tackling this. The logic seems sound to me, but I have some efficiency concerns. (Note, this is conjecture, I don't know if this is realistically a big problem)

Is this going to be used anywhere?

We do this condition splitting optimization in GlobalISel too, so that will also need fixing: see IRTranslator.cpp.

This seems to have caused failures on Darwin platforms. I've pushed a fix in 48e7b34f but let me know if you disagree with the approach.

In D93423#2459024, @aditya_nandakumar wrote:

In D93423#2459018, @aemerson wrote:

In D93423#2459004, @aditya_nandakumar wrote:

If slot indexes are preserved and used in other passes that use CSE, do you think the compile time impact is amortized/minimized?

It's possible it would help a bit. I think the localizer may also benefit from this. I don't see it making a big difference either way though in most code.

In our out of tree backend, CSE is used and preserved for 6 passes in total. I suspect if slot indexes are also preserved, the overhead might not be too much. Hence I was curious if you actually collected some numbers. If only legalizer is being looked at for compile time impact, we're not really evaluating the compile time savings to be had in other passes (assuming amortized cost of slot index) right?

In D93423#2459004, @aditya_nandakumar wrote:

If slot indexes are preserved and used in other passes that use CSE, do you think the compile time impact is amortized/minimized?

In D93423#2459003, @aditya_nandakumar wrote:

In D93423#2458972, @arsenm wrote:

I still think the CSE MIR builder shouldn't be required to CSE and can bail early on long dominance queries

I guess the concern is how exactly one defines "early" and "long". Should it be universal? Opt in? Configurable ?.. What if it bailed just a few instructions away from the target instruction. What about the compile time impact of counting and checking in loop in the cases where you don't need and so on.

In D93423#2458972, @arsenm wrote:

I still think the CSE MIR builder shouldn't be required to CSE and can bail early on long dominance queries

Yes, a test would be good.

LGTM.

Can you also check if there's any code size impact at -O0 and -Os. I'm wary of regressing -O0 and leaving extra unnecessary moves around.

As discussed offline, this also needs an implementation for GlobalISel. You can just add check lines to the test with -global-isel -global-isel-abort=1 for the tests.

LGTM.

ping

@simon_tatham Can you take a look?

Add buildShuffleSplat and buildShuffleVector to MachineIRBuilder and use.

LGTM once we use the new matcher.

LGTM.

Maybe use PatternMatch for this too?

LGTM with nit.

Check for SEXT_INREG and use copy instead of modifying operand reg.