|
| 1 | +From af289e04413504c3bdc252e08c3fe17bf7ea6dc8 Mon Sep 17 00:00:00 2001 |
| 2 | +From: Peter Collingbourne < [email protected]> |
| 3 | +Date: Wed, 30 Mar 2016 22:05:13 +0000 |
| 4 | +Subject: [PATCH] Cloning: Reduce complexity of debug info cloning and fix |
| 5 | + correctness issue. |
| 6 | + |
| 7 | +Commit r260791 contained an error in that it would introduce a cross-module |
| 8 | +reference in the old module. It also introduced O(N^2) complexity in the |
| 9 | +module cloner by requiring the entire module to be visited for each function. |
| 10 | +Fix both of these problems by avoiding use of the CloneDebugInfoMetadata |
| 11 | +function (which is only designed to do intra-module cloning) and cloning |
| 12 | +function-attached metadata in the same way that we clone all other metadata. |
| 13 | + |
| 14 | +Differential Revision: http://reviews.llvm.org/D18583 |
| 15 | + |
| 16 | +git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@264935 91177308-0d34-0410-b5e6-96231b3b80d8 |
| 17 | +--- |
| 18 | + include/llvm/Transforms/Utils/Cloning.h | 5 ----- |
| 19 | + lib/Transforms/Utils/CloneFunction.cpp | 13 +++++++++++-- |
| 20 | + lib/Transforms/Utils/CloneModule.cpp | 1 - |
| 21 | + unittests/Transforms/Utils/Cloning.cpp | 6 ++++++ |
| 22 | + 4 files changed, 17 insertions(+), 8 deletions(-) |
| 23 | + |
| 24 | +diff --git a/include/llvm/Transforms/Utils/Cloning.h b/include/llvm/Transforms/Utils/Cloning.h |
| 25 | +index 0bae2bd..4f006f2 100644 |
| 26 | +--- a/include/llvm/Transforms/Utils/Cloning.h |
| 27 | ++++ b/include/llvm/Transforms/Utils/Cloning.h |
| 28 | +@@ -130,11 +130,6 @@ Function *CloneFunction(const Function *F, ValueToValueMapTy &VMap, |
| 29 | + bool ModuleLevelChanges, |
| 30 | + ClonedCodeInfo *CodeInfo = nullptr); |
| 31 | + |
| 32 | +-/// Clone the module-level debug info associated with OldFunc. The cloned data |
| 33 | +-/// will point to NewFunc instead. |
| 34 | +-void CloneDebugInfoMetadata(Function *NewFunc, const Function *OldFunc, |
| 35 | +- ValueToValueMapTy &VMap); |
| 36 | +- |
| 37 | + /// Clone OldFunc into NewFunc, transforming the old arguments into references |
| 38 | + /// to VMap values. Note that if NewFunc already has basic blocks, the ones |
| 39 | + /// cloned into it will be added to the end of the function. This function |
| 40 | +diff --git a/lib/Transforms/Utils/CloneFunction.cpp b/lib/Transforms/Utils/CloneFunction.cpp |
| 41 | +index 05b0a17..8e1715a 100644 |
| 42 | +--- a/lib/Transforms/Utils/CloneFunction.cpp |
| 43 | ++++ b/lib/Transforms/Utils/CloneFunction.cpp |
| 44 | +@@ -119,6 +119,15 @@ void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc, |
| 45 | + .addAttributes(NewFunc->getContext(), AttributeSet::FunctionIndex, |
| 46 | + OldAttrs.getFnAttributes())); |
| 47 | + |
| 48 | ++ SmallVector<std::pair<unsigned, MDNode *>, 1> MDs; |
| 49 | ++ OldFunc->getAllMetadata(MDs); |
| 50 | ++ for (auto MD : MDs) |
| 51 | ++ NewFunc->setMetadata( |
| 52 | ++ MD.first, |
| 53 | ++ MapMetadata(MD.second, VMap, |
| 54 | ++ ModuleLevelChanges ? RF_None : RF_NoModuleLevelChanges, |
| 55 | ++ TypeMapper, Materializer)); |
| 56 | ++ |
| 57 | + // Loop over all of the basic blocks in the function, cloning them as |
| 58 | + // appropriate. Note that we save BE this way in order to handle cloning of |
| 59 | + // recursive functions into themselves. |
| 60 | +@@ -187,8 +196,8 @@ static void AddOperand(DICompileUnit *CU, DISubprogramArray SPs, |
| 61 | + |
| 62 | + // Clone the module-level debug info associated with OldFunc. The cloned data |
| 63 | + // will point to NewFunc instead. |
| 64 | +-void llvm::CloneDebugInfoMetadata(Function *NewFunc, const Function *OldFunc, |
| 65 | +- ValueToValueMapTy &VMap) { |
| 66 | ++static void CloneDebugInfoMetadata(Function *NewFunc, const Function *OldFunc, |
| 67 | ++ ValueToValueMapTy &VMap) { |
| 68 | + DebugInfoFinder Finder; |
| 69 | + Finder.processModule(*OldFunc->getParent()); |
| 70 | + |
| 71 | +diff --git a/lib/Transforms/Utils/CloneModule.cpp b/lib/Transforms/Utils/CloneModule.cpp |
| 72 | +index 494e275..929f51b 100644 |
| 73 | +--- a/lib/Transforms/Utils/CloneModule.cpp |
| 74 | ++++ b/lib/Transforms/Utils/CloneModule.cpp |
| 75 | +@@ -138,7 +138,6 @@ std::unique_ptr<Module> llvm::CloneModule( |
| 76 | + VMap[&*J] = &*DestI++; |
| 77 | + } |
| 78 | + |
| 79 | +- CloneDebugInfoMetadata(F, &*I, VMap); |
| 80 | + SmallVector<ReturnInst*, 8> Returns; // Ignore returns cloned. |
| 81 | + CloneFunctionInto(F, &*I, VMap, /*ModuleLevelChanges=*/true, Returns); |
| 82 | + } |
| 83 | +diff --git a/unittests/Transforms/Utils/Cloning.cpp b/unittests/Transforms/Utils/Cloning.cpp |
| 84 | +index b761e4e..f06a20f 100644 |
| 85 | +--- a/unittests/Transforms/Utils/Cloning.cpp |
| 86 | ++++ b/unittests/Transforms/Utils/Cloning.cpp |
| 87 | +@@ -464,6 +464,12 @@ TEST_F(CloneModule, Verify) { |
| 88 | + EXPECT_FALSE(verifyModule(*NewM)); |
| 89 | + } |
| 90 | + |
| 91 | ++TEST_F(CloneModule, OldModuleUnchanged) { |
| 92 | ++ DebugInfoFinder Finder; |
| 93 | ++ Finder.processModule(*OldM); |
| 94 | ++ EXPECT_EQ(1U, Finder.subprogram_count()); |
| 95 | ++} |
| 96 | ++ |
| 97 | + TEST_F(CloneModule, Subprogram) { |
| 98 | + Function *NewF = NewM->getFunction("f"); |
| 99 | + DISubprogram *SP = NewF->getSubprogram(); |
0 commit comments