[PATCH] In LICM, don't create more than one copy of an instruction per loop exit block when sinking.

[Chandler Carruth]

What Hal said, plus one more request. =] Patch looks good otherwise.

================
Comment at: lib/Transforms/Scalar/LICM.cpp:567-597
@@ -563,26 +566,33 @@
 
-    Instruction *New = I.clone();
-    ExitBlock->getInstList().insert(ExitBlock->getFirstInsertionPt(), New);
-    if (!I.getName().empty())
-      New->setName(I.getName() + ".le");
-
-    // Build LCSSA PHI nodes for any in-loop operands. Note that this is
-    // particularly cheap because we can rip off the PHI node that we're
-    // replacing for the number and blocks of the predecessors.
-    // OPT: If this shows up in a profile, we can instead finish sinking all
-    // invariant instructions, and then walk their operands to re-establish
-    // LCSSA. That will eliminate creating PHI nodes just to nuke them when
-    // sinking bottom-up.
-    for (User::op_iterator OI = New->op_begin(), OE = New->op_end(); OI != OE;
-         ++OI)
-      if (Instruction *OInst = dyn_cast<Instruction>(*OI))
-        if (Loop *OLoop = LI->getLoopFor(OInst->getParent()))
-          if (!OLoop->contains(PN)) {
-            PHINode *OpPN = PHINode::Create(
-                OInst->getType(), PN->getNumIncomingValues(),
-                OInst->getName() + ".lcssa", ExitBlock->begin());
-            for (unsigned i = 0, e = PN->getNumIncomingValues(); i != e; ++i)
-              OpPN->addIncoming(OInst, PN->getIncomingBlock(i));
-            *OI = OpPN;
-          }
+    Instruction *New;
+    auto It = SunkCopies.find(ExitBlock);
+    if (It != SunkCopies.end()) {
+      New = It->second;
+    } else {
+      New = I.clone();
+      SunkCopies[ExitBlock] = New;
+      ExitBlock->getInstList().insert(ExitBlock->getFirstInsertionPt(), New);
+      if (!I.getName().empty())
+        New->setName(I.getName() + ".le");
+
+      // Build LCSSA PHI nodes for any in-loop operands. Note that this is
+      // particularly cheap because we can rip off the PHI node that we're
+      // replacing for the number and blocks of the predecessors.
+      // OPT: If this shows up in a profile, we can instead finish sinking all
+      // invariant instructions, and then walk their operands to re-establish
+      // LCSSA. That will eliminate creating PHI nodes just to nuke them when
+      // sinking bottom-up.
+      for (User::op_iterator OI = New->op_begin(), OE = New->op_end(); OI != OE;
+           ++OI)
+        if (Instruction *OInst = dyn_cast<Instruction>(*OI))
+          if (Loop *OLoop = LI->getLoopFor(OInst->getParent()))
+            if (!OLoop->contains(PN)) {
+              PHINode *OpPN = PHINode::Create(
+                  OInst->getType(), PN->getNumIncomingValues(),
+                  OInst->getName() + ".lcssa", ExitBlock->begin());
+              for (unsigned i = 0, e = PN->getNumIncomingValues(); i != e; ++i)
+                OpPN->addIncoming(OInst, PN->getIncomingBlock(i));
+              *OI = OpPN;
+            }
+    }
 
----------------
I would factor some part of this into a helper function, there is too much deeply nested conditional logic here. Maybe the right thing to do is to factor out the logic handling the build up a clone in the exit block?

(But do that in a separate patch either before or after landing th ebugfix)

http://reviews.llvm.org/D4252