[PATCH] During PHI elimination, split critical edges that move copies out of loops

[Daniel Jasper]

Hi chandlerc, qcolombet,

This prevents the behavior observed in llvm.org/PR22369. I am not sure whether I am reading the code correctly, but the early exit based on isLiveOutPastPHIs() seems like a bug as it prevents the nice loop-splitting logic below.

This is still work in progress as it currently breaks a few tests. Mainly, it seems to influence which copies are local and which are non-local, which influences the order in which the RegisterCoalescer visits copies. Coalescing the wrong registers first has unwanted side-effects.

http://reviews.llvm.org/D8016

Files:
  lib/CodeGen/PHIElimination.cpp

Index: lib/CodeGen/PHIElimination.cpp
===================================================================
--- lib/CodeGen/PHIElimination.cpp
+++ lib/CodeGen/PHIElimination.cpp
@@ -573,12 +573,14 @@
       // there is a risk it may not be coalesced away.
       //
       // If the copy would be a kill, there is no need to split the edge.
-      if (!isLiveOutPastPHIs(Reg, PreMBB) && !SplitAllCriticalEdges)
-        continue;
-
-      DEBUG(dbgs() << PrintReg(Reg) << " live-out before critical edge BB#"
-                   << PreMBB->getNumber() << " -> BB#" << MBB.getNumber()
-                   << ": " << *BBI);
+      // if (!isLiveOutPastPHIs(Reg, PreMBB) && !SplitAllCriticalEdges)
+      //   continue;
+      bool ShouldSplit = isLiveOutPastPHIs(Reg, PreMBB);
+      if (ShouldSplit) {
+        DEBUG(dbgs() << PrintReg(Reg) << " live-out before critical edge BB#"
+                     << PreMBB->getNumber() << " -> BB#" << MBB.getNumber()
+                     << ": " << *BBI);
+      }
 
       // If Reg is not live-in to MBB, it means it must be live-in to some
       // other PreMBB successor, and we can avoid the interference by splitting
@@ -588,7 +590,7 @@
       // is likely to be left after coalescing. If we are looking at a loop
       // exiting edge, split it so we won't insert code in the loop, otherwise
       // don't bother.
-      bool ShouldSplit = !isLiveIn(Reg, &MBB) || SplitAllCriticalEdges;
+      ShouldSplit = ShouldSplit && !isLiveIn(Reg, &MBB);
 
       // Check for a loop exiting edge.
       if (!ShouldSplit && CurLoop != PreLoop) {
@@ -603,7 +605,7 @@
         // Split unless this edge is entering CurLoop from an outer loop.
         ShouldSplit = PreLoop && !PreLoop->contains(CurLoop);
       }
-      if (!ShouldSplit)
+      if (!ShouldSplit && !SplitAllCriticalEdges)
         continue;
       if (!PreMBB->SplitCriticalEdge(&MBB, this)) {
         DEBUG(dbgs() << "Failed to split critical edge.\n");

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D8016.21037.patch
Type: text/x-patch
Size: 1964 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150302/b1ef7c68/attachment.bin>