[PATCH] D96531: TransformUtils: Fix metadata handling in CloneModule (and improve CloneFunctionInto)

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

dexonsmith added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/CloneFunction.cpp:255-256
 
-  // Register all DICompileUnits of the old parent module in the new parent module
-  auto* OldModule = OldFunc->getParent();
+  if (Changes != CloneFunctionChangeType::DifferentModule)
+    return;
+
----------------
Note: this returns early if `CloneFunctionChangeType` is `CloneModule`.


================
Comment at: llvm/lib/Transforms/Utils/CloneModule.cpp:188
     const NamedMDNode &NMD = *I;
     NamedMDNode *NewNMD = New->getOrInsertNamedMetadata(NMD.getName());
+    for (unsigned i = 0, e = NMD.getNumOperands(); i != e; ++i)
----------------
aprantl wrote:
> Why is this no longer necessary?
This was replaced by an early return in `CloneFunctionInto`, which restricts the special `!llvm.dbg.cu` logic to when the CloneFunctionChangeStyle is precisely `DifferentModule`. Letting CloneModule do it also ensures that the metadata are in the same order... you're getting an exact clone, not just something semantically equivalent.

Let me know if you think that the early return deserves a comment.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96531/new/

https://reviews.llvm.org/D96531