During PHI elimination, split critical edges that move copies out of loops.
authorDaniel Jasper <djasper@google.com>
Tue, 3 Mar 2015 10:23:11 +0000 (10:23 +0000)
committerDaniel Jasper <djasper@google.com>
Tue, 3 Mar 2015 10:23:11 +0000 (10:23 +0000)
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 to make the wrong assumption that
RegisterCoalescer won't be able to coalesce those copies later.

This change hides the new behavior behind -no-phi-elim-live-out-early-exit
as it currently breaks four tests:
 * Assertion in:
     CodeGen/Hexagon/hwloop-cleanup.ll
 * Worse code in:
     CodeGen/X86/coalescer-commute4.ll
     CodeGen/X86/phys_subreg_coalesce-2.ll
     CodeGen/X86/zlib-longest-match.ll
   The root cause here seems to be that the heuristic that determines
   the visitation order in RegisterCoalescer gets less lucky.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@231064 91177308-0d34-0410-b5e6-96231b3b80d8

lib/CodeGen/PHIElimination.cpp
test/CodeGen/X86/phielim-split.ll

index def2e3d48ac0cfe01e1b90acbb90445533193dfb..7098057c1c54937683da211b3c091e3ffd301afb 100644 (file)
@@ -46,6 +46,10 @@ SplitAllCriticalEdges("phi-elim-split-all-critical-edges", cl::init(false),
                       cl::Hidden, cl::desc("Split all critical edges during "
                                            "PHI elimination"));
 
+static cl::opt<bool> NoPhiElimLiveOutEarlyExit(
+    "no-phi-elim-live-out-early-exit", cl::init(false), cl::Hidden,
+    cl::desc("Do not use an early exit if isLiveOutPastPHIs returns true."));
+
 namespace {
   class PHIElimination : public MachineFunctionPass {
     MachineRegisterInfo *MRI; // Machine register information
@@ -573,12 +577,14 @@ bool PHIElimination::SplitPHIEdges(MachineFunction &MF,
       // 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)
+      bool ShouldSplit = isLiveOutPastPHIs(Reg, PreMBB);
+      if (!ShouldSplit && !NoPhiElimLiveOutEarlyExit)
         continue;
-
-      DEBUG(dbgs() << PrintReg(Reg) << " live-out before critical edge BB#"
-                   << PreMBB->getNumber() << " -> BB#" << MBB.getNumber()
-                   << ": " << *BBI);
+      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 +594,7 @@ bool PHIElimination::SplitPHIEdges(MachineFunction &MF,
       // 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 +609,7 @@ bool PHIElimination::SplitPHIEdges(MachineFunction &MF,
         // 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");
index 148b8033161cd155e32a246933240bd4299c996a..423ef0486ac7486dd0ae881720417685449f504f 100644 (file)
@@ -1,4 +1,4 @@
-; RUN: llc < %s -verify-machineinstrs | FileCheck %s
+; RUN: llc < %s -verify-machineinstrs -no-phi-elim-live-out-early-exit | FileCheck %s
 target triple = "x86_64-apple-macosx10.8.0"
 
 ; The critical edge from for.cond to if.end2 should be split to avoid injecting
@@ -28,3 +28,40 @@ if.end2:                                          ; preds = %for.cond, %entry
   %add = add nsw i32 %r.0, %b
   ret i32 %add
 }
+
+; CHECK: split_live_out
+; CHECK: %while.body
+; CHECK: cmp
+; CHECK-NEXT: ja
+define i8* @split_live_out(i32 %value, i8* %target) nounwind uwtable readonly ssp {
+entry:
+  %cmp10 = icmp ugt i32 %value, 127
+  br i1 %cmp10, label %while.body.preheader, label %while.end
+
+while.body.preheader:                             ; preds = %entry
+  br label %while.body
+
+while.body:                                       ; preds = %while.body.preheader, %while.body
+  %target.addr.012 = phi i8* [ %incdec.ptr, %while.body ], [ %target, %while.body.preheader ]
+  %value.addr.011 = phi i32 [ %shr, %while.body ], [ %value, %while.body.preheader ]
+  %or = or i32 %value.addr.011, 128
+  %conv = trunc i32 %or to i8
+  store i8 %conv, i8* %target.addr.012, align 1
+  %shr = lshr i32 %value.addr.011, 7
+  %incdec.ptr = getelementptr inbounds i8, i8* %target.addr.012, i64 1
+  %cmp = icmp ugt i32 %value.addr.011, 16383
+  br i1 %cmp, label %while.body, label %while.end.loopexit
+
+while.end.loopexit:                               ; preds = %while.body
+  %incdec.ptr.lcssa = phi i8* [ %incdec.ptr, %while.body ]
+  %shr.lcssa = phi i32 [ %shr, %while.body ]
+  br label %while.end
+
+while.end:                                        ; preds = %while.end.loopexit, %entry
+  %target.addr.0.lcssa = phi i8* [ %target, %entry ], [ %incdec.ptr.lcssa, %while.end.loopexit ]
+  %value.addr.0.lcssa = phi i32 [ %value, %entry ], [ %shr.lcssa, %while.end.loopexit ]
+  %conv1 = trunc i32 %value.addr.0.lcssa to i8
+  store i8 %conv1, i8* %target.addr.0.lcssa, align 1
+  %incdec.ptr3 = getelementptr inbounds i8, i8* %target.addr.0.lcssa, i64 1
+  ret i8* %incdec.ptr3
+}