Add all constant physical registers to callee preserved masks

[arichardson]

Summary

This allows MachineCopyPropagation to eliminate copies of constant registers
such as zero registers. They were previously not being eliminated as the
check for MO.clobbersPhysReg(AvailSrc) would return true for constant
registers such as MIPS $zero.

To avoid having to manually add the zero registers to all CalleeSavedRegs
instantiations in tablegen, I instead added a new isConstant bit to the
Register and set this for MIPS, RISC-V, and AArch64 zero registers.
RegisterInfoEmitter.cpp looks at this flag and adds all constant registers
to the preserved register mask.

This may also benefit other passes but so far I have only seen differences
in MachineCopyPropagation. In the future it might make sense to generate
isConstantPhysReg() from this information.

Original source: https://github.com/CTSRD-CHERI/llvm-project/commit/8588d8b81458ed6d87b674893e7752e6a6915574

[luismarques]

llvm/test/CodeGen/Mips/avoid-zero-copy.mir

12–13

Redundant move

[arichardson]

Add constant registers for the remaining architectures

[arichardson]

llvm/test/CodeGen/Mips/avoid-zero-copy.mir

12–13

The previous one is in the delay-slot, so it's actually setting the argument register a0.

[luismarques]

Makes sense to me but somebody else should give it a more thorough review.

To avoid having to manually add the zero registers to all CalleeSavedRegs instantiations in tablegen,

It seems wise to avoid that approach, even if that also would have worked.

llvm/test/CodeGen/Mips/avoid-zero-copy.mir

12–13

Oops, ignore this.

[arichardson]

Add missing RISCV::VLENB

[jrtc27]

llvm/include/llvm/Target/Target.td

192

One could follow up with a way to TableGen the implementation of isConstantPhysReg

llvm/lib/Target/AMDGPU/SIRegisterInfo.td

240 ↗

(On Diff #452969)

let ... in would avoid the need to mess with the multiclasses but that's a matter of taste for AMDGPU maintainers

llvm/lib/Target/RISCV/RISCVRegisterInfo.td

80

I don't know about other architectures, but RISCV style would be to use let ... in (like CostPerUse below). It'd be good to make sure your change it style-consistent for each backend.

llvm/utils/TableGen/CodeGenRegisters.cpp

159

I would expect Constant and Artificial to be handled the same way but I don't know why this is here...

llvm/utils/TableGen/RegisterInfoEmitter.cpp

1616

New line before the comment like above

[luismarques]

In D131958#3725972, @arichardson wrote:

Add missing RISCV::VLENB

Is this actually useful in the context of CSRs?

[arichardson]

llvm/include/llvm/Target/Target.td

192

D131962

[arichardson]

In D131958#3726007, @luismarques wrote:

In D131958#3725972, @arichardson wrote:

Add missing RISCV::VLENB

Is this actually useful in the context of CSRs?

There are no test diffs, but it is listed by ::isConstantPhysReg, so I added it here as well.

[arichardson]

Will move the tablegen changes into the commit that generates isConstantPhysReg() so that it's easier to verify that none were missed.

[arichardson]

rebase on top of D131962

[arichardson]

fix coding style

[arichardson]

llvm/test/CodeGen/RISCV/double-convert.ll

632

Still got a load of zero into a saved register here, not quite sure why. blt s2, s4, .LBB12_4 should be able to use the zero register.

[asb]

llvm/test/CodeGen/RISCV/double-convert.ll

632

Yes, there's something weird going on in general with selection of the zero register. See D130809 and the bug tracking this issue. I'm not sure if it's a general problem with register coalescer or if there's some hook that's not set properly somewhere.

[arsenm]

LGTM although this does feel a bit magical

llvm/include/llvm/Target/Target.td

class Register<string n, list<string> altNames = []> {
   // x86 register AX is covered by its sub-registers AL and AH, but EAX is not
   // covered by its sub-register AX.
   bit CoveredBySubRegs = false;
 
   // HWEncoding - The target specific hardware encoding for this register.
   bits<16> HWEncoding = 0;
 
   bit isArtificial = false;
+
+  // isConstant - This register always holds a constant value (e.g. the zero
+  // register in architectures such as MIPS)
+  bit isConstant = false;
 }
 
 // RegisterWithSubRegs - This can be used to define instances of Register which
 // need to specify sub-registers.
 // List "subregs" specifies which registers are sub-registers to this one. This
 // is used to populate the SubRegs and AliasSet fields of TargetRegisterDesc.
 // This allows the code generator to be careful not to put two values with
 // overlapping live ranges into registers which alias.

llvm/lib/Target/AArch64/AArch64RegisterInfo.td

 def W24   : AArch64Reg<24, "w24">, DwarfRegNum<[24]>;
 def W25   : AArch64Reg<25, "w25">, DwarfRegNum<[25]>;
 def W26   : AArch64Reg<26, "w26">, DwarfRegNum<[26]>;
 def W27   : AArch64Reg<27, "w27">, DwarfRegNum<[27]>;
 def W28   : AArch64Reg<28, "w28">, DwarfRegNum<[28]>;
 def W29   : AArch64Reg<29, "w29">, DwarfRegNum<[29]>;
 def W30   : AArch64Reg<30, "w30">, DwarfRegNum<[30]>;
 def WSP   : AArch64Reg<31, "wsp">, DwarfRegNum<[31]>;
-def WZR   : AArch64Reg<31, "wzr">, DwarfRegAlias<WSP>;
+def WZR   : AArch64Reg<31, "wzr">, DwarfRegAlias<WSP> { let isConstant = true; }
 
 let SubRegIndices = [sub_32] in {
 def X0    : AArch64Reg<0,   "x0",  [W0]>, DwarfRegAlias<W0>;
 def X1    : AArch64Reg<1,   "x1",  [W1]>, DwarfRegAlias<W1>;
 def X2    : AArch64Reg<2,   "x2",  [W2]>, DwarfRegAlias<W2>;
 def X3    : AArch64Reg<3,   "x3",  [W3]>, DwarfRegAlias<W3>;
 def X4    : AArch64Reg<4,   "x4",  [W4]>, DwarfRegAlias<W4>;
 def X5    : AArch64Reg<5,   "x5",  [W5]>, DwarfRegAlias<W5>;

 def X24   : AArch64Reg<24, "x24", [W24]>, DwarfRegAlias<W24>;
 def X25   : AArch64Reg<25, "x25", [W25]>, DwarfRegAlias<W25>;
 def X26   : AArch64Reg<26, "x26", [W26]>, DwarfRegAlias<W26>;
 def X27   : AArch64Reg<27, "x27", [W27]>, DwarfRegAlias<W27>;
 def X28   : AArch64Reg<28, "x28", [W28]>, DwarfRegAlias<W28>;
 def FP    : AArch64Reg<29, "x29", [W29]>, DwarfRegAlias<W29>;
 def LR    : AArch64Reg<30, "x30", [W30]>, DwarfRegAlias<W30>;
 def SP    : AArch64Reg<31, "sp",  [WSP]>, DwarfRegAlias<WSP>;
-def XZR   : AArch64Reg<31, "xzr", [WZR]>, DwarfRegAlias<WSP>;
+def XZR   : AArch64Reg<31, "xzr", [WZR]>, DwarfRegAlias<WSP> { let isConstant = true; }
 }
 
 // Condition code register.
 def NZCV  : AArch64Reg<0, "nzcv">;
 
 // First fault status register
 def FFR : AArch64Reg<0, "ffr">, DwarfRegNum<[47]>;
 

llvm/lib/Target/Mips/MipsRegisterInfo.td

 class HWR<bits<16> Enc, string n> : MipsReg<Enc, n>;
 
 //===----------------------------------------------------------------------===//
 //  Registers
 //===----------------------------------------------------------------------===//
 
 let Namespace = "Mips" in {
   // General Purpose Registers
-  def ZERO : MipsGPRReg< 0, "zero">, DwarfRegNum<[0]>;
+  def ZERO : MipsGPRReg< 0, "zero">, DwarfRegNum<[0]> { let isConstant = true; }
   def AT   : MipsGPRReg< 1, "1">,    DwarfRegNum<[1]>;
   def V0   : MipsGPRReg< 2, "2">,    DwarfRegNum<[2]>;
   def V1   : MipsGPRReg< 3, "3">,    DwarfRegNum<[3]>;
   def A0   : MipsGPRReg< 4, "4">,    DwarfRegNum<[4]>;
   def A1   : MipsGPRReg< 5, "5">,    DwarfRegNum<[5]>;
   def A2   : MipsGPRReg< 6, "6">,    DwarfRegNum<[6]>;
   def A3   : MipsGPRReg< 7, "7">,    DwarfRegNum<[7]>;
   def T0   : MipsGPRReg< 8, "8">,    DwarfRegNum<[8]>;
let Namespace = "Mips" in {
   def K0   : MipsGPRReg< 26, "26">,  DwarfRegNum<[26]>;
   def K1   : MipsGPRReg< 27, "27">,  DwarfRegNum<[27]>;
   def GP   : MipsGPRReg< 28, "gp">,  DwarfRegNum<[28]>;
   def SP   : MipsGPRReg< 29, "sp">,  DwarfRegNum<[29]>;
   def FP   : MipsGPRReg< 30, "fp">,  DwarfRegNum<[30]>;
   def RA   : MipsGPRReg< 31, "ra">,  DwarfRegNum<[31]>;
 
   // General Purpose 64-bit Registers
-  def ZERO_64 : Mips64GPRReg< 0, "zero", [ZERO]>, DwarfRegNum<[0]>;
+  def ZERO_64 : Mips64GPRReg< 0, "zero", [ZERO]>, DwarfRegNum<[0]> { let isConstant = true; }
   def AT_64   : Mips64GPRReg< 1, "1",    [AT]>, DwarfRegNum<[1]>;
   def V0_64   : Mips64GPRReg< 2, "2",    [V0]>, DwarfRegNum<[2]>;
   def V1_64   : Mips64GPRReg< 3, "3",    [V1]>, DwarfRegNum<[3]>;
   def A0_64   : Mips64GPRReg< 4, "4",    [A0]>, DwarfRegNum<[4]>;
   def A1_64   : Mips64GPRReg< 5, "5",    [A1]>, DwarfRegNum<[5]>;
   def A2_64   : Mips64GPRReg< 6, "6",    [A2]>, DwarfRegNum<[6]>;
   def A3_64   : Mips64GPRReg< 7, "7",    [A3]>, DwarfRegNum<[7]>;
   def T0_64   : Mips64GPRReg< 8, "8",    [T0]>, DwarfRegNum<[8]>;

llvm/lib/Target/RISCV/RISCVRegisterInfo.td

 // Integer registers
 // CostPerUse is set higher for registers that may not be compressible as they
 // are not part of GPRC, the most restrictive register class used by the
 // compressed instruction set. This will influence the greedy register
 // allocator to reduce the use of registers that can't be encoded in 16 bit
 // instructions.
 
 let RegAltNameIndices = [ABIRegAltName] in {
-  def X0  : RISCVReg<0, "x0", ["zero"]>, DwarfRegNum<[0]>;
+  def X0  : RISCVReg<0, "x0", ["zero"]>, DwarfRegNum<[0]> { let isConstant = true; }
   let CostPerUse = [0, 1] in {
   def X1  : RISCVReg<1, "x1", ["ra"]>, DwarfRegNum<[1]>;
   def X2  : RISCVReg<2, "x2", ["sp"]>, DwarfRegNum<[2]>;
   def X3  : RISCVReg<3, "x3", ["gp"]>, DwarfRegNum<[3]>;
   def X4  : RISCVReg<4, "x4", ["tp"]>, DwarfRegNum<[4]>;
   def X5  : RISCVReg<5, "x5", ["t0"]>, DwarfRegNum<[5]>;
   def X6  : RISCVReg<6, "x6", ["t1"]>, DwarfRegNum<[6]>;
   def X7  : RISCVReg<7, "x7", ["t2"]>, DwarfRegNum<[7]>;

llvm/test/CodeGen/AArch64/avoid-zero-copy.mir

 # Check that we can remove the redundant save of constant registers such as $wzr
 # RUN: llc -mtriple=aarch64-unknown-linux %s -start-before=machine-cp -o - | FileCheck %s --check-prefix ASM
 # RUN: llc -mtriple=aarch64-unknown-linux %s -run-pass=machine-cp -o - | FileCheck %s
 --- |
   target triple = "aarch64-unknown-linux"
   declare i32 @bar(i32) nounwind
   define i32 @foo() nounwind {
   ; ASM-LABEL: foo:
   ; ASM:       // %bb.0:
-  ; ASM-NEXT:    stp x30, x19, [sp, #-16]! // 16-byte Folded Spill
+  ; ASM-NEXT:    str x30, [sp, #-16]! // 8-byte Folded Spill
   ; ASM-NEXT:    mov w0, wzr
-  ; ASM-NEXT:    mov w19, wzr
   ; ASM-NEXT:    bl bar
-  ; ASM-NEXT:    mov w0, w19
-  ; ASM-NEXT:    ldp x30, x19, [sp], #16 // 16-byte Folded Reload
+  ; ASM-NEXT:    mov w0, wzr
+  ; ASM-NEXT:    ldr x30, [sp], #16 // 8-byte Folded Reload
   ; ASM-NEXT:    ret
     call i32 @bar(i32 0)
     ret i32 0
   }
 ...
 ---
 name: foo
 body: |
   bb.0 (%ir-block.0):
     ; CHECK-LABEL: name: foo
     ; CHECK: ADJCALLSTACKDOWN 0, 0, implicit-def dead $sp, implicit $sp
-    ; CHECK-NEXT: renamable $w19 = COPY $wzr
     ; CHECK-NEXT: $w0 = COPY $wzr
     ; CHECK-NEXT: BL @bar, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit $w0, implicit-def $sp, implicit-def $w0
     ; CHECK-NEXT: ADJCALLSTACKUP 0, 0, implicit-def dead $sp, implicit $sp
-    ; CHECK-NEXT: $w0 = COPY killed renamable $w19
+    ; CHECK-NEXT: $w0 = COPY $wzr
     ; CHECK-NEXT: RET_ReallyLR implicit $w0
     ADJCALLSTACKDOWN 0, 0, implicit-def dead $sp, implicit $sp
     renamable $w19 = COPY $wzr
     $w0 = COPY renamable $w19
     BL @bar, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit $w0, implicit-def $sp, implicit-def $w0
     ADJCALLSTACKUP 0, 0, implicit-def dead $sp, implicit $sp
     $w0 = COPY killed renamable $w19
     RET_ReallyLR implicit $w0
 ...

llvm/test/CodeGen/Mips/avoid-zero-copy.mir

 # Check that we can remove the redundant save of constant registers such as $zero
 # RUN: llc -mtriple=mips64-unknown-freebsd %s -start-before=machine-cp -o - | FileCheck %s --check-prefix ASM
 # RUN: llc -mtriple=mips64-unknown-freebsd %s -verify-machineinstrs -run-pass=machine-cp -o - | FileCheck %s
 --- |
   target triple = "mips64-unknown-freebsd"
   define i32 @a() nounwind {
   ; ASM-LABEL: a:
   ; ASM:       # %bb.0:
   ; ASM-NEXT:    daddiu $sp, $sp, -16
   ; ASM-NEXT:    sd $ra, 8($sp) # 8-byte Folded Spill
-  ; ASM-NEXT:    sd $16, 0($sp) # 8-byte Folded Spill
-  ; ASM-NEXT:    move $16, $zero
   ; ASM-NEXT:    jalr $25
   ; ASM-NEXT:    move $4, $zero
-  ; ASM-NEXT:    move $4, $16
-  ; ASM-NEXT:    ld $16, 0($sp) # 8-byte Folded Reload
+  ; ASM-NEXT:    move $4, $zero
   ; ASM-NEXT:    ld $ra, 8($sp) # 8-byte Folded Reload
   ; ASM-NEXT:    jr $ra
   ; ASM-NEXT:    daddiu $sp, $sp, 16
     ret i32 0
   }
 ...
 ---
 name:            a
 body:             |
   bb.0 (%ir-block.0):
     liveins: $a0_64, $t9_64, $ra_64, $fp_64, $gp_64
     ; CHECK-LABEL: name: a
     ; CHECK: liveins: $a0_64, $t9_64, $ra_64, $fp_64, $gp_64
     ; CHECK-NEXT: {{  $}}
     ; CHECK-NEXT: ADJCALLSTACKDOWN 0, 0, implicit-def dead $sp, implicit $sp
-    ; CHECK-NEXT: renamable $s0_64 = COPY $zero_64
     ; CHECK-NEXT: $a0_64 = COPY $zero_64
     ; CHECK-NEXT: JALR64Pseudo $t9_64, csr_n64, implicit-def dead $ra, implicit $a0_64, implicit $gp_64, implicit-def $sp, implicit-def $v0
     ; CHECK-NEXT: ADJCALLSTACKUP 0, 0, implicit-def dead $sp, implicit $sp
-    ; CHECK-NEXT: $a0_64 = COPY killed renamable $s0_64
+    ; CHECK-NEXT: $a0_64 = COPY $zero_64
     ; CHECK-NEXT: PseudoReturn64 undef $ra_64, implicit $v0_64, implicit killed $a0_64
     ADJCALLSTACKDOWN 0, 0, implicit-def dead $sp, implicit $sp
     renamable $s0_64 = COPY $zero_64
     $a0_64 = COPY renamable $s0_64
     JALR64Pseudo $t9_64, csr_n64, implicit-def dead $ra, implicit $a0_64, implicit $gp_64, implicit-def $sp, implicit-def $v0
     ADJCALLSTACKUP 0, 0, implicit-def dead $sp, implicit $sp
     $a0_64 = COPY killed renamable $s0_64
     PseudoReturn64 undef $ra_64, implicit $v0_64, implicit killed $a0_64
 ...

llvm/test/CodeGen/RISCV/double-convert.ll

 ; RV32I-NEXT:    lui a0, 278016
 ; RV32I-NEXT:    addi s3, a0, -1
 ; RV32I-NEXT:    li a2, -1
 ; RV32I-NEXT:    mv a0, s1
 ; RV32I-NEXT:    mv a3, s3
 ; RV32I-NEXT:    call __gtdf2@plt
 ; RV32I-NEXT:    mv s4, a0
 ; RV32I-NEXT:    lui a3, 802304
 ; RV32I-NEXT:    li s2, 0
 ; RV32I-NEXT:    mv a0, s1
 ; RV32I-NEXT:    mv a1, s0
 ; RV32I-NEXT:    li a2, 0
 ; RV32I-NEXT:    call __gedf2@plt
 ; RV32I-NEXT:    mv s6, a0
 ; RV32I-NEXT:    mv a0, s1
 ; RV32I-NEXT:    mv a1, s0
 ; RV32I-NEXT:    call __fixdfdi@plt
 ; RV32I-NEXT:    mv s5, a1
-; RV32I-NEXT:    mv a1, s2
+; RV32I-NEXT:    li a1, 0
 ; RV32I-NEXT:    bltz s6, .LBB12_2
 ; RV32I-NEXT:  # %bb.1: # %start
 ; RV32I-NEXT:    mv a1, a0
 ; RV32I-NEXT:  .LBB12_2: # %start
 ; RV32I-NEXT:    li s6, -1
 ; RV32I-NEXT:    blt s2, s4, .LBB12_4
 ; RV32I-NEXT:  # %bb.3: # %start
 ; RV32I-NEXT:    mv s6, a1

 ; RV32I-NEXT:    li s0, 0
 ; RV32I-NEXT:    mv a0, s2
 ; RV32I-NEXT:    li a2, 0
 ; RV32I-NEXT:    call __gtdf2@plt
 ; RV32I-NEXT:    mv s3, a0
 ; RV32I-NEXT:    lui a3, 790016
 ; RV32I-NEXT:    mv a0, s2
 ; RV32I-NEXT:    mv a1, s1
-; RV32I-NEXT:    mv a2, s0
+; RV32I-NEXT:    li a2, 0
 ; RV32I-NEXT:    call __gedf2@plt
 ; RV32I-NEXT:    mv s4, a0
 ; RV32I-NEXT:    mv a0, s2
 ; RV32I-NEXT:    mv a1, s1
 ; RV32I-NEXT:    call __fixdfsi@plt
 ; RV32I-NEXT:    lui s5, 1048568
 ; RV32I-NEXT:    bltz s4, .LBB26_2
 ; RV32I-NEXT:  # %bb.1: # %start

 ; RV32I-NEXT:    lui a3, 263676
 ; RV32I-NEXT:    li s0, 0
 ; RV32I-NEXT:    li a2, 0
 ; RV32I-NEXT:    call __gtdf2@plt
 ; RV32I-NEXT:    mv s3, a0
 ; RV32I-NEXT:    lui a3, 787968
 ; RV32I-NEXT:    mv a0, s2
 ; RV32I-NEXT:    mv a1, s1
-; RV32I-NEXT:    mv a2, s0
+; RV32I-NEXT:    li a2, 0
 ; RV32I-NEXT:    call __gedf2@plt
 ; RV32I-NEXT:    mv s4, a0
 ; RV32I-NEXT:    mv a0, s2
 ; RV32I-NEXT:    mv a1, s1
 ; RV32I-NEXT:    call __fixdfsi@plt
 ; RV32I-NEXT:    li a1, -128
 ; RV32I-NEXT:    bltz s4, .LBB30_2
 ; RV32I-NEXT:  # %bb.1: # %start

llvm/utils/TableGen/CodeGenRegisters.h

namespace llvm {
   /// CodeGenRegister - Represents a register definition.
   struct CodeGenRegister {
     Record *TheDef;
     unsigned EnumValue;
     std::vector<int64_t> CostPerUse;
     bool CoveredBySubRegs;
     bool HasDisjunctSubRegs;
     bool Artificial;
+    bool Constant;
 
     // Map SubRegIndex -> Register.
     typedef std::map<CodeGenSubRegIndex *, CodeGenRegister *,
                      deref<std::less<>>>
         SubRegMap;
 
     CodeGenRegister(Record *R, unsigned Enum);
 

llvm/utils/TableGen/CodeGenRegisters.cpp

 //===----------------------------------------------------------------------===//
 //                              CodeGenRegister
 //===----------------------------------------------------------------------===//
 
 CodeGenRegister::CodeGenRegister(Record *R, unsigned Enum)
     : TheDef(R), EnumValue(Enum),
       CostPerUse(R->getValueAsListOfInts("CostPerUse")),
       CoveredBySubRegs(R->getValueAsBit("CoveredBySubRegs")),
-      HasDisjunctSubRegs(false), SubRegsComplete(false),
-      SuperRegsComplete(false), TopoSig(~0u) {
+      HasDisjunctSubRegs(false), Constant(R->getValueAsBit("isConstant")),
+      SubRegsComplete(false), SuperRegsComplete(false), TopoSig(~0u) {
   Artificial = R->getValueAsBit("isArtificial");
 }
 
 void CodeGenRegister::buildObjectGraph(CodeGenRegBank &RegBank) {
   std::vector<Record*> SRIs = TheDef->getValueAsListOfDefs("SubRegIndices");
   std::vector<Record*> SRs = TheDef->getValueAsListOfDefs("SubRegs");
 
   if (SRIs.size() != SRs.size())
     PrintFatalError(TheDef->getLoc(),

llvm/utils/TableGen/RegisterInfoEmitter.cpp

for (unsigned i = 0, e = CSRSets.size(); i != e; ++i) {
     // Add those registers to RegMask, but not to SaveList.
     if (DagInit *OPDag =
         dyn_cast<DagInit>(CSRSet->getValueInit("OtherPreserved"))) {
       SetTheory::RecSet OPSet;
       RegBank.getSets().evaluate(OPDag, OPSet, CSRSet->getLoc());
       Covered |= RegBank.computeCoveredRegisters(
         ArrayRef<Record*>(OPSet.begin(), OPSet.end()));
     }
+    // Add all constant physical registers to the preserved mask:
+    SetTheory::RecSet ConstantSet;
+    for (auto& Reg : RegBank.getRegisters()) {
+      if (Reg.Constant)
+        ConstantSet.insert(Reg.TheDef);
+    }
+    Covered |= RegBank.computeCoveredRegisters(
+        ArrayRef<Record*>(ConstantSet.begin(), ConstantSet.end()));
 
     OS << "static const uint32_t " << CSRSet->getName()
        << "_RegMask[] = { ";
     printBitVectorAsHex(OS, Covered, 32);
     OS << "};\n";
   }
   OS << "\n\n";