From d73686e841203a46002793b1d3e5fe79794ae0dc Mon Sep 17 00:00:00 2001 From: Gor Nishanov Date: Fri, 7 Jul 2017 18:24:20 +0000 Subject: [PATCH] [cloning] Do not duplicate types when cloning functions Summary: This is an addon to the change rl304488 cloning fixes. (Originally rl304226 reverted rl304228 and reapplied rl304488 https://reviews.llvm.org/D33655) rl304488 works great when DILocalVariables that comes from the inlined function has a 'unique-ed' type, but, in the case when the variable type is distinct we will create a second DILocalVariable in the scope of the original function that was inlined. Consider cloning of the following function: ``` define private void @f() !dbg !5 { %1 = alloca i32, !dbg !11 call void @llvm.dbg.declare(metadata i32* %1, metadata !14, metadata !12), !dbg !18 ret void, !dbg !18 } !14 = !DILocalVariable(name: "inlined", scope: !15, file: !6, line: 5, type: !17) ; came from an inlined function !15 = distinct !DISubprogram(name: "inlined", linkageName: "inlined", scope: null, file: !6, line: 8, type: !7, isLocal: true, isDefinition: true, scopeLine: 9, isOptimized: false, unit: !0, variables: !16) !16 = !{!14} !17 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "some_struct", size: 32, align: 32) ``` Without this fix, when function 'f' is cloned, we will create another DILocalVariable for "inlined", due to its type being distinct. ``` define private void @f.1() !dbg !23 { %1 = alloca i32, !dbg !26 call void @llvm.dbg.declare(metadata i32* %1, metadata !28, metadata !12), !dbg !30 ret void, !dbg !30 } !14 = !DILocalVariable(name: "inlined", scope: !15, file: !6, line: 5, type: !17) !15 = distinct !DISubprogram(name: "inlined", linkageName: "inlined", scope: null, file: !6, line: 8, type: !7, isLocal: true, isDefinition: true, scopeLine: 9, isOptimized: false, unit: !0, variables: !16) !16 = !{!14} !17 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "some_struct", size: 32, align: 32) ; !28 = !DILocalVariable(name: "inlined", scope: !15, file: !6, line: 5, type: !29) ; OOPS second DILocalVariable !29 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "some_struct", size: 32, align: 32) ``` Now we have two DILocalVariable for "inlined" within the same scope. This result in assert in AsmPrinter/DwarfDebug.h:131: void llvm::DbgVariable::addMMIEntry(const llvm::DbgVariable &): Assertion `V.Var == Var && "conflicting variable"' failed. (Full example: See: https://bugs.llvm.org/show_bug.cgi?id=33492) In this change we prevent duplication of types so that when a metadata for DILocalVariable is cloned it will get uniqued to the same metadate node as an original variable. Reviewers: loladiro, dblaikie, aprantl, echristo Reviewed By: loladiro Subscribers: EricWF, llvm-commits Differential Revision: https://reviews.llvm.org/D35106 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@307418 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Transforms/Utils/CloneFunction.cpp | 20 +++++++++++++++++--- unittests/Transforms/Utils/Cloning.cpp | 13 +++++++++++-- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/lib/Transforms/Utils/CloneFunction.cpp b/lib/Transforms/Utils/CloneFunction.cpp index 314c990293c..7e75e884778 100644 --- a/lib/Transforms/Utils/CloneFunction.cpp +++ b/lib/Transforms/Utils/CloneFunction.cpp @@ -46,13 +46,21 @@ BasicBlock *llvm::CloneBasicBlock(const BasicBlock *BB, ValueToValueMapTy &VMap, if (BB->hasName()) NewBB->setName(BB->getName()+NameSuffix); bool hasCalls = false, hasDynamicAllocas = false, hasStaticAllocas = false; - + Module *TheModule = F ? F->getParent() : nullptr; + // Loop over all instructions, and copy them over. for (BasicBlock::const_iterator II = BB->begin(), IE = BB->end(); II != IE; ++II) { - if (DIFinder && F->getParent() && II->getDebugLoc()) - DIFinder->processLocation(*F->getParent(), II->getDebugLoc().get()); + if (DIFinder && TheModule) { + if (auto *DDI = dyn_cast(II)) + DIFinder->processDeclare(*TheModule, DDI); + else if (auto *DVI = dyn_cast(II)) + DIFinder->processValue(*TheModule, DVI); + + if (auto DbgLoc = II->getDebugLoc()) + DIFinder->processLocation(*TheModule, DbgLoc.get()); + } Instruction *NewInst = II->clone(); if (II->hasName()) @@ -153,6 +161,8 @@ void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc, // When we remap instructions, we want to avoid duplicating inlined // DISubprograms, so record all subprograms we find as we duplicate // instructions and then freeze them in the MD map. + // We also record information about dbg.value and dbg.declare to avoid + // duplicating the types. DebugInfoFinder DIFinder; // Loop over all of the basic blocks in the function, cloning them as @@ -193,6 +203,10 @@ void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc, } } + for (auto *Type : DIFinder.types()) { + VMap.MD()[Type].reset(Type); + } + // Loop over all of the instructions in the function, fixing up operand // references as we go. This uses VMap to do all the hard work. for (Function::iterator BB = diff --git a/unittests/Transforms/Utils/Cloning.cpp b/unittests/Transforms/Utils/Cloning.cpp index db3d10847cd..72a91d14417 100644 --- a/unittests/Transforms/Utils/Cloning.cpp +++ b/unittests/Transforms/Utils/Cloning.cpp @@ -312,11 +312,16 @@ protected: DBuilder.insertDbgValueIntrinsic(AllocaContent, 0, Variable, E, DL, Entry); // Also create an inlined variable. + // Create a distinct struct type that we should not duplicate during + // cloning). + auto *StructType = DICompositeType::getDistinct( + C, dwarf::DW_TAG_structure_type, "some_struct", nullptr, 0, nullptr, + nullptr, 32, 32, 0, DINode::FlagZero, nullptr, 0, nullptr, nullptr); auto *InlinedSP = DBuilder.createFunction(CU, "inlined", "inlined", File, 8, FuncType, true, true, 9, DINode::FlagZero, false); auto *InlinedVar = - DBuilder.createAutoVariable(InlinedSP, "inlined", File, 5, IntType, true); + DBuilder.createAutoVariable(InlinedSP, "inlined", File, 5, StructType, true); auto *Scope = DBuilder.createLexicalBlock( DBuilder.createLexicalBlockFile(InlinedSP, File), File, 1, 1); auto InlinedDL = @@ -426,7 +431,11 @@ TEST_F(CloneFunc, DebugIntrinsics) { EXPECT_EQ(NewFunc, cast(NewIntrin->getAddress())-> getParent()->getParent()); - if (!OldIntrin->getDebugLoc()->getInlinedAt()) { + if (OldIntrin->getDebugLoc()->getInlinedAt()) { + // Inlined variable should refer to the same DILocalVariable as in the + // Old Function + EXPECT_EQ(OldIntrin->getVariable(), NewIntrin->getVariable()); + } else { // Old variable must belong to the old function. EXPECT_EQ(OldFunc->getSubprogram(), cast(OldIntrin->getVariable()->getScope()));