Skip to content

Conversation

@sdesmalen-arm
Copy link
Collaborator

By moving the code a bit later, we can factor out some of the conditions as those are now already tested.
This will also be useful when adding another fix on top that uses NewMI's subreg index (to follow as a separate PR).

The change is intended to be NFC.

By moving the code a bit later, we can factor out some of the conditions as
those are now already tested, which will help when adding another fix
on top that uses `NewMI`'s subreg index (NewIdx).

This change is intended to be NFC.
@llvmbot
Copy link
Member

llvmbot commented Jan 6, 2025

@llvm/pr-subscribers-llvm-regalloc

Author: Sander de Smalen (sdesmalen-arm)

Changes

By moving the code a bit later, we can factor out some of the conditions as those are now already tested.
This will also be useful when adding another fix on top that uses NewMI's subreg index (to follow as a separate PR).

The change is intended to be NFC.


Full diff: https://github.com/llvm/llvm-project/pull/121779.diff

1 Files Affected:

  • (modified) llvm/lib/CodeGen/RegisterCoalescer.cpp (+20-21)
diff --git a/llvm/lib/CodeGen/RegisterCoalescer.cpp b/llvm/lib/CodeGen/RegisterCoalescer.cpp
index 20ad6445344d83..712c64bb9cf6d5 100644
--- a/llvm/lib/CodeGen/RegisterCoalescer.cpp
+++ b/llvm/lib/CodeGen/RegisterCoalescer.cpp
@@ -1374,27 +1374,6 @@ bool RegisterCoalescer::reMaterializeTrivialDef(const CoalescerPair &CP,
   MachineInstr &NewMI = *std::prev(MII);
   NewMI.setDebugLoc(DL);
 
-  // In a situation like the following:
-  //
-  //    undef %2.subreg:reg = INST %1:reg         ; DefMI (rematerializable),
-  //                                              ; DefSubIdx = subreg
-  //    %3:reg = COPY %2                          ; SrcIdx = DstIdx = 0
-  //    .... = SOMEINSTR %3:reg
-  //
-  // there are no subranges for %3 so after rematerialization we need
-  // to explicitly create them. Undefined subranges are removed later on.
-  if (DstReg.isVirtual() && DefSubIdx && !CP.getSrcIdx() && !CP.getDstIdx() &&
-      MRI->shouldTrackSubRegLiveness(DstReg)) {
-    LiveInterval &DstInt = LIS->getInterval(DstReg);
-    if (!DstInt.hasSubRanges()) {
-      LaneBitmask FullMask = MRI->getMaxLaneMaskForVReg(DstReg);
-      LaneBitmask UsedLanes = TRI->getSubRegIndexLaneMask(DefSubIdx);
-      LaneBitmask UnusedLanes = FullMask & ~UsedLanes;
-      DstInt.createSubRangeFrom(LIS->getVNInfoAllocator(), UsedLanes, DstInt);
-      DstInt.createSubRangeFrom(LIS->getVNInfoAllocator(), UnusedLanes, DstInt);
-    }
-  }
-
   // In a situation like the following:
   //     %0:subreg = instr              ; DefMI, subreg = DstIdx
   //     %1        = copy %0:subreg ; CopyMI, SrcIdx = 0
@@ -1523,6 +1502,26 @@ bool RegisterCoalescer::reMaterializeTrivialDef(const CoalescerPair &CP,
     // sure that "undef" is not set.
     if (NewIdx == 0)
       NewMI.getOperand(0).setIsUndef(false);
+
+    // In a situation like the following:
+    //
+    //    undef %2.subreg:reg = INST %1:reg         ; DefMI (rematerializable),
+    //                                              ; DefSubIdx = subreg
+    //    %3:reg = COPY %2                          ; SrcIdx = DstIdx = 0
+    //    .... = SOMEINSTR %3:reg
+    //
+    // there are no subranges for %3 so after rematerialization we need
+    // to explicitly create them. Undefined subranges are removed later on.
+    if (DefSubIdx && !CP.getSrcIdx() && !CP.getDstIdx() &&
+        MRI->shouldTrackSubRegLiveness(DstReg) && !DstInt.hasSubRanges()) {
+      LaneBitmask FullMask = MRI->getMaxLaneMaskForVReg(DstReg);
+      LaneBitmask UsedLanes = TRI->getSubRegIndexLaneMask(DefSubIdx);
+      LaneBitmask UnusedLanes = FullMask & ~UsedLanes;
+      VNInfo::Allocator &Alloc = LIS->getVNInfoAllocator();
+      DstInt.createSubRangeFrom(Alloc, UsedLanes, DstInt);
+      DstInt.createSubRangeFrom(Alloc, UnusedLanes, DstInt);
+    }
+
     // Add dead subregister definitions if we are defining the whole register
     // but only part of it is live.
     // This could happen if the rematerialization instruction is rematerializing

@sdesmalen-arm sdesmalen-arm merged commit 5514865 into main Jan 7, 2025
10 checks passed
@sdesmalen-arm sdesmalen-arm deleted the users/sdesmalen-arm/srlt-fix-coalescer-the-sequel-nfci branch January 7, 2025 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants