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

[Matthias Braun via llvm-commits]

MatzeB accepted this revision.
MatzeB added a comment.
This revision is now accepted and ready to land.

I've looked at the rematerialization code am convinced now that we can safely rematerialize a full vreg def even if we only use parts of it later.

So this LGTM with the comments below addressed.


================
Comment at: lib/CodeGen/RegisterCoalescer.cpp:1021
@@ +1020,3 @@
+      for (LiveInterval::SubRange &SR : DstInt.subranges()) {
+        if (!SR.liveAt(CurrIdx.getRegSlot()))
+          SR.createDeadDef(CurrIdx.getRegSlot(), LIS->getVNInfoAllocator());
----------------
You should move the DefIndex definition (from below) up and use it everywhere where you currently use CurrIdx.getRegSlot().

================
Comment at: lib/CodeGen/RegisterCoalescer.cpp:1027-1028
@@ +1026,4 @@
+        LiveInterval::SubRange *SR =
+            DstInt.createSubRange(LIS->getVNInfoAllocator(), MaxMask);
+        SR->createDeadDef(CurrIdx.getRegSlot(), LIS->getVNInfoAllocator());
+        SlotIndex DefIndex = CurrIdx.getRegSlot(NewMI->getOperand(0).isEarlyClobber());
----------------
LIS->getVNInfoAllocator() is repeated 3 times and could be pulled into a variable.

================
Comment at: lib/CodeGen/RegisterCoalescer.cpp:1030
@@ +1029,3 @@
+        SlotIndex DefIndex = CurrIdx.getRegSlot(NewMI->getOperand(0).isEarlyClobber());
+        SR->createDeadDef(DefIndex, LIS->getVNInfoAllocator());
+      }
----------------
This is a 2nd createDeadDef here, you only need one (at DefIndex)!


Repository:
  rL LLVM

http://reviews.llvm.org/D16693