[llvm-commits] [llvm] r67049 - in /llvm/trunk: lib/CodeGen/PHIElimination.cpp test/CodeGen/X86/2009-03-13-PHIElimBug.ll test/CodeGen/X86/2009-03-16-PHIElimInLPad.ll
Bill Wendling
isanbard at gmail.com
Mon Mar 16 13:23:40 PDT 2009
Duncan,
This test failed because of this patch. I'm going to temporarily
revert it. Please investigate it.
Thanks!
-bw
Running /Volumes/Sandbox/Buildbot/llvm/full-llvm/build/llvm.src/test/CodeGen/X86/dg.exp
...
FAIL: /Volumes/Sandbox/Buildbot/llvm/full-llvm/build/llvm.src/test/CodeGen/X86/2009-03-13-PHIElimBug.ll
for PR3784
Failed with exit(1) at line 1
while running: llvm-as <
/Volumes/Sandbox/Buildbot/llvm/full-llvm/build/llvm.src/test/CodeGen/X86/2009-03-13-PHIElimBug.ll
| llc -march=x86 | /usr/bin/grep -A 2 {call f} | /usr/bin/grep movl
child process exited abnormally
On Mon, Mar 16, 2009 at 11:58 AM, Duncan Sands <baldrick at free.fr> wrote:
> Author: baldrick
> Date: Mon Mar 16 14:58:38 2009
> New Revision: 67049
>
> URL: http://llvm.org/viewvc/llvm-project?rev=67049&view=rev
> Log:
> Tweak the fix for PR3784: be less sensitive about just
> how invokes are set up. The fix could be disturbed by
> register copies coming after the EH_LABEL, and also didn't
> behave quite right when it was the invoke result that
> was used in a phi node. Also (see new testcase) fix
> another phi elimination bug while there: register copies
> in the landing pad need to come after the EH_LABEL, because
> that's where execution branches to when unwinding. If they
> come before the EH_LABEL then they will never be executed...
> Also tweak the original testcase so it doesn't use a no-longer
> existing counter.
> The accumulated phi elimination changes fix two of seven Ada
> testsuite failures that turned up after landing pad critical
> edge splitting was turned off. So there's probably more to come.
>
> Added:
> llvm/trunk/test/CodeGen/X86/2009-03-16-PHIElimInLPad.ll
> Modified:
> llvm/trunk/lib/CodeGen/PHIElimination.cpp
> llvm/trunk/test/CodeGen/X86/2009-03-13-PHIElimBug.ll
>
> Modified: llvm/trunk/lib/CodeGen/PHIElimination.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/PHIElimination.cpp?rev=67049&r1=67048&r2=67049&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/PHIElimination.cpp (original)
> +++ llvm/trunk/lib/CodeGen/PHIElimination.cpp Mon Mar 16 14:58:38 2009
> @@ -14,6 +14,8 @@
> //===----------------------------------------------------------------------===//
>
> #define DEBUG_TYPE "phielim"
> +#include "llvm/BasicBlock.h"
> +#include "llvm/Instructions.h"
> #include "llvm/CodeGen/LiveVariables.h"
> #include "llvm/CodeGen/Passes.h"
> #include "llvm/CodeGen/MachineFunctionPass.h"
> @@ -31,7 +33,6 @@
> using namespace llvm;
>
> STATISTIC(NumAtomic, "Number of atomic phis lowered");
> -STATISTIC(NumEH, "Number of EH try blocks skipped");
>
> namespace {
> class VISIBILITY_HIDDEN PNE : public MachineFunctionPass {
> @@ -66,8 +67,25 @@
> ///
> void analyzePHINodes(const MachineFunction& Fn);
>
> - void WalkPassEHTryRange(MachineBasicBlock &MBB,
> - MachineBasicBlock::iterator &I, unsigned SrcReg);
> + // FindCopyInsertPoint - Find a safe place in MBB to insert a copy from
> + // SrcReg. This needs to be after any def or uses of SrcReg, but before
> + // any subsequent point where control flow might jump out of the basic
> + // block.
> + MachineBasicBlock::iterator FindCopyInsertPoint(MachineBasicBlock &MBB,
> + unsigned SrcReg);
> +
> + // SkipPHIsAndLabels - Copies need to be inserted after phi nodes and
> + // also after any exception handling labels: in landing pads execution
> + // starts at the label, so any copies placed before it won't be executed!
> + MachineBasicBlock::iterator SkipPHIsAndLabels(MachineBasicBlock &MBB,
> + MachineBasicBlock::iterator I) {
> + // Rather than assuming that EH labels come before other kinds of labels,
> + // just skip all labels.
> + while (I != MBB.end() &&
> + (I->getOpcode() == TargetInstrInfo::PHI || I->isLabel()))
> + ++I;
> + return I;
> + }
>
> typedef std::pair<const MachineBasicBlock*, unsigned> BBVRegPair;
> typedef std::map<BBVRegPair, unsigned> VRegPHIUse;
> @@ -120,10 +138,7 @@
>
> // 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.begin();
> - while (AfterPHIsIt != MBB.end() &&
> - AfterPHIsIt->getOpcode() == TargetInstrInfo::PHI)
> - ++AfterPHIsIt; // Skip over all of the PHI nodes...
> + MachineBasicBlock::iterator AfterPHIsIt = SkipPHIsAndLabels(MBB, MBB.begin());
>
> while (MBB.front().getOpcode() == TargetInstrInfo::PHI)
> LowerAtomicPHINode(MBB, AfterPHIsIt);
> @@ -144,37 +159,47 @@
> return true;
> }
>
> -void PNE::WalkPassEHTryRange(MachineBasicBlock &MBB,
> - MachineBasicBlock::iterator &I, unsigned SrcReg) {
> - if (I == MBB.begin())
> - return;
> - MachineBasicBlock::iterator PI = prior(I);
> - if (PI->getOpcode() != TargetInstrInfo::EH_LABEL)
> - return;
> -
> - // Trying to walk pass the EH try range. If we run into a use instruction,
> - // we want to insert the copy there.
> - SmallPtrSet<MachineInstr*, 4> UsesInMBB;
> - for (MachineRegisterInfo::use_iterator UI = MRI->use_begin(SrcReg),
> - UE = MRI->use_end(); UI != UE; ++UI) {
> - MachineInstr *UseMI = &*UI;
> - if (UseMI->getParent() == &MBB)
> - UsesInMBB.insert(UseMI);
> +// FindCopyInsertPoint - Find a safe place in MBB to insert a copy from SrcReg.
> +// This needs to be after any def or uses of SrcReg, but before any subsequent
> +// point where control flow might jump out of the basic block.
> +MachineBasicBlock::iterator PNE::FindCopyInsertPoint(MachineBasicBlock &MBB,
> + unsigned SrcReg) {
> + // Handle the trivial case trivially.
> + if (MBB.empty())
> + return MBB.begin();
> +
> + // If this basic block does not contain an invoke, then control flow always
> + // reaches the end of it, so place the copy there. The logic below works in
> + // this case too, but is more expensive.
> + if (!isa<InvokeInst>(MBB.getBasicBlock()->getTerminator()))
> + return MBB.getFirstTerminator();
> +
> + // Discover any definition/uses in this basic block.
> + SmallPtrSet<MachineInstr*, 8> DefUsesInMBB;
> + for (MachineRegisterInfo::reg_iterator RI = MRI->reg_begin(SrcReg),
> + RE = MRI->reg_end(); RI != RE; ++RI) {
> + MachineInstr *DefUseMI = &*RI;
> + if (DefUseMI->getParent() == &MBB)
> + DefUsesInMBB.insert(DefUseMI);
> }
>
> - while (PI != MBB.begin()) {
> - --PI;
> - if (PI->getOpcode() == TargetInstrInfo::EH_LABEL) {
> - ++NumEH;
> - I = PI;
> - return;
> - } else if (UsesInMBB.count(&*PI)) {
> - ++NumEH;
> - I = next(PI);
> - return;
> - }
> + MachineBasicBlock::iterator InsertPoint;
> + if (DefUsesInMBB.empty()) {
> + // No def/uses. Insert the copy at the start of the basic block.
> + InsertPoint = MBB.begin();
> + } else if (DefUsesInMBB.size() == 1) {
> + // Insert the copy immediately after the definition/use.
> + InsertPoint = *DefUsesInMBB.begin();
> + ++InsertPoint;
> + } else {
> + // Insert the copy immediately after the last definition/use.
> + InsertPoint = MBB.end();
> + while (!DefUsesInMBB.count(&*--InsertPoint)) {}
> + ++InsertPoint;
> }
> - return;
> +
> + // Make sure the copy goes after any phi nodes however.
> + return SkipPHIsAndLabels(MBB, InsertPoint);
> }
>
> /// LowerAtomicPHINode - Lower the PHI node at the top of the specified block,
> @@ -273,10 +298,7 @@
>
> // Find a safe location to insert the copy, this may be the first terminator
> // in the block (or end()).
> - MachineBasicBlock::iterator InsertPos = opBlock.getFirstTerminator();
> -
> - // Walk pass EH try range if needed.
> - WalkPassEHTryRange(opBlock, InsertPos, SrcReg);
> + MachineBasicBlock::iterator InsertPos = FindCopyInsertPoint(opBlock, SrcReg);
>
> // Insert the copy.
> TII->copyRegToReg(opBlock, InsertPos, IncomingReg, SrcReg, RC, RC);
>
> Modified: llvm/trunk/test/CodeGen/X86/2009-03-13-PHIElimBug.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/2009-03-13-PHIElimBug.ll?rev=67049&r1=67048&r2=67049&view=diff
>
> ==============================================================================
> --- llvm/trunk/test/CodeGen/X86/2009-03-13-PHIElimBug.ll (original)
> +++ llvm/trunk/test/CodeGen/X86/2009-03-13-PHIElimBug.ll Mon Mar 16 14:58:38 2009
> @@ -1,37 +1,28 @@
> -; RUN: llvm-as < %s | llc -mtriple=i386-pc-linux-gnu -stats |& grep phielim | grep {Number of EH try blocks skipped} | grep 4
> +; RUN: llvm-as < %s | llc -march=x86 | grep -A 2 {call f} | grep movl
> +; Check the register copy comes after the call to f and before the call to g
> ; PR3784
>
> - %struct.c38002a__arr___XUB = type { i32, i32 }
> - %struct.c38002a__arr_name = type { [0 x i32]*, %struct.c38002a__arr___XUB* }
> - %struct.c38002a__rec = type { i32, %struct.c38002a__arr_name }
> +declare i32 @f()
>
> -define void @_ada_c38002a() {
> -entry:
> - %0 = invoke i8* @__gnat_malloc(i32 12)
> - to label %invcont unwind label %lpad ; <i8*> [#uses=0]
> +declare i32 @g()
>
> -invcont: ; preds = %entry
> - %1 = invoke i8* @__gnat_malloc(i32 20)
> - to label %invcont1 unwind label %lpad ; <i8*> [#uses=0]
> -
> -invcont1: ; preds = %invcont
> - %2 = invoke i32 @report__ident_int(i32 2)
> - to label %.noexc unwind label %lpad ; <i32> [#uses=0]
> -
> -.noexc: ; preds = %invcont1
> - %3 = invoke i32 @report__ident_int(i32 3)
> - to label %.noexc88 unwind label %lpad ; <i32> [#uses=0]
> -
> -.noexc88: ; preds = %.noexc
> - unreachable
> -
> -lpad: ; preds = %.noexc, %invcont1, %invcont, %entry
> - %r.0 = phi %struct.c38002a__rec* [ null, %entry ], [ null, %invcont ], [ null, %invcont1 ], [ null, %.noexc ] ; <%struct.c38002a__rec*> [#uses=1]
> - %4 = getelementptr %struct.c38002a__rec* %r.0, i32 0, i32 0 ; <i32*> [#uses=1]
> - %5 = load i32* %4, align 4 ; <i32> [#uses=0]
> - ret void
> +define i32 @phi() {
> +entry:
> + %a = call i32 @f() ; <i32> [#uses=1]
> + %b = invoke i32 @g()
> + to label %cont unwind label %lpad ; <i32> [#uses=1]
> +
> +cont: ; preds = %entry
> + %x = phi i32 [ %b, %entry ] ; <i32> [#uses=0]
> + %aa = call i32 @g() ; <i32> [#uses=1]
> + %bb = invoke i32 @g()
> + to label %cont2 unwind label %lpad ; <i32> [#uses=1]
> +
> +cont2: ; preds = %cont
> + %xx = phi i32 [ %bb, %cont ] ; <i32> [#uses=1]
> + ret i32 %xx
> +
> +lpad: ; preds = %cont, %entry
> + %y = phi i32 [ %a, %entry ], [ %aa, %cont ] ; <i32> [#uses=1]
> + ret i32 %y
> }
> -
> -declare i32 @report__ident_int(i32)
> -
> -declare i8* @__gnat_malloc(i32)
>
> Added: llvm/trunk/test/CodeGen/X86/2009-03-16-PHIElimInLPad.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/2009-03-16-PHIElimInLPad.ll?rev=67049&view=auto
>
> ==============================================================================
> --- llvm/trunk/test/CodeGen/X86/2009-03-16-PHIElimInLPad.ll (added)
> +++ llvm/trunk/test/CodeGen/X86/2009-03-16-PHIElimInLPad.ll Mon Mar 16 14:58:38 2009
> @@ -0,0 +1,21 @@
> +; RUN: llvm-as < %s | llc -march=x86 | grep -A 1 lpad | grep Llabel
> +; Check that register copies in the landing pad come after the EH_LABEL
> +
> +declare i32 @f()
> +
> +define i32 @phi(i32 %x) {
> +entry:
> + %a = invoke i32 @f()
> + to label %cont unwind label %lpad ; <i32> [#uses=1]
> +
> +cont: ; preds = %entry
> + %b = invoke i32 @f()
> + to label %cont2 unwind label %lpad ; <i32> [#uses=1]
> +
> +cont2: ; preds = %cont
> + ret i32 %b
> +
> +lpad: ; preds = %cont, %entry
> + %v = phi i32 [ %x, %entry ], [ %a, %cont ] ; <i32> [#uses=1]
> + ret i32 %v
> +}
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
More information about the llvm-commits
mailing list