[RISCV] Don't include X1 in the X0_PD register pair

[asb]

Summary

Zdinx on RV32 defines the D instructions as taking even register pairs, and specifies that if using X0 when as a destination then X1 won't be written, and if using X0 as a source then the value is still all 0s (i.e. X1 isn't read). Therefore, it's incorrect to model X0_PD as having X1 as a subregister. This will also be the case for register pairs in Zacas and the P extension (and this patch takes the same approach as D95588 does).

This patch introduces a dummy register that is solely used as a subreg alongside X0 in X0_PD. An earlier version of the patch had a minor effect on register allocation in some tests, which is now avoided by:

1. Adding RISCV::DUMMY_REG_PAIR_WITH_X0 to RISCVRegisterInfo::getReservedRegs
2. Defining a new register class that includes DUMMY_REG_PAIR_WITH_X0

[craig.topper]

I don't have any better ideas. The test change is kind of weird.

[asb]

In D153974#4461536, @craig.topper wrote:

I don't have any better ideas. The test change is kind of weird.

From tracing through, I think (at least from looking for the likely divergence in a -debug trace) the issue is a change in the regPressureSetLimit for GPRs (going from 28 to 29). The PGPR regpressure limit goes from 14 to 13 and GPRC_with_PGPR changes name to PGPR_with_GPRC and goes from 20 to 19.

@MatzeB does this sound like a possible bug in the register pressure code (or indeed, us doing something wrong), or is this just expected change when altering the register set?

[craig.topper]

Do we need to reserve the new register in RISCVRegisterInfo::getReservedRegs?

[asb]

In D153974#4493649, @craig.topper wrote:

Do we need to reserve the new register in RISCVRegisterInfo::getReservedRegs?

Good suggestion. We might need to, but unfortunately that has no impact on the changed test case.

[craig.topper]

In D153974#4494029, @asb wrote:

In D153974#4493649, @craig.topper wrote:

Do we need to reserve the new register in RISCVRegisterInfo::getReservedRegs?

Good suggestion. We might need to, but unfortunately that has no impact on the changed test case.

Marking the register reserved and adding this to RISCVRegisterInfo.td recovers the tests.

def GPRAll : GPRRegisterClass<(add GPR, DUMMY_REG_PAIR_WITH_X0)>;

[asb]

Updated to incorporate suggested fixes from @craig.topper that means unwanted codegen changes are avoided.

[craig.topper]

LGTM

llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp

BitVector RISCVRegisterInfo::getReservedRegs(const MachineFunction &MF) const {
   markSuperRegs(Reserved, RISCV::X4); // tp
   if (TFI->hasFP(MF))
     markSuperRegs(Reserved, RISCV::X8); // fp
   // Reserve the base register if we need to realign the stack and allocate
   // variable-sized objects at runtime.
   if (TFI->hasBP(MF))
     markSuperRegs(Reserved, RISCVABI::getBPReg()); // bp
 
+  // Additionally reserve dummy register used to form the register pair
+  // beginning with 'x0' for instructions that take register pairs.
+  markSuperRegs(Reserved, RISCV::DUMMY_REG_PAIR_WITH_X0);
+
   // V registers for code generation. We handle them manually.
   markSuperRegs(Reserved, RISCV::VL);
   markSuperRegs(Reserved, RISCV::VTYPE);
   markSuperRegs(Reserved, RISCV::VXSAT);
   markSuperRegs(Reserved, RISCV::VXRM);
   markSuperRegs(Reserved, RISCV::VLENB); // vlenb (constant)
 
   // Floating point environment registers.

llvm/lib/Target/RISCV/RISCVRegisterInfo.td

def VMV0 : RegisterClass<"RISCV", VMaskVTs, 64, (add V0)> {
   let Size = 64;
 }
 
 let RegInfos = XLenRI in {
 def GPRF16  : RegisterClass<"RISCV", [f16], 16, (add GPR)>;
 def GPRF32  : RegisterClass<"RISCV", [f32], 32, (add GPR)>;
 } // RegInfos = XLenRI
 
+// Dummy zero register for use in the register pair containing X0 (as X1 is
+// not read to or written when the X0 register pair is used).
+def DUMMY_REG_PAIR_WITH_X0 : RISCVReg<0, "0">;
+
+// Must add DUMMY_REG_PAIR_WITH_X0 to a separate register class to prevent the
+// register's existence from changing codegen (due to the regPressureSetLimit
+// for the GPR register class being altered).
+def GPRAll : GPRRegisterClass<(add GPR, DUMMY_REG_PAIR_WITH_X0)>;
+
 let RegAltNameIndices = [ABIRegAltName] in {
-  foreach I = 0-15 in {
+  def X0_PD : RISCVRegWithSubRegs<0, X0.AsmName,
+                                     [X0, DUMMY_REG_PAIR_WITH_X0],
+                                     X0.AltNames> {
+    let SubRegIndices = [sub_32, sub_32_hi];
+    let CoveredBySubRegs = 1;
+  }
+  foreach I = 1-15 in {
     defvar Index = !shl(I, 1);
     defvar Reg = !cast<Register>("X"#Index);
     defvar RegP1 = !cast<Register>("X"#!add(Index,1));
     def X#Index#_PD : RISCVRegWithSubRegs<Index, Reg.AsmName,
                                           [Reg, RegP1],
                                           Reg.AltNames> {
       let SubRegIndices = [sub_32, sub_32_hi];
       let CoveredBySubRegs = 1;