[llvm-commits] [llvm] r66976 - in /llvm/trunk: lib/CodeGen/PHIElimination.cpp lib/Transforms/Scalar/CodeGenPrepare.cpp test/CodeGen/X86/2009-03-13-PHIElimBug.ll test/CodeGen/X86/split-eh-lpad-edges.ll

[Duncan Sands]

How about this?

Ciao,

Duncan.

Index: llvm/lib/CodeGen/PHIElimination.cpp
===================================================================
--- llvm.orig/lib/CodeGen/PHIElimination.cpp	2009-03-15 20:41:40.000000000 +0100
+++ llvm/lib/CodeGen/PHIElimination.cpp	2009-03-15 21:45:50.000000000 +0100
@@ -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,12 @@
     ///
     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);
 
     typedef std::pair<const MachineBasicBlock*, unsigned> BBVRegPair;
     typedef std::map<BBVRegPair, unsigned> VRegPHIUse;
@@ -144,37 +149,51 @@
   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.
+  while (InsertPoint != MBB.end() &&
+         InsertPoint->getOpcode() == TargetInstrInfo::PHI)
+    ++InsertPoint;
+
+  return InsertPoint;
 }
 
 /// LowerAtomicPHINode - Lower the PHI node at the top of the specified block,
@@ -273,10 +292,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);