RegisterCoalescer: Ignore interferences for constant physregs

[MatzeB]

Summary

When copying to/from a constant register interferences can be ignored.

Also update the documentation for isConstantPhysReg() to make it more
obvious that this transformation is valid.

[gberry]

This seems reasonable to me, but I'm not familiar enough with the details of this pass to confidently approve it, so you should probably wait for Quentin's approval.

[qcolombet]

LGTM.

Nitpicks below.

include/llvm/CodeGen/MachineRegisterInfo.h

555 ↗

(On Diff #76277)

Isn't that implied by "constant"?

test/CodeGen/AArch64/regcoal-constreg.mir

6 ↗

(On Diff #76277)

Could you add a comment explaining what this function is testing?

llvm/trunk/include/llvm/CodeGen/MachineRegisterInfo.h

public:
   /// optimization passes which extend register lifetimes and need only
   /// preserve conservative kill flag information.
   void clearKillFlags(unsigned Reg) const;
 
 #ifndef NDEBUG
   void dumpUses(unsigned RegNo) const;
 #endif
 
-  /// isConstantPhysReg - Returns true if PhysReg is unallocatable and constant
-  /// throughout the function.  It is safe to move instructions that read such
-  /// a physreg.
+  /// Returns true if PhysReg is unallocatable and constant throughout the
+  /// function. Writing to a constant register has no effect.
   bool isConstantPhysReg(unsigned PhysReg) const;
 
   /// Get an iterator over the pressure sets affected by the given physical or
   /// virtual register. If RegUnit is physical, it must be a register unit (from
   /// MCRegUnitIterator).
   PSetIterator getPressureSets(unsigned RegUnit) const;
 
   //===--------------------------------------------------------------------===//

llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp

bool RegisterCoalescer::joinReservedPhysReg(CoalescerPair &CP) {
 
   // Optimization for reserved registers like ESP. We can only merge with a
   // reserved physreg if RHS has a single value that is a copy of DstReg.
   // The live range of the reserved register will look like a set of dead defs
   // - we don't properly track the live range of reserved registers.
 
   // Deny any overlapping intervals.  This depends on all the reserved
   // register live ranges to look like dead defs.
+  if (!MRI->isConstantPhysReg(DstReg)) {
     for (MCRegUnitIterator UI(DstReg, TRI); UI.isValid(); ++UI)
       if (RHS.overlaps(LIS->getRegUnit(*UI))) {
         DEBUG(dbgs() << "\t\tInterference: " << PrintRegUnit(*UI, TRI) << '\n');
         return false;
       }
+  }
 
   // Skip any value computations, we are not adding new values to the
   // reserved register.  Also skip merging the live ranges, the reserved
   // register live range doesn't need to be accurate as long as all the
   // defs are there.
 
   // Delete the identity copy.
   MachineInstr *CopyMI;
   if (CP.isFlipped()) {
     CopyMI = MRI->getVRegDef(RHS.reg);
   } else {
     if (!MRI->hasOneNonDBGUse(RHS.reg)) {
       DEBUG(dbgs() << "\t\tMultiple vreg uses!\n");
       return false;
     }
 
     MachineInstr *DestMI = MRI->getVRegDef(RHS.reg);
     CopyMI = &*MRI->use_instr_nodbg_begin(RHS.reg);
     const SlotIndex CopyRegIdx = LIS->getInstructionIndex(*CopyMI).getRegSlot();
     const SlotIndex DestRegIdx = LIS->getInstructionIndex(*DestMI).getRegSlot();
 
+    if (!MRI->isConstantPhysReg(DstReg)) {
       // We checked above that there are no interfering defs of the physical
       // register. However, for this case, where we intent to move up the def of
       // the physical register, we also need to check for interfering uses.
       SlotIndexes *Indexes = LIS->getSlotIndexes();
       for (SlotIndex SI = Indexes->getNextNonNullIndex(DestRegIdx);
            SI != CopyRegIdx; SI = Indexes->getNextNonNullIndex(SI)) {
         MachineInstr *MI = LIS->getInstructionFromIndex(SI);
         if (MI->readsRegister(DstReg, TRI)) {
           DEBUG(dbgs() << "\t\tInterference (read): " << *MI);
           return false;
         }
 
         // We must also check for clobbers caused by regmasks.
         for (const auto &MO : MI->operands()) {
           if (MO.isRegMask() && MO.clobbersPhysReg(DstReg)) {
             DEBUG(dbgs() << "\t\tInterference (regmask clobber): " << *MI);
             return false;
           }
         }
       }
+    }
 
     // We're going to remove the copy which defines a physical reserved
     // register, so remove its valno, etc.
     DEBUG(dbgs() << "\t\tRemoving phys reg def of " << DstReg << " at "
           << CopyRegIdx << "\n");
 
     LIS->removePhysRegDefAt(DstReg, CopyRegIdx);
     // Create a new dead def at the new def location.

llvm/trunk/test/CodeGen/AArch64/machine_cse_impdef_killflags.ll

 ; RUN: llc < %s -mtriple=aarch64-apple-ios -fast-isel -verify-machineinstrs | FileCheck %s
 
 ; Check that the kill flag is cleared between CSE'd instructions on their
 ; imp-def'd registers.
 ; The verifier would complain otherwise.
 define i64 @csed-impdef-killflag(i64 %a) {
 ; CHECK-LABEL: csed-impdef-killflag
-; CHECK-DAG:  mov    [[REG0:w[0-9]+]], wzr
 ; CHECK-DAG:  orr    [[REG1:w[0-9]+]], wzr, #0x1
 ; CHECK-DAG:  orr    [[REG2:x[0-9]+]], xzr, #0x2
 ; CHECK-DAG:  orr    [[REG3:x[0-9]+]], xzr, #0x3
 ; CHECK:      cmp    x0, #0
-; CHECK-DAG:  csel   w[[SELECT_WREG_1:[0-9]+]], [[REG0]], [[REG1]], ne
+; CHECK-DAG:  csel   w[[SELECT_WREG_1:[0-9]+]], wzr, [[REG1]], ne
 ; CHECK-DAG:  csel   [[SELECT_XREG_2:x[0-9]+]], [[REG2]], [[REG3]], ne
 ; CHECK:      ubfx   [[SELECT_XREG_1:x[0-9]+]], x[[SELECT_WREG_1]], #0, #32
 ; CHECK-NEXT: add    x0, [[SELECT_XREG_2]], [[SELECT_XREG_1]]
 ; CHECK-NEXT: ret
 
   %1 = icmp ne i64 %a, 0
   %2 = select i1 %1, i32 0, i32 1
   %3 = icmp ne i64 %a, 0
   %4 = select i1 %3, i64 2, i64 3
   %5 = zext i32 %2 to i64
   %6 = add i64 %4, %5
   ret i64 %6
 }

llvm/trunk/test/CodeGen/AArch64/regcoal-constreg.mir

+# RUN: llc -mtriple=aarch64-- -run-pass=simple-register-coalescing %s -o - | FileCheck %s
+--- |
+  define void @func() { ret void }
+...
+---
+# Check that we eliminate copies to/from constant physregs regardless of
+# "interfering" reads/writes.
+# CHECK: name: func
+# CHECK-NOT: COPY
+# CHECK: STRWui %wzr, %x1
+# CHECK-NOT: COPY
+# CHECK: STRXui %xzr, %x1
+# CHECK: %wzr = SUBSWri %w1, 0, 0
+name: func
+registers:
+  - { id: 0, class: gpr32 }
+  - { id: 1, class: gpr64 }
+  - { id: 2, class: gpr32 }
+body: |
+  bb.0:
+    %0 = COPY %wzr
+    dead %wzr = SUBSWri %w1, 0, 0, implicit-def %nzcv
+    STRWui %0, %x1, 0
+
+    %1 = COPY %xzr
+    dead %wzr = SUBSWri %w1, 0, 0, implicit-def %nzcv
+    STRXui %1, %x1, 0
+
+    %2 = SUBSWri %w1, 0, 0, implicit-def %nzcv
+    %wzr = COPY %2
+...