[llvm-commits] [patch]: Fix regalloc issue with splitting range of spilling registers

[Jakob Stoklund Olesen]

On Dec 12, 2011, at 11:53 AM, Peter Cooper wrote:

> Hi
> 
> Please review the attached patch which fixes a register allocator issue that came up in clang self hosting.
> 
> The problem is that if we create new intervals for registers that are currently being spilled then the new intervals might not spill.  Any reads from the spilled location then won't get the value from the non-spilled locations.
> 
> The patch fixes this by avoiding creating the new intervals when they are for a spilling register.

Looks good!

--- lib/CodeGen/LiveRangeEdit.h	(revision 146134)
+++ lib/CodeGen/LiveRangeEdit.h	(working copy)
@@ -193,6 +193,7 @@
   /// and further dead efs to be eliminated.
   void eliminateDeadDefs(SmallVectorImpl<MachineInstr*> &Dead,
                          LiveIntervals&, VirtRegMap&,
+                         ArrayRef<unsigned> RegsBeingSpilled,
                          const TargetInstrInfo&);

Could you give RegsBeingSpilled a default value, please? Then you won't have to:

-  Edit->eliminateDeadDefs(Dead, LIS, VRM, TII);
+  Edit->eliminateDeadDefs(Dead, LIS, VRM, ArrayRef<unsigned>(), TII);
 }

Also add a comment explaining what the argument is for.
 
+    // don't create new intervals for a register being spilled.
+    // the new intervals would have to be spilled anyway so its not worth it
+    // also they currently aren't spilled so creating them and not spilling
+    // them results in incorrect code

Please capitalize correctly.

Thanks for tracking this down!

/jakob