Revert "[MachineCP] Correctly handle register masks and sub-registers (#122472)"

This reverts commit e2a071e.

This causes a large compile-time regression.

Author: nikic

llvm/lib/CodeGen/MachineCopyPropagation.cpp

@@ -164,91 +164,67 @@ class CopyTracker {
       Copies.erase(Unit);
   }
 
-  /// Clobber a single register unit, removing it from the tracker's copy maps.
-  void clobberRegUnit(MCRegUnit Unit, const TargetRegisterInfo &TRI,
-                      const TargetInstrInfo &TII, bool UseCopyInstr) {
-    auto I = Copies.find(Unit);
-    if (I != Copies.end()) {
-      // When we clobber the source of a copy, we need to clobber everything
-      // it defined.
-      markRegsUnavailable(I->second.DefRegs, TRI);
-      // When we clobber the destination of a copy, we need to clobber the
-      // whole register it defined.
-      if (MachineInstr *MI = I->second.MI) {
-        std::optional<DestSourcePair> CopyOperands =
-            isCopyInstr(*MI, TII, UseCopyInstr);
-
-        MCRegister Def = CopyOperands->Destination->getReg().asMCReg();
-        MCRegister Src = CopyOperands->Source->getReg().asMCReg();
-
-        markRegsUnavailable(Def, TRI);
-
-        // Since we clobber the destination of a copy, the semantic of Src's
-        // "DefRegs" to contain Def is no longer effectual. We will also need
-        // to remove the record from the copy maps that indicates Src defined
-        // Def. Failing to do so might cause the target to miss some
-        // opportunities to further eliminate redundant copy instructions.
-        // Consider the following sequence during the
-        // ForwardCopyPropagateBlock procedure:
-        // L1: r0 = COPY r9     <- TrackMI
-        // L2: r0 = COPY r8     <- TrackMI (Remove r9 defined r0 from tracker)
-        // L3: use r0           <- Remove L2 from MaybeDeadCopies
-        // L4: early-clobber r9 <- Clobber r9 (L2 is still valid in tracker)
-        // L5: r0 = COPY r8     <- Remove NopCopy
-        for (MCRegUnit SrcUnit : TRI.regunits(Src)) {
-          auto SrcCopy = Copies.find(SrcUnit);
-          if (SrcCopy != Copies.end() && SrcCopy->second.LastSeenUseInCopy) {
-            // If SrcCopy defines multiple values, we only need
-            // to erase the record for Def in DefRegs.
-            for (auto itr = SrcCopy->second.DefRegs.begin();
-                 itr != SrcCopy->second.DefRegs.end(); itr++) {
-              if (*itr == Def) {
-                SrcCopy->second.DefRegs.erase(itr);
-                // If DefReg becomes empty after removal, we can remove the
-                // SrcCopy from the tracker's copy maps. We only remove those
-                // entries solely record the Def is defined by Src. If an
-                // entry also contains the definition record of other Def'
-                // registers, it cannot be cleared.
-                if (SrcCopy->second.DefRegs.empty() && !SrcCopy->second.MI) {
-                  Copies.erase(SrcCopy);
+  /// Clobber a single register, removing it from the tracker's copy maps.
+  void clobberRegister(MCRegister Reg, const TargetRegisterInfo &TRI,
+                       const TargetInstrInfo &TII, bool UseCopyInstr) {
+    for (MCRegUnit Unit : TRI.regunits(Reg)) {
+      auto I = Copies.find(Unit);
+      if (I != Copies.end()) {
+        // When we clobber the source of a copy, we need to clobber everything
+        // it defined.
+        markRegsUnavailable(I->second.DefRegs, TRI);
+        // When we clobber the destination of a copy, we need to clobber the
+        // whole register it defined.
+        if (MachineInstr *MI = I->second.MI) {
+          std::optional<DestSourcePair> CopyOperands =
+              isCopyInstr(*MI, TII, UseCopyInstr);
+
+          MCRegister Def = CopyOperands->Destination->getReg().asMCReg();
+          MCRegister Src = CopyOperands->Source->getReg().asMCReg();
+
+          markRegsUnavailable(Def, TRI);
+
+          // Since we clobber the destination of a copy, the semantic of Src's
+          // "DefRegs" to contain Def is no longer effectual. We will also need
+          // to remove the record from the copy maps that indicates Src defined
+          // Def. Failing to do so might cause the target to miss some
+          // opportunities to further eliminate redundant copy instructions.
+          // Consider the following sequence during the
+          // ForwardCopyPropagateBlock procedure:
+          // L1: r0 = COPY r9     <- TrackMI
+          // L2: r0 = COPY r8     <- TrackMI (Remove r9 defined r0 from tracker)
+          // L3: use r0           <- Remove L2 from MaybeDeadCopies
+          // L4: early-clobber r9 <- Clobber r9 (L2 is still valid in tracker)
+          // L5: r0 = COPY r8     <- Remove NopCopy
+          for (MCRegUnit SrcUnit : TRI.regunits(Src)) {
+            auto SrcCopy = Copies.find(SrcUnit);
+            if (SrcCopy != Copies.end() && SrcCopy->second.LastSeenUseInCopy) {
+              // If SrcCopy defines multiple values, we only need
+              // to erase the record for Def in DefRegs.
+              for (auto itr = SrcCopy->second.DefRegs.begin();
+                   itr != SrcCopy->second.DefRegs.end(); itr++) {
+                if (*itr == Def) {
+                  SrcCopy->second.DefRegs.erase(itr);
+                  // If DefReg becomes empty after removal, we can remove the
+                  // SrcCopy from the tracker's copy maps. We only remove those
+                  // entries solely record the Def is defined by Src. If an
+                  // entry also contains the definition record of other Def'
+                  // registers, it cannot be cleared.
+                  if (SrcCopy->second.DefRegs.empty() && !SrcCopy->second.MI) {
+                    Copies.erase(SrcCopy);
+                  }
+                  break;
                 }
-                break;
               }
             }
           }
         }
+        // Now we can erase the copy.
+        Copies.erase(I);
       }
-      // Now we can erase the copy.
-      Copies.erase(I);
     }
   }
 
-  /// Clobber a single register, removing it from the tracker's copy maps.
-  void clobberRegister(MCRegister Reg, const TargetRegisterInfo &TRI,
-                       const TargetInstrInfo &TII, bool UseCopyInstr) {
-    for (MCRegUnit Unit : TRI.regunits(Reg)) {
-      clobberRegUnit(Unit, TRI, TII, UseCopyInstr);
-    }
-  }
-
-  /// Clobber all registers which are not preserved by RegMask, removing them
-  /// from the tracker's copy maps.
-  void clobberRegistersExceptMask(const MachineOperand *RegMask,
-                                  const TargetRegisterInfo &TRI,
-                                  const TargetInstrInfo &TII,
-                                  bool UseCopyInstr) {
-    BitVector SafeRegUnits(TRI.getNumRegUnits());
-
-    for (unsigned SafeReg = 0, E = TRI.getNumRegs(); SafeReg < E; ++SafeReg)
-      if (!RegMask->clobbersPhysReg(SafeReg))
-        for (auto SafeUnit : TRI.regunits(SafeReg))
-          SafeRegUnits.set(SafeUnit);
-
-    for (unsigned Unit = 0, E = TRI.getNumRegUnits(); Unit < E; ++Unit)
-      if (!SafeRegUnits.test(Unit))
-        clobberRegUnit(Unit, TRI, TII, UseCopyInstr);
-  }
-
   /// Track copy's src users, and return false if that can't be done.
   /// We can only track if we have a COPY instruction which source is
   /// the same as the Reg.
@@ -984,10 +960,6 @@ void MachineCopyPropagation::ForwardCopyPropagateBlock(MachineBasicBlock &MBB) {
     // a large set of registers.  Treat clobbered registers the same way as
     // defined registers.
     if (RegMask) {
-      // Invalidate all entries in the copy map which are not preserved by this
-      // register mask.
-      Tracker.clobberRegistersExceptMask(RegMask, *TRI, *TII, UseCopyInstr);
-
       // Erase any MaybeDeadCopies whose destination register is clobbered.
       for (SmallSetVector<MachineInstr *, 8>::iterator DI =
                MaybeDeadCopies.begin();
@@ -1006,6 +978,10 @@ void MachineCopyPropagation::ForwardCopyPropagateBlock(MachineBasicBlock &MBB) {
         LLVM_DEBUG(dbgs() << "MCP: Removing copy due to regmask clobbering: ";
                    MaybeDead->dump());
 
+        // Make sure we invalidate any entries in the copy maps before erasing
+        // the instruction.
+        Tracker.clobberRegister(Reg, *TRI, *TII, UseCopyInstr);
+
         // erase() will return the next valid iterator pointing to the next
         // element after the erased one.
         DI = MaybeDeadCopies.erase(DI);

llvm/test/CodeGen/AArch64/machine-cp-sub-reg.mir

@@ -1,16 +1,5 @@
 # NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 4
-# RUN: llc -o - %s --run-pass=machine-cp -mcp-use-is-copy-instr -mtriple=arm64-apple-macos | FileCheck %s
-
---- |
-  declare void @foo()
-
-  define void @test() {
-    unreachable
-  }
-  define void @test2() {
-    unreachable
-  }
-...
+# RUN: llc -o - %s --run-pass=machine-cp -mcp-use-is-copy-instr -mtriple=arm64-apple-macos --verify-machineinstrs | FileCheck %s
 
 ---
 name: test
@@ -41,22 +30,3 @@ body:             |
 
     RET undef $lr, implicit $x0
 ...
----
-name:            test2
-tracksRegLiveness: true
-body: |
-  bb.0:
-    liveins: $q14, $d29, $x0, $x1
-    ; CHECK-LABEL: name: test2
-    ; CHECK: liveins: $q14, $d29, $x0, $x1
-    ; CHECK-NEXT: {{  $}}
-    ; CHECK-NEXT: renamable $d8 = COPY killed renamable $d29
-    ; CHECK-NEXT: BL @foo, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit-def $sp
-    ; CHECK-NEXT: renamable $b0 = SMAXVv8i8v killed renamable $d8, implicit-def $q0
-    ; CHECK-NEXT: RET_ReallyLR implicit $b0
-    renamable $q8 = COPY renamable $q14
-    renamable $d8 = COPY killed renamable $d29
-    BL @foo, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit-def $sp
-    renamable $b0 = SMAXVv8i8v killed renamable $d8, implicit-def $q0
-    RET_ReallyLR implicit $b0
-...