From 867bfcd546c1fffd20678dc31b04e9e94750c882 Mon Sep 17 00:00:00 2001 From: Cameron Zwarich Date: Mon, 1 Jul 2013 19:42:46 +0000 Subject: [PATCH] Fix PR16508. When phis get lowered, destination copies are inserted using an iterator that is determined once for all phis in the block, which BuildMI interprets as a request to insert an instruction directly before the iterator. In the case of a cyclic phi, source copies may also be inserted directly before this iterator, which can cause source copies to be inserted before destination copies. The fix is to keep an iterator to the last phi and then advance it while lowering each phi in order to insert destination copies directly after the phis. llvm-svn: 185363 --- llvm/lib/CodeGen/PHIElimination.cpp | 12 ++++++---- llvm/test/CodeGen/PowerPC/2013-07-01-PHIElimBug.ll | 28 ++++++++++++++++++++++ 2 files changed, 36 insertions(+), 4 deletions(-) create mode 100644 llvm/test/CodeGen/PowerPC/2013-07-01-PHIElimBug.ll diff --git a/llvm/lib/CodeGen/PHIElimination.cpp b/llvm/lib/CodeGen/PHIElimination.cpp index 5584708..985967e 100644 --- a/llvm/lib/CodeGen/PHIElimination.cpp +++ b/llvm/lib/CodeGen/PHIElimination.cpp @@ -66,7 +66,7 @@ namespace { /// bool EliminatePHINodes(MachineFunction &MF, MachineBasicBlock &MBB); void LowerPHINode(MachineBasicBlock &MBB, - MachineBasicBlock::iterator AfterPHIsIt); + MachineBasicBlock::iterator LastPHIIt); /// analyzePHINodes - Gather information about the PHI nodes in /// here. In particular, we want to map the number of uses of a virtual @@ -185,10 +185,11 @@ bool PHIElimination::EliminatePHINodes(MachineFunction &MF, // Get an iterator to the first instruction after the last PHI node (this may // also be the end of the basic block). - MachineBasicBlock::iterator AfterPHIsIt = MBB.SkipPHIsAndLabels(MBB.begin()); + MachineBasicBlock::iterator LastPHIIt = + prior(MBB.SkipPHIsAndLabels(MBB.begin())); while (MBB.front().isPHI()) - LowerPHINode(MBB, AfterPHIsIt); + LowerPHINode(MBB, LastPHIIt); return true; } @@ -218,8 +219,11 @@ static bool isSourceDefinedByImplicitDef(const MachineInstr *MPhi, /// LowerPHINode - Lower the PHI node at the top of the specified block, /// void PHIElimination::LowerPHINode(MachineBasicBlock &MBB, - MachineBasicBlock::iterator AfterPHIsIt) { + MachineBasicBlock::iterator LastPHIIt) { ++NumLowered; + + MachineBasicBlock::iterator AfterPHIsIt = next(LastPHIIt); + // Unlink the PHI node from the basic block, but don't delete the PHI yet. MachineInstr *MPhi = MBB.remove(MBB.begin()); diff --git a/llvm/test/CodeGen/PowerPC/2013-07-01-PHIElimBug.ll b/llvm/test/CodeGen/PowerPC/2013-07-01-PHIElimBug.ll new file mode 100644 index 0000000..635062b8 --- /dev/null +++ b/llvm/test/CodeGen/PowerPC/2013-07-01-PHIElimBug.ll @@ -0,0 +1,28 @@ +; RUN: llc < %s -verify-machineinstrs | FileCheck %s + +target datalayout = "E-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-f128:128:128-v128:128:128-n32:64" +target triple = "powerpc64-unknown-linux-gnu" + +@g_51 = external global [8 x i32], align 4 + +; CHECK: func_7 + +; Function Attrs: nounwind +define fastcc void @func_7() #0 { +entry: + %arrayidx638 = getelementptr inbounds [3 x [1 x i32]]* undef, i64 0, i64 1, i64 0 + br i1 undef, label %for.cond940, label %if.end1018 + +for.cond940: ; preds = %for.cond940, %if.else876 + %l_655.1 = phi i32* [ getelementptr inbounds ([8 x i32]* @g_51, i64 0, i64 6), %entry ], [ %l_654.0, %for.cond940 ] + %l_654.0 = phi i32* [ null, %entry ], [ %arrayidx638, %for.cond940 ] + %exitcond = icmp eq i32 undef, 20 + br i1 %exitcond, label %if.end1018, label %for.cond940 + +if.end1018: ; preds = %for.end957, %for.end834 + %l_655.3.ph33 = phi i32* [ %l_655.1, %for.cond940 ], [ getelementptr inbounds ([8 x i32]* @g_51, i64 0, i64 6), %entry ] + store i32 0, i32* %l_655.3.ph33, align 4 + ret void +} + +attributes #0 = { nounwind "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf"="true" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "unsafe-fp-math"="false" "use-soft-float"="false" } -- 2.7.4