[llvm] r266258 - ValueMapper: Resolve cycles on the new nodes

[Duncan P. N. Exon Smith via llvm-commits]

Author: dexonsmith
Date: Wed Apr 13 17:54:01 2016
New Revision: 266258

URL: http://llvm.org/viewvc/llvm-project?rev=266258&view=rev
Log:
ValueMapper: Resolve cycles on the new nodes

Fix a major bug from r265456.  Although it's now much rarer, ValueMapper
sometimes has to duplicate cycles.  The
might-transitively-reference-a-temporary counts don't decrement on their
own when there are cycles, and you need to call MDNode::resolveCycles to
fix it.

r265456 was checking the input nodes to see if they were unresolved.
This is useless; they should never be unresolved.  Instead we should
check the output nodes and resolve cycles on them.

Modified:
    llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp
    llvm/trunk/unittests/Transforms/Utils/ValueMapperTest.cpp

Modified: llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp?rev=266258&r1=266257&r2=266258&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp Wed Apr 13 17:54:01 2016
@@ -587,6 +587,7 @@ void MDNodeMapper::mapDistinctNodes() {
 
 void MDNodeMapper::mapUniquedNodes() {
   // Construct uniqued nodes, building forward references as necessary.
+  SmallVector<MDNode *, 16> CyclicNodes;
   for (auto *N : POT) {
     if (N->isDistinct())
       continue;
@@ -601,11 +602,12 @@ void MDNodeMapper::mapUniquedNodes() {
 
     TempMDNode ClonedN = D.Placeholder ? std::move(D.Placeholder) : N->clone();
     remapOperands(D, *ClonedN);
-    M.mapToMetadata(N, MDNode::replaceWithUniqued(std::move(ClonedN)));
+    CyclicNodes.push_back(MDNode::replaceWithUniqued(std::move(ClonedN)));
+    M.mapToMetadata(N, CyclicNodes.back());
   }
 
   // Resolve cycles.
-  for (auto *N : POT)
+  for (auto *N : CyclicNodes)
     if (!N->isResolved())
       N->resolveCycles();
 }

Modified: llvm/trunk/unittests/Transforms/Utils/ValueMapperTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Transforms/Utils/ValueMapperTest.cpp?rev=266258&r1=266257&r2=266258&view=diff
==============================================================================
--- llvm/trunk/unittests/Transforms/Utils/ValueMapperTest.cpp (original)
+++ llvm/trunk/unittests/Transforms/Utils/ValueMapperTest.cpp Wed Apr 13 17:54:01 2016
@@ -9,6 +9,7 @@
 
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/Function.h"
+#include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Metadata.h"
 #include "llvm/Transforms/Utils/ValueMapper.h"
@@ -63,6 +64,47 @@ TEST(ValueMapperTest, MapMetadataCycle)
   }
 }
 
+TEST(ValueMapperTest, MapMetadataDuplicatedCycle) {
+  LLVMContext Context;
+  auto *PtrTy = Type::getInt8Ty(Context)->getPointerTo();
+  std::unique_ptr<GlobalVariable> G0 = llvm::make_unique<GlobalVariable>(
+      PtrTy, false, GlobalValue::ExternalLinkage, nullptr, "G0");
+  std::unique_ptr<GlobalVariable> G1 = llvm::make_unique<GlobalVariable>(
+      PtrTy, false, GlobalValue::ExternalLinkage, nullptr, "G1");
+
+  // Create a cycle that references G0.
+  MDNode *N0; // !0 = !{!1}
+  MDNode *N1; // !1 = !{!0, i8* @G0}
+  {
+    auto T0 = MDTuple::getTemporary(Context, nullptr);
+    Metadata *Ops1[] = {T0.get(), ConstantAsMetadata::get(G0.get())};
+    N1 = MDTuple::get(Context, Ops1);
+    T0->replaceOperandWith(0, N1);
+    N0 = MDNode::replaceWithUniqued(std::move(T0));
+  }
+
+  // Resolve N0 and N1.
+  ASSERT_FALSE(N0->isResolved());
+  ASSERT_FALSE(N1->isResolved());
+  N0->resolveCycles();
+  ASSERT_TRUE(N0->isResolved());
+  ASSERT_TRUE(N1->isResolved());
+
+  // Seed the value map to map G0 to G1 and map the nodes.  The output should
+  // have new nodes that reference G1 (instead of G0).
+  ValueToValueMapTy VM;
+  VM[G0.get()] = G1.get();
+  MDNode *MappedN0 = MapMetadata(N0, VM);
+  MDNode *MappedN1 = MapMetadata(N1, VM);
+  EXPECT_NE(N0, MappedN0);
+  EXPECT_NE(N1, MappedN1);
+  EXPECT_EQ(ConstantAsMetadata::get(G1.get()), MappedN1->getOperand(1));
+
+  // Check that the output nodes are resolved.
+  EXPECT_TRUE(MappedN0->isResolved());
+  EXPECT_TRUE(MappedN1->isResolved());
+}
+
 TEST(ValueMapperTest, MapMetadataUnresolved) {
   LLVMContext Context;
   TempMDTuple T = MDTuple::getTemporary(Context, None);