[llvm] [CodeGen][SplitKit] Fix a crash in addDeadDef (PR #197014)

[via llvm-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-regalloc

Author: Quentin Colombet (qcolombet)

<details>
<summary>Changes</summary>

This commit fixes https://github.com/llvm/llvm-project/issues/178867.

The problem was that we were trying to materialize a lanemask that does not existing in the parent live-interval and were crashing.

This happened because we were using the original live-intveral to materialize the live subranges (original is the ancestor of the parent and may cover some lanes in the lanemasks that are not cover by the parent live-interval).

The crash was reported by a downstream user and they were not able to capture the issue with an upstream target. The issue appears because of the refining of the subranges. We are creating values out of thin air but they may cover more lanes than the parent because we infer them from the original live-range.

The fix consists in taking the lanes covered by the parent, not the original value, when creating the live-interval for the children.

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


1 Files Affected:

- (modified) llvm/lib/CodeGen/SplitKit.cpp (+26-29) 


``````````diff
diff --git a/llvm/lib/CodeGen/SplitKit.cpp b/llvm/lib/CodeGen/SplitKit.cpp
index 560aa03f38164..0440d2ac0dbd5 100644
--- a/llvm/lib/CodeGen/SplitKit.cpp
+++ b/llvm/lib/CodeGen/SplitKit.cpp
@@ -413,15 +413,26 @@ const LiveInterval::SubRange &getSubRangeForMaskExact(LaneBitmask LM,
 }
 
 /// Find a subrange corresponding to the lane mask @p LM, or a superset of it,
-/// in the live interval @p LI. The interval @p LI is assumed to contain such
-/// a subrange.  This function is used to find corresponding subranges between
-/// the original interval and the new intervals.
-const LiveInterval::SubRange &getSubRangeForMask(LaneBitmask LM,
-                                                 const LiveInterval &LI) {
+/// in the live interval @p LI.
+/// \return nullptr is such subrange is not found.
+const LiveInterval::SubRange *findSubRangeForMask(LaneBitmask LM,
+                                                  const LiveInterval &LI) {
   for (const LiveInterval::SubRange &S : LI.subranges())
     if ((S.LaneMask & LM) == LM)
-      return S;
-  llvm_unreachable("SubRange for this mask not found");
+      return &S;
+  return nullptr;
+}
+
+LaneBitmask getLiveLaneMaskAt(const LiveInterval &LI, SlotIndex Idx,
+                              const MachineRegisterInfo &MRI) {
+  if (!LI.hasSubRanges())
+    return MRI.getMaxLaneMaskForVReg(LI.reg());
+
+  LaneBitmask LaneMask = LaneBitmask::getNone();
+  for (const LiveInterval::SubRange &S : LI.subranges())
+    if (S.liveAt(Idx))
+      LaneMask |= S.LaneMask;
+  return LaneMask;
 }
 
 void SplitEditor::addDeadDef(LiveInterval &LI, VNInfo *VNI, bool Original) {
@@ -436,8 +447,11 @@ void SplitEditor::addDeadDef(LiveInterval &LI, VNInfo *VNI, bool Original) {
     // to only update the subranges for which the original subranges had
     // a def at this location.
     for (LiveInterval::SubRange &S : LI.subranges()) {
-      auto &PS = getSubRangeForMask(S.LaneMask, Edit->getParent());
-      VNInfo *PV = PS.getVNInfoAt(Def);
+      const LiveInterval::SubRange *PS =
+          findSubRangeForMask(S.LaneMask, Edit->getParent());
+      if (PS == nullptr)
+        continue;
+      VNInfo *PV = PS->getVNInfoAt(Def);
       if (PV != nullptr && PV->def == Def)
         S.createDeadDef(Def, LIS.getVNInfoAllocator());
     }
@@ -637,21 +651,15 @@ VNInfo *SplitEditor::defFromParent(unsigned RegIdx, const VNInfo *ParentVNI,
   VNInfo *OrigVNI = OrigLI.getVNInfoAt(UseIdx);
 
   Register Reg = LI->reg();
-  if (OrigVNI) {
+  LaneBitmask LaneMask = getLiveLaneMaskAt(Edit->getParent(), UseIdx, MRI);
+  if (OrigVNI && LaneMask.any()) {
     LiveRangeEdit::Remat RM(ParentVNI);
     RM.OrigMI = LIS.getInstructionFromIndex(OrigVNI->def);
     if (RM.OrigMI && TII.isAsCheapAsAMove(*RM.OrigMI) &&
         Edit->canRematerializeAt(RM, UseIdx)) {
       if (!rematWillIncreaseRestriction(RM.OrigMI, MBB, UseIdx)) {
-        LaneBitmask UsedLanes = LaneBitmask::getAll();
-        if (OrigLI.hasSubRanges()) {
-          UsedLanes = LaneBitmask::getNone();
-          for (const LiveInterval::SubRange &SR : OrigLI.subranges())
-            if (SR.liveAt(UseIdx))
-              UsedLanes |= SR.LaneMask;
-        }
         SlotIndex Def = Edit->rematerializeAt(MBB, I, Reg, RM, TRI, Late, 0,
-                                              nullptr, UsedLanes);
+                                              nullptr, LaneMask);
         ++NumRemats;
         // Define the value in Reg.
         return defValue(RegIdx, ParentVNI, Def, false);
@@ -663,17 +671,6 @@ VNInfo *SplitEditor::defFromParent(unsigned RegIdx, const VNInfo *ParentVNI,
     }
   }
 
-  LaneBitmask LaneMask;
-  if (OrigLI.hasSubRanges()) {
-    LaneMask = LaneBitmask::getNone();
-    for (LiveInterval::SubRange &S : OrigLI.subranges()) {
-      if (S.liveAt(UseIdx))
-        LaneMask |= S.LaneMask;
-    }
-  } else {
-    LaneMask = LaneBitmask::getAll();
-  }
-
   SlotIndex Def;
   if (LaneMask.none()) {
     const MCInstrDesc &Desc = TII.get(TargetOpcode::IMPLICIT_DEF);

``````````

</details>


https://github.com/llvm/llvm-project/pull/197014