PPC should use REG_SEQUENCE instead of sequences of INSERT_SUBREG

[arsenm]

The PPC backend is using a sequence of INSERT_SUBREGs to build register tuples, which is harder to deal with in optimizations than REG_SEQUENCE which is intended for this kind of usage.

Attached starter patch. There appear to be other backend bugs resulting in verifier errors, and I'm pretty sure these pseudos should be eliminated too.

0001-PPC-Use-REG_SEQUENCE-instead-of-pairs-of-INSERT_SUBR.patch

[llvmbot]

@llvm/issue-subscribers-backend-powerpc

Author: Matt Arsenault (arsenm)

The PPC backend is using a sequence of INSERT_SUBREGs to build register tuples, which is harder to deal with in optimizations than REG_SEQUENCE which is intended for this kind of usage.

Attached starter patch. There appear to be other backend bugs resulting in verifier errors, and I'm pretty sure these pseudos should be eliminated too.

0001-PPC-Use-REG_SEQUENCE-instead-of-pairs-of-INSERT_SUBR.patch

[nemanjai]

@arsenm I'm not sure that we can accomplish this with REG_SEQUENCE nor what exactly needs to change in order to accomplish this. What seems clear is that the minimal patch probably isn't sufficient as (from what I understand), it causes some invalid allocations of these subregisters.
As to your question about the intermediate pseudo-instructions, I'll try to explain here and maybe that will shed some light on why this patch isn't sufficient.

These registers are for an on-CPU accelerator that does matrix multiplication on Power10 CPUs. And this "MMA engine" has some peculiar restrictions.

1. The registers are 512-bits wide, overlapping 4 128-bit VSR registers
2. The underlying VSR registers need to be consecutive
3. The register 4-tuples have to consist of register numbers that start at a multiple of 4 (i.e. VSR0, VSR8, VSR16, etc.)
4. There are no load/store instructions that store the whole 4-tuple - pair loads/stores must be used (not individual loads/stores)
5. The overlaying of registers requires that the registers be "primed" and "de-primed" which means that a register is only a valid accumulator register after instruction mtacc is executed on it and is no longer a valid accumulator register after instruction mfacc is executed on it
6. No operations can be done on the underlying VSR registers when it's in accumulator mode and no MMA operations can be done on the overlayed accumulator when it is not in accumulator mode

[arsenm]

Points 1-3 are perfect for reg_sequence, this is a matter of defining the register classes and subregister accurately. Point 4 doesn't matter. for copying. Not sure I understand 5-6

[nemanjai]

I'm not sure that 4. doesn't matter. The paired register loads/stores would themselves need to populate a register built up of 2 VSR's by REG_SEQUENCE. I must admit that I only took a quick look at the TwoAddress pass that gets rid of these and I'm not convinced that it will handle a REG_SEQUENCE of REG_SEQUENCE's correctly.

To illustrate 5 and 6:

xxlor %vs0, %vs1, %vs2             # Operation on VSR, valid here
lxvp %vs0, 16(%r1)                 # Load VS0 and VS1
lxvp %vs2, 32(%r1)                 # Load VS2 and VS3
xvi16ger2 %acc0, %vs25, %vs31      # Operation on ACC0, invalid, ACC0 is not primed yet
mtacc %acc0                        # Enable ACC0 ("prime" the register)
xvi16ger2 %acc0, %vs25, %vs31      # Operation on ACC0, valid here
xxlor %vs23, %vs1, %vs2            # Operation on VSR, not valid here because VS1 and VS2 are part of ACC0
mfacc %acc0                        # Disable ACC0 ("de-prime" the register)
# VSR0 - VSR3 can be set or used now

Although I don't work on PPC any longer, I'm sure my former colleagues would be happy to simplify this code so we're certainly grateful for your feedback and your time.

[lei137]

Build with the starter patch seem to produce worst code gen worst with more copies being generated and causes the backend to crash for CodeGen/PowerPC/ppc64-acc-regalloc-bugfix.ll.out.

I dug around a bit and it seems when the REG_SEQUECE is nested like it is for VecsToVecQuad:

def ConcatsMMA {
  dag VecsToVecPair0 = (v256i1 (REG_SEQUENCE VSRpRC, $vs0, sub_vsx1, $vs1, sub_vsx0));
  dag VecsToVecPair1 = (v256i1 (REG_SEQUENCE VSRpRC, $vs2, sub_vsx1, $vs3, sub_vsx0));
  dag VecsToVecQuad = (BUILD_UACC
          (v512i1 (REG_SEQUENCE UACCRC,
                                (KILL_PAIR VecsToVecPair0), sub_pair0,
                                (KILL_PAIR VecsToVecPair1), sub_pair1)));
}

It causes the register allocator to not allocate the expected consecutive register pairs for the ACC/UACC types.
I am not sure we can remove the pseudos BUILD_UACC since this is how we "de-prime" the "primed" acc register.

Looking at TwoAddressInstructionImpl::eliminateRegSequence(), it doesn't look like this will handle nested usages of REG_SEQUENCE.

Changing the patch so that VecsToVecQuad is a REG_SEQUENCE of VecsToVecPair created via INSERT_SUBREG doesn't have this issue (#129941).