[PATCH] D92405: [VirtRegRewriter] Insert missing killed flags when tracking subregister liveness

[Baptiste Saleil via Phabricator via llvm-commits]

bsaleil marked 4 inline comments as done.
bsaleil added inline comments.


================
Comment at: llvm/lib/CodeGen/LiveIntervals.cpp:705-706
-  // Keep track of subregister ranges.
-  SmallVector<std::pair<const LiveInterval::SubRange*,
-                        LiveRange::const_iterator>, 4> SRs;
 
----------------
arsenm wrote:
> What was the point of copying out the subranges here before?
To be honest, I don't understand why we copied the subranges here. @MatzeB, do you have any idea ?


================
Comment at: llvm/lib/CodeGen/LiveIntervals.cpp:728-750
+      SlotIndex InstrSI = RI->end;
       // A block index indicates an MBB edge.
-      if (RI->end.isBlock())
+      if (InstrSI.isBlock())
         continue;
-      MachineInstr *MI = getInstructionFromIndex(RI->end);
+      MachineInstr *MI = getInstructionFromIndex(InstrSI);
       if (!MI)
         continue;
----------------
arsenm wrote:
> This looks like a separate NFC change
you're right, I removed that.


================
Comment at: llvm/lib/CodeGen/LiveIntervals.cpp:794-795
+            if (MO.getSubReg() == 0)
+              if (const TargetRegisterClass *TRC = MRI->getRegClassOrNull(Reg))
+                UseMask = TRC->getLaneMask();
             if ((UseMask & ~DefinedLanesMask).any())
----------------
lkail wrote:
> arsenm wrote:
> > getRegClass should be sufficient. There can't be a vreg without a class at this point
> I think it can be simplified as
> ```
>             unsigned SubReg = MO.getSubReg();
>             LaneBitmask UseMask = SubReg ? TRI->getSubRegIndexLaneMask(SubReg)
>                                          : MRI->getMaxLaneMaskForVReg(Reg);
> 
> ```
Thanks, good catch @lkail I simplified that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92405/new/

https://reviews.llvm.org/D92405