From bbaf4fd14c6aa79bf63572e0d1a4d2f7abe9f407 Mon Sep 17 00:00:00 2001 From: Daniel Jasper Date: Tue, 3 Mar 2015 10:23:11 +0000 Subject: [PATCH] During PHI elimination, split critical edges that move copies out of loops. 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 | 20 ++++++++++------ test/CodeGen/X86/phielim-split.ll | 39 ++++++++++++++++++++++++++++++- 2 files changed, 51 insertions(+), 8 deletions(-) diff --git a/lib/CodeGen/PHIElimination.cpp b/lib/CodeGen/PHIElimination.cpp index def2e3d48ac..7098057c1c5 100644 --- a/lib/CodeGen/PHIElimination.cpp +++ b/lib/CodeGen/PHIElimination.cpp @@ -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 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"); diff --git a/test/CodeGen/X86/phielim-split.ll b/test/CodeGen/X86/phielim-split.ll index 148b8033161..423ef0486ac 100644 --- a/test/CodeGen/X86/phielim-split.ll +++ b/test/CodeGen/X86/phielim-split.ll @@ -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 +} -- 2.34.1