IR: Drop uniquing for self-referencing MDNodes

It doesn't make sense to unique self-referencing nodes.  Drop uniquing
for them.

Note that `MDNode::intersect()` occasionally returns self-referencing
nodes.  Previously these would be returned by `MDNode::get()`.  I'm not
convinced this was intended behaviour -- to me it seems it should return
a node whose only operand is the self-reference -- but I don't know much
about alias scopes so I'm preserving it for now.

This is part of PR21532.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@223618 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/IR/Metadata.cpp b/lib/IR/Metadata.cpp
index e5dfaec74e592fc586a0f21d62fbbcee4203ced1..70415b01016618df5b6a160944588f56e438ef01 100644 (file)
--- a/lib/IR/Metadata.cpp
+++ b/lib/IR/Metadata.cpp
@@ -353,7 +353,9 @@ void MDNode::replaceOperand(MDNodeOperand *Op, Value *To) {
   // anymore.  This commonly occurs during destruction, and uniquing these
   // brings little reuse.  Also, this means we don't need to include
   // isFunctionLocal bits in the hash for MDNodes.
-  if (!To) {
+  //
+  // Also drop uniquing if this has a reference to itself.
+  if (!To || To == this) {
     setIsNotUniqued();
     return;
   }
@@ -406,6 +408,18 @@ MDNode *MDNode::intersect(MDNode *A, MDNode *B) {
       }
   }
 
+  // Handle alias scope self-references specially.
+  //
+  // FIXME: This preserves long-standing behaviour, but is it really the right
+  // behaviour?  Or was that an unintended side-effect of node uniquing?
+  if (!Vals.empty())
+    if (MDNode *N = dyn_cast_or_null<MDNode>(Vals[0]))
+      if (N->getNumOperands() == Vals.size() && N == N->getOperand(0)) {
+        for (unsigned I = 1, E = Vals.size(); I != E; ++I)
+          if (Vals[I] != N->getOperand(I))
+            return MDNode::get(A->getContext(), Vals);
+        return N;
+      }
   return MDNode::get(A->getContext(), Vals);
 }
 

diff --git a/unittests/IR/MetadataTest.cpp b/unittests/IR/MetadataTest.cpp
index 1a6f3d2df1614370b39cce721d57d8201135f5d1..0e2599051fe1818f0b167431a2e0d962fb72a9b3 100644 (file)
--- a/unittests/IR/MetadataTest.cpp
+++ b/unittests/IR/MetadataTest.cpp
@@ -123,6 +123,46 @@ TEST_F(MDNodeTest, Delete) {
   delete I;
 }
 
+TEST_F(MDNodeTest, SelfReference) {
+  // !0 = metadata !{metadata !0}
+  // !1 = metadata !{metadata !0}
+  {
+    MDNode *Temp = MDNode::getTemporary(Context, None);
+    Value *Args[] = {Temp};
+    MDNode *Self = MDNode::get(Context, Args);
+    Self->replaceOperandWith(0, Self);
+    MDNode::deleteTemporary(Temp);
+    ASSERT_EQ(Self, Self->getOperand(0));
+
+    // Self-references should be distinct, so MDNode::get() should grab a
+    // uniqued node that references Self, not Self.
+    Args[0] = Self;
+    MDNode *Ref1 = MDNode::get(Context, Args);
+    MDNode *Ref2 = MDNode::get(Context, Args);
+    EXPECT_NE(Self, Ref1);
+    EXPECT_EQ(Ref1, Ref2);
+  }
+
+  // !0 = metadata !{metadata !0, metadata !{}}
+  // !1 = metadata !{metadata !0, metadata !{}}
+  {
+    MDNode *Temp = MDNode::getTemporary(Context, None);
+    Value *Args[] = {Temp, MDNode::get(Context, None)};
+    MDNode *Self = MDNode::get(Context, Args);
+    Self->replaceOperandWith(0, Self);
+    MDNode::deleteTemporary(Temp);
+    ASSERT_EQ(Self, Self->getOperand(0));
+
+    // Self-references should be distinct, so MDNode::get() should grab a
+    // uniqued node that references Self, not Self itself.
+    Args[0] = Self;
+    MDNode *Ref1 = MDNode::get(Context, Args);
+    MDNode *Ref2 = MDNode::get(Context, Args);
+    EXPECT_NE(Self, Ref1);
+    EXPECT_EQ(Ref1, Ref2);
+  }
+}
+
 TEST(NamedMDNodeTest, Search) {
   LLVMContext Context;
   Constant *C = ConstantInt::get(Type::getInt32Ty(Context), 1);