[PATCH] D16693: Make sure all subranges are define during re-materialization of constants in RegCoalescer

[Marcello Maggioni via llvm-commits]

kariddi created this revision.
kariddi added a reviewer: MatzeB.
kariddi added a subscriber: llvm-commits.
kariddi set the repository for this revision to rL LLVM.
Herald added subscribers: qcolombet, MatzeB.

Hello,

In our target we sometimes load multiple constants with the same instruction into registers that have disjoint subregisters and then use the subregisters separately.

In certain cases during constant re-materialization in the register coalescer the one of these multiple constants load instructions can be selected to rematerialize only one of the constants.

The situation is something like this:

 vreg1 = LOAD CONSTANTS 5, 8 ; Loading both 5 and 8 in different subregs
 ; Copying only part of the register here, but the rest is undef.
 vreg2:sub_16bit<def, read-undef> = COPY vreg1:sub_16bit
 ==>
 ; Materialize all the constants but only using one eventually.
 vreg2 = LOAD_CONSTANTS 5, 8

In the above case we had an undefined registers that gets partially defined by the copy of one of the two constants in VREG1.
The re-materialization substitutes the copy with the load instructions defining the whole register now.
This is fine, because the register was undefined, but if this happens we need to update the sub-register liveness to keep track that there is a dead definition of the lanes that weren't loaded previously.

Repository:
  rL LLVM

http://reviews.llvm.org/D16693

Files:
  lib/CodeGen/MachineVerifier.cpp
  lib/CodeGen/RegisterCoalescer.cpp

Index: lib/CodeGen/RegisterCoalescer.cpp
===================================================================
--- lib/CodeGen/RegisterCoalescer.cpp
+++ lib/CodeGen/RegisterCoalescer.cpp
@@ -999,6 +999,38 @@
 
     updateRegDefsUses(DstReg, DstReg, DstIdx);
     NewMI->getOperand(0).setSubReg(NewIdx);
+    // 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
+    // more than actually is used in the register.
+    // An example would be:
+    // vreg1 = LOAD CONSTANTS 5, 8 ; Loading both 5 and 8 in different subregs
+    // ; Copying only part of the register here, but the rest is undef.
+    // vreg2:sub_16bit<def, read-undef> = COPY vreg1:sub_16bit
+    // ==>
+    // ; Materialize all the constants but only using one
+    // vreg2 = LOAD_CONSTANTS 5, 8
+    //
+    // at this point for the part that wasn't defined before we could have
+    // subranges missing the definition.
+    if (MRI->shouldTrackSubRegLiveness(DstReg)) {
+      LiveInterval &DstInt = LIS->getInterval(DstReg);
+      if (NewIdx == 0 && DstInt.hasSubRanges()) {
+        SlotIndex CurrIdx = LIS->getInstructionIndex(NewMI);
+        unsigned MaxMask = MRI->getMaxLaneMaskForVReg(DstReg);
+        for (auto &SR : DstInt.subranges()) {
+          if (!SR.liveAt(CurrIdx.getRegSlot())) {
+            SR.createDeadDef(CurrIdx.getRegSlot(), LIS->getVNInfoAllocator());
+          }
+          MaxMask &= ~SR.LaneMask;
+        }
+        if (MaxMask != 0) {
+          LiveInterval::SubRange *SR =
+              DstInt.createSubRange(LIS->getVNInfoAllocator(), MaxMask);
+          SR->createDeadDef(CurrIdx.getRegSlot(), LIS->getVNInfoAllocator());
+        }
+      }
+    }
   } else if (NewMI->getOperand(0).getReg() != CopyDstReg) {
     // The New instruction may be defining a sub-register of what's actually
     // been asked for. If so it must implicitly define the whole thing.
Index: lib/CodeGen/MachineVerifier.cpp
===================================================================
--- lib/CodeGen/MachineVerifier.cpp
+++ lib/CodeGen/MachineVerifier.cpp
@@ -1713,6 +1713,32 @@
 
   LaneBitmask Mask = 0;
   LaneBitmask MaxMask = MRI->getMaxLaneMaskForVReg(Reg);
+  // If the LiveInterval has subregister information we want to check that
+  // for each definition that defines the whole register every subrange has at
+  // least
+  // a dead definition for it.
+  if (LI.hasSubRanges()) {
+    for (auto DI = MRI->def_begin(Reg), DE = MachineRegisterInfo::def_end();
+         DI != DE; ++DI) {
+      MachineOperand &MO = *DI;
+      // We are defining the whole register, so all the subranges should have
+      // at least a dead definition.
+      if (MO.getSubReg() == 0) {
+        unsigned LiveMask = 0;
+        for (auto &SR : LI.subranges()) {
+          SlotIndex DefSlot =
+              LiveInts->getInstructionIndex(MO.getParent()).getRegSlot();
+          if (SR.liveAt(DefSlot))
+            LiveMask |= SR.LaneMask;
+        }
+        // If any lane didn't have a proper definition report.
+        if (LiveMask != MaxMask)
+          report("Lane mask defined but no sub range reports its definition",
+                 MF);
+      }
+    }
+  }
+
   for (const LiveInterval::SubRange &SR : LI.subranges()) {
     if ((Mask & SR.LaneMask) != 0) {
       report("Lane masks of sub ranges overlap in live interval", MF);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D16693.46308.patch
Type: text/x-patch
Size: 3477 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160128/7dbb6f38/attachment.bin>