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

Matthias Braun via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 2 13:20:57 PST 2016

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)!



More information about the llvm-commits mailing list