[Metadata] Use push_back() to append operands of MDTuples to other MDTuples

[wolfgangp]

This patch depends on D125998. When appending module flags to the module flags of another module (probably during module linking) we want to append the operands to the destination module's MDTuple instead of creating new ones. This should fix issue 51893.

[wolfgangp]

Use a better way to create a distinct node from an existing uniqued one and simplify dealing with AppendUnique module flags.

[dexonsmith]

LGTM once the prep commits land!

llvm/lib/Linker/IRMover.cpp

if (SrcBehaviorValue != DstBehaviorValue) {
                          SrcBehaviorValue == Module::Warning);
       if (!(MaxAndWarn || MinAndWarn))
         return stringErr("linking module flags '" + ID->getString() +
                          "': IDs have conflicting behaviors in '" +
                          SrcM->getModuleIdentifier() + "' and '" +
                          DstM.getModuleIdentifier() + "'");
     }
 
-    auto replaceDstValue = [&](MDNode *New) {
+    auto ensureDistinctOp = [&](MDNode *DstValue) {
+      assert(isa<MDTuple>(DstValue) &&
+             "Expected MDTuple when appending module flags");
+      MDTuple *DstValueTuple = dyn_cast<MDTuple>(DstValue);
+      if (DstValueTuple->isDistinct())
+        return DstValueTuple;
+      MDTuple *New = MDTuple::getDistinct(DstM.getContext(),
+                                          DstValue->operandsAsArrayRef());
       Metadata *FlagOps[] = {DstOp->getOperand(0), ID, New};
-      MDNode *Flag = MDNode::get(DstM.getContext(), FlagOps);
+      MDNode *Flag = MDTuple::getDistinct(DstM.getContext(), FlagOps);
       DstModFlags->setOperand(DstIndex, Flag);
       Flags[ID].first = Flag;
+      return New;
     };
 
     // Emit a warning if the values differ and either source or destination
     // request Warning behavior.
     if ((DstBehaviorValue == Module::Warning ||
          SrcBehaviorValue == Module::Warning) &&
         SrcOp->getOperand(2) != DstOp->getOperand(2)) {
       std::string Str;
for (unsigned I = 0, E = SrcModFlags->getNumOperands(); I != E; ++I) {
     }
     case Module::Warning: {
       break;
     }
     case Module::Max: {
       break;
     }
     case Module::Append: {
-      MDNode *DstValue = cast<MDNode>(DstOp->getOperand(2));
+      MDTuple *DstValue = ensureDistinctOp(cast<MDNode>(DstOp->getOperand(2)));
       MDNode *SrcValue = cast<MDNode>(SrcOp->getOperand(2));
-      SmallVector<Metadata *, 8> MDs;
-      MDs.reserve(DstValue->getNumOperands() + SrcValue->getNumOperands());
-      MDs.append(DstValue->op_begin(), DstValue->op_end());
-      MDs.append(SrcValue->op_begin(), SrcValue->op_end());
-
-      replaceDstValue(MDNode::get(DstM.getContext(), MDs));
+      for (const auto &O : SrcValue->operands())
+        DstValue->push_back(O);
       break;
     }
     case Module::AppendUnique: {
       SmallSetVector<Metadata *, 16> Elts;
-      MDNode *DstValue = cast<MDNode>(DstOp->getOperand(2));
+      MDTuple *DstValue = ensureDistinctOp(cast<MDNode>(DstOp->getOperand(2)));
       MDNode *SrcValue = cast<MDNode>(SrcOp->getOperand(2));
       Elts.insert(DstValue->op_begin(), DstValue->op_end());
       Elts.insert(SrcValue->op_begin(), SrcValue->op_end());
-
-      replaceDstValue(MDNode::get(DstM.getContext(),
-                                  makeArrayRef(Elts.begin(), Elts.end())));
+      for (const auto &O : SmallSetVector<Metadata *, 8>(
+               Elts.begin() + DstValue->getNumOperands(), Elts.end()))
+        DstValue->push_back(O);
       break;
     }
     }
 
   }
 
   // Check all of the requirements.
   for (unsigned I = 0, E = Requirements.size(); I != E; ++I) {

llvm/lib/Transforms/Instrumentation/CGProfile.cpp

addModuleFlags(Module &M,
   for (auto E : Counts) {
     Metadata *Vals[] = {ValueAsMetadata::get(E.first.first),
                         ValueAsMetadata::get(E.first.second),
                         MDB.createConstant(ConstantInt::get(
                             Type::getInt64Ty(Context), E.second))};
     Nodes.push_back(MDNode::get(Context, Vals));
   }
 
-  M.addModuleFlag(Module::Append, "CG Profile", MDNode::get(Context, Nodes));
+  M.addModuleFlag(Module::Append, "CG Profile",
+                  MDTuple::getDistinct(Context, Nodes));
   return true;
 }
 
 static bool runCGProfilePass(
     Module &M, function_ref<BlockFrequencyInfo &(Function &)> GetBFI,
     function_ref<TargetTransformInfo &(Function &)> GetTTI, bool LazyBFI) {
   MapVector<std::pair<Function *, Function *>, uint64_t> Counts;
   InstrProfSymtab Symtab;

llvm/test/Instrumentation/cgprofile.ll

 }
 
 !1 = !{!"function_entry_count", i64 32}
 !2 = !{!"branch_weights", i32 5, i32 10}
 !3 = !{!"VP", i32 0, i64 1600, i64 7651369219802541373, i64 1030, i64 -4377547752858689819, i64 410, i64 -6929281286627296573, i64 150, i64 -2545542355363006406, i64 10, i64 3667884930908592509, i64 1, i64 15435711456043681792, i64 0}
 
 ; CHECK: !llvm.module.flags = !{![[cgprof:[0-9]+]]}
 ; CHECK: ![[cgprof]] = !{i32 5, !"CG Profile", ![[prof:[0-9]+]]}
-; CHECK: ![[prof]] = !{![[e0:[0-9]+]], ![[e1:[0-9]+]], ![[e2:[0-9]+]], ![[e3:[0-9]+]], ![[e4:[0-9]+]], ![[e5:[0-9]+]], ![[e6:[0-9]+]]}
+; CHECK: ![[prof]] = distinct !{![[e0:[0-9]+]], ![[e1:[0-9]+]], ![[e2:[0-9]+]], ![[e3:[0-9]+]], ![[e4:[0-9]+]], ![[e5:[0-9]+]], ![[e6:[0-9]+]]}
 ; CHECK: ![[e0]] = !{void ()* @a, void ()* @b, i64 32}
 ; CHECK: ![[e1]] = !{void (i1)* @freq, i32 ()* @func4, i64 1030}
 ; CHECK: ![[e2]] = !{void (i1)* @freq, i32 ()* @func2, i64 410}
 ; CHECK: ![[e3]] = !{void (i1)* @freq, i32 ()* @func3, i64 150}
 ; CHECK: ![[e4]] = !{void (i1)* @freq, i32 ()* @func1, i64 10}
 ; CHECK: ![[e5]] = !{void (i1)* @freq, void ()* @a, i64 11}
 ; CHECK: ![[e6]] = !{void (i1)* @freq, void ()* @b, i64 21}
 ; CHECK-NOT: !{void (i1)* @freq, void ()* @func5, i64 1}
 ; CHECK-NOT: !{void (i1)* @freq, void ()* @func6, i64 0}

llvm/test/Linker/module-flags-8-a.ll

 ; RUN: llvm-link %s %p/module-flags-8-b.ll -S -o - | sort | FileCheck %s
 
 ; Test append-type module flags.
 
-; CHECK: !0 = !{i32 5, !"flag-0", !1}
-; CHECK: !1 = !{i32 0, i32 0, i32 1}
-; CHECK: !2 = !{i32 6, !"flag-1", !3}
-; CHECK: !3 = !{i32 0, i32 1, i32 2}
+; CHECK: !0 = distinct !{i32 5, !"flag-0", !1}
+; CHECK: !1 = distinct !{i32 0, i32 0, i32 1}
+; CHECK: !2 = distinct !{i32 6, !"flag-1", !3}
+; CHECK: !3 = distinct !{i32 0, i32 1, i32 2}
 ; CHECK: !llvm.module.flags = !{!0, !2}
 
 !0 = !{ i32 5, !"flag-0", !{ i32 0 } }
 !1 = !{ i32 6, !"flag-1", !{ i32 0, i32 1 } }
 
 !llvm.module.flags = !{ !0, !1 }

llvm/test/Linker/module-flags-dont-change-others.ll

 ; RUN: llvm-link %s %S/Inputs/module-flags-dont-change-others.ll -S -o - | FileCheck %s
 
 ; Test that module-flag linking doesn't change other metadata.  In particular,
 ; !named should still point at the unmodified tuples (!3, !4, and !5) that
 ; happen to also serve as module flags.
 
 ; CHECK: !named = !{!0, !1, !2, !3, !4, !5}
-; CHECK: !llvm.module.flags = !{!6, !7, !8}
+; CHECK: !llvm.module.flags = !{!6, !7, !9}
 !named = !{!0, !1, !2, !3, !4, !5}
 !llvm.module.flags = !{!3, !4, !5}
 
 ; CHECK: !0 = !{}
 ; CHECK: !1 = !{!0}
 ; CHECK: !2 = !{!0, !1}
 ; CHECK: !3 = !{i32 1, !"foo", i32 927}
 ; CHECK: !4 = !{i32 5, !"bar", !0}
 ; CHECK: !5 = !{i32 6, !"baz", !1}
 ; CHECK: !6 = !{i32 4, !"foo", i32 37}
-; CHECK: !7 = !{i32 5, !"bar", !1}
-; CHECK: !8 = !{i32 6, !"baz", !2}
+; CHECK: !7 = distinct !{i32 5, !"bar", !8}
+; CHECK: !8 = distinct !{!0}
+; CHECK: !9 = distinct !{i32 6, !"baz", !10}
+; CHECK: !10 = distinct !{!0, !1}
 !0 = !{}
 !1 = !{!0}
 !2 = !{!0, !1}
 !3 = !{i32 1, !"foo", i32 927}
 !4 = !{i32 5, !"bar", !0}
 !5 = !{i32 6, !"baz", !1}

llvm/test/Transforms/FunctionImport/cg_profile.ll

 ; Check that bitcast in "CG Profile" related metadata nodes (in this test case,
 ; generated during function importing in IRMover's RAUW operations) are accepted
 ; by verifier.
 ; RUN: opt -cg-profile -module-summary %s -o %t.bc
 ; RUN: opt -module-summary %p/Inputs/cg_profile.ll -o %t2.bc
 ; RUN: llvm-lto -thinlto -o %t3 %t.bc %t2.bc
 ; RUN: opt -function-import -print-imports -summary-file %t3.thinlto.bc %t.bc \
 ; RUN:   -S 2>&1 | FileCheck %s
 
 ; CHECK:      !0 = !{i32 1, !"EnableSplitLTOUnit", i32 0}
 ; CHECK-NEXT: !1 = !{i32 5, !"CG Profile", !2}
-; CHECK-NEXT: !2 = !{!3}
+; CHECK-NEXT: !2 = distinct !{!3}
 ; CHECK-NEXT: !3 = !{void ()* @foo, void (%class.A*)* bitcast (void (%class.A.0*)* @bar to void (%class.A*)*), i64 2753}
 
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 ; %class.A is defined differently in %p/Inputs/cg_profile.ll. This is to trigger
 ; bitcast.
 %class.A = type { i8 }