[PATCH] D18583: Cloning: Reduce complexity of debug info cloning and fix correctness issue.

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

If you add a test this LGTM.  I suggest adding something to
unittests/Transforms/Utils/.

> On 2016-Mar-29, at 16:09, Peter Collingbourne <peter at pcc.me.uk> wrote:
> 
> pcc created this revision.
> pcc added reviewers: dexonsmith, aprantl.
> pcc added subscribers: llvm-commits, loladiro, tejohnson, eugenis, probinson.
> 
> Commit r260791 contained an error in that it would introduce a cross-module
> reference in the old module. It also introduced O(N^2) complexity in the
> module cloner by requiring the entire module to be visited for each function.
> Fix both of these problems by avoiding use of the CloneDebugInfoMetadata
> function (which is only designed to do intra-module cloning) and cloning
> function-attached metadata in the same way that we clone all other metadata.
> 
> http://reviews.llvm.org/D18583
> 
> Files:
>  include/llvm/Transforms/Utils/Cloning.h
>  lib/Transforms/Utils/CloneFunction.cpp
>  lib/Transforms/Utils/CloneModule.cpp
> 
> Index: lib/Transforms/Utils/CloneModule.cpp
> ===================================================================
> --- lib/Transforms/Utils/CloneModule.cpp
> +++ lib/Transforms/Utils/CloneModule.cpp
> @@ -138,7 +138,6 @@
>         VMap[&*J] = &*DestI++;
>       }
> 
> -      CloneDebugInfoMetadata(F, &*I, VMap);
>       SmallVector<ReturnInst*, 8> Returns;  // Ignore returns cloned.
>       CloneFunctionInto(F, &*I, VMap, /*ModuleLevelChanges=*/true, Returns);
>     }
> Index: lib/Transforms/Utils/CloneFunction.cpp
> ===================================================================
> --- lib/Transforms/Utils/CloneFunction.cpp
> +++ lib/Transforms/Utils/CloneFunction.cpp
> @@ -119,6 +119,11 @@
>           .addAttributes(NewFunc->getContext(), AttributeSet::FunctionIndex,
>                          OldAttrs.getFnAttributes()));
> 
> +  SmallVector<std::pair<unsigned, MDNode *>, 1> MDs;
> +  OldFunc->getAllMetadata(MDs);
> +  for (auto MD : MDs)
> +    NewFunc->setMetadata(MD.first, MapMetadata(MD.second, VMap));
> +
>   // Loop over all of the basic blocks in the function, cloning them as
>   // appropriate.  Note that we save BE this way in order to handle cloning of
>   // recursive functions into themselves.
> @@ -187,8 +192,8 @@
> 
> // Clone the module-level debug info associated with OldFunc. The cloned data
> // will point to NewFunc instead.
> -void llvm::CloneDebugInfoMetadata(Function *NewFunc, const Function *OldFunc,
> -                                  ValueToValueMapTy &VMap) {
> +static void CloneDebugInfoMetadata(Function *NewFunc, const Function *OldFunc,
> +                                   ValueToValueMapTy &VMap) {
>   DebugInfoFinder Finder;
>   Finder.processModule(*OldFunc->getParent());
> 
> Index: include/llvm/Transforms/Utils/Cloning.h
> ===================================================================
> --- include/llvm/Transforms/Utils/Cloning.h
> +++ include/llvm/Transforms/Utils/Cloning.h
> @@ -130,11 +130,6 @@
>                         bool ModuleLevelChanges,
>                         ClonedCodeInfo *CodeInfo = nullptr);
> 
> -/// Clone the module-level debug info associated with OldFunc. The cloned data
> -/// will point to NewFunc instead.
> -void CloneDebugInfoMetadata(Function *NewFunc, const Function *OldFunc,
> -                            ValueToValueMapTy &VMap);
> -
> /// Clone OldFunc into NewFunc, transforming the old arguments into references
> /// to VMap values.  Note that if NewFunc already has basic blocks, the ones
> /// cloned into it will be added to the end of the function.  This function
> 
> 
> <D18583.52001.patch>