LGTM.

Looks fine to me. More ideas inline.

Rebase on D89038.

In D89525#2333924, @hliao wrote:

@yaxunl could you double-check that OpenCL also follows that rule.
@nhaehnle could you check whether that potentially breaks graphics.

Add a vector test case for matchFastFloatClamp.

Test simplifySelectWithFCmp on non-uniform vectors.

Address feedback.

But it would be nice to somehow get the codegen improvements shown in some of the MIPS and X86 tests.

Thanks for testing @gargaroff.

Remove peephole

In D89392#2330590, @aditya_nandakumar wrote:

I attempted this in the past but abandoned it due to infinite loops in legalization like mentioned earlier. Constant folding during legalization seems okay as long as it operates on legal types (ie fold an operation of constants (which are already legal) to the same type.

In D89392#2330153, @gargaroff wrote:

Is there anything I need to do to use the CSEMIRBuilder?

In D89392#2330084, @arsenm wrote:

Automatically folding legalization artifacts makes me a bit nervous. What happens if you want to legalize a constant by widening, and the legalizer wants to insert the trunc from a widened constant?

Makes sense to me. As I understand it the biggest practical effect is that the compiler will start making use of output modifiers in compute shaders.

Reverse ping! If you just don't have time to work on this, I'd be interested in commandeering the "insert_vec_elt(build_vector) -> build_vector" part.

Looks good. Some nits inline.

LGTM.

Lowercase subtarget feature names.

In D89247#2325783, @rampitec wrote:

It also seems to lack changes in SIFoldOperands, SIInstrInfo::FoldImmediate, SIInstrInfo::convertToThreeAddress, SIInstrInfo::canShrink. I think these can go in a separate change.

Looks good to me if the target maintainers are happy.

Querying the register bank bank for VS_32 doesn't make any sense, and I suspect would assert.

In D64393#2324102, @vpykhtin wrote:

I've found the case when execMayBeModifiedBeforeAnyUse randomly leads to a coredump, which is hard to debug. Most likely it's because an instruction beyond the end of a basic block is accessed. This means that the first loop calculates some instructions twice and I was wrong assuming use_nodbg_instructions doesn't repeat them. In fact there is no code in MachineRegisterInfo::verifyUseList that ensures that uses belonging to one instruction should be sequent in the use list nor the traces of such ordering can be found in MachineRegisterInfo::addRegOperandToUseList. I'm going to fix this code.

In D89077#2320804, @rampitec wrote:

In D89077#2320691, @t-tye wrote:

In D89077#2320512, @kerbowa wrote:

In D89077#2320507, @t-tye wrote:

Is this now running after the waitcnt insertion pass? That would avoid the NOPs currently being inserted to split memory clauses that are not necessary as the waitcnt instructions will split the clauses.

We also insert nops in the post-RA scheduler.

In earlier conversation it was suggested that the spurious NOPs were explained as happening because the hazard recognizer inserted them to break memory clauses, and then the waitcnt pass ran. There would be no need to insert the NOPs if the waitcnt instructions were already there. So seems that was not a valid explanation, perhaps the post-RA scheduler is an explanation, but I am unclear why it put those ones in. @rampitec can you help explain?

Post-RA scheduler and hazard recognizer is the same pass if you run post-RA scheduler. If not it is a separate pass.

Use matchers to handle fixed width vectors.

It might make sense to rebase this on D89038.

Rebase after D89000.

I'll rebase this after D89000 lands, since it touches some of the same code in VOP2Instructions.td and gfx1030_err.s.

Add some matchers to PatternMatch and use them.

Fix a couple more problems shown up by running the MC tests:

• Update asm/dis special cases for V_MAC_LEGACY_F32 now that it uses the VOP_MAC_F32 profile.
• Fix use of OtherPredicates in VOP2 pseudos.

Rebase.

Update comments for NoMods patterns.

Is "spill" the right name? They come from TargetLowering::expandUnalignedLoad copying the data to a temporary stack slot, not from the register allocator.

Could you pre-commit the test case?

LGTM, though I can't help feeling the whole function could be simpler.

This looks like a clear win to me on all the affected test cases except for one slight regression noted inline.

LGTM apart from the nitpick about comments.

@ecnelises did you really mean to add yourself as a "blocking reviewer" rather than a normal reviewer?