Make it illegal for two Functions to point to the same DISubprogram

[aprantl]

Summary

As recently discussed on llvm-dev [1], this patch makes it illegal for two Functions to point to the same DISubprogram and updates FunctionCloner to also clone the debug info of a function to conform to the new requirement. To simplify the implementation it also factors out the creation of inlineAt locations from the Inliner into a general-purpose utility in DILocation.

[1] http://lists.llvm.org/pipermail/llvm-dev/2017-May/112661.html

[dblaikie]

Looks pretty good to me

lib/IR/DebugLoc.cpp

167 ↗

(On Diff #98194)

Probably not need to explicitly state the return type here, unless you particularly like/want to do so?

lib/Transforms/Utils/CloneFunction.cpp

131 ↗

(On Diff #98194)

Is this ever not true? I guess it could be if CloneFunction is used to clone from one Module to another? is that a thing that happens?

171–172 ↗

(On Diff #98194)

Unrelated formatting - probably revert that.

unittests/Transforms/Utils/Cloning.cpp

364 ↗

(On Diff #98194)

EXPECT_NE ?

391–392 ↗

(On Diff #98194)

I don't really understand why this change is here - could you explain it?

I guess the actual change you made to CloneFunction makes the new function look as though the old function was inlined into it? That's a great idea/way to do it! :) (though won't be what I need for the LTO+Fission issue - but perhaps I can use some of the pieces you've built/generalized/etc here)

[aprantl]

lib/Transforms/Utils/CloneFunction.cpp

131 ↗

(On Diff #98194)

This is public LLVM API, so I think we need to brace for every possibility here.

unittests/Transforms/Utils/Cloning.cpp

391–392 ↗

(On Diff #98194)

Much simpler: This change is necessary because above, I'm adding an inlined function into OldFunc (line 309ff). DILocation::getInlinedAtScope() returns either the parent function of a regular instruction or the parent of the inlined-at location of an inlined instruction, both of which should be the DISubprogram attached to the function the instruction is part of.

[dblaikie]

unittests/Transforms/Utils/Cloning.cpp

391–392 ↗

(On Diff #98194)

Hmm, OK - could you explain more why you're doing that? To test the inlining scope rewriting functionality? That's a good point/something I'll have to deal with as well for the ThinLTO case - the frontend compilation might've already done some inlining... oh, except those won't ever be from another module, so that's easier... unless the module linking messed that up, maybe, and picked the foreign module's version of the subprogram :/ Will have to check/ponder.

But this also raises an interesting question then: what if we did represent function cloning as an act of inlining? That might actually be better? maybe? (I wouldn't do it in this commit, probably better separately)

[loladiro]

I'm really confused by the entirety of the changes to the CloneFunction parts of this. The whole point of the ValueMapper is to fix up value/metadata references. Why do we need a manual version? Also what's up with the DISubprogram::getDistinct? The ValueMapper will create new distinct nodes as it goes along unless RF_MoveDistinctMDs is set (which it isn't inside CloneFunction). Could the problem perhaps have been an incorrectly unset ModuleLevelChanges flag? I'm seeing a ton of assertion failures after this change. I'll submit an appropriate PR to revert the CloneFunction parts of this.

llvm/trunk/include/llvm/IR/DebugLoc.h

public:
     /// forwards to \a DILocation::get().
     ///
     /// If \c !Scope, returns a default-constructed \a DebugLoc.
     ///
     /// FIXME: Remove this.  Users should use DILocation::get().
     static DebugLoc get(unsigned Line, unsigned Col, const MDNode *Scope,
                         const MDNode *InlinedAt = nullptr);
 
+    enum { ReplaceLastInlinedAt = true };
+    /// Rebuild the entire inlined-at chain for this instruction so that the top of
+    /// the chain now is inlined-at the new call site.
+    /// \param   InlinedAt    The new outermost inlined-at in the chain.
+    /// \param   ReplaceLast  Replace the last location in the inlined-at chain.
+    static DebugLoc appendInlinedAt(DebugLoc DL, DILocation *InlinedAt,
+                                    LLVMContext &Ctx,
+                                    DenseMap<const MDNode *, MDNode *> &Cache,
+                                    bool ReplaceLast = false);
+
+    /// Reparent all debug locations referenced by \c I that belong to \c OrigSP
+    /// to become (possibly indirect) children of \c NewSP.
+    static void reparentDebugInfo(Instruction &I, DISubprogram *OrigSP,
+                                  DISubprogram *NewSP,
+                                  DenseMap<const MDNode *, MDNode *> &Cache);
+
     unsigned getLine() const;
     unsigned getCol() const;
     MDNode *getScope() const;
     DILocation *getInlinedAt() const;
 
     /// \brief Get the fully inlined-at scope for a DebugLoc.
     ///
     /// Gets the inlined-at scope for a DebugLoc.

llvm/trunk/lib/IR/DebugLoc.cpp

 //===-- DebugLoc.cpp - Implement DebugLoc class ---------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/IR/DebugLoc.h"
+#include "llvm/IR/IntrinsicInst.h"
 #include "LLVMContextImpl.h"
 #include "llvm/IR/DebugInfo.h"
 using namespace llvm;
 
 //===----------------------------------------------------------------------===//
 // DebugLoc Implementation
 //===----------------------------------------------------------------------===//
 DebugLoc::DebugLoc(const DILocation *L) : Loc(const_cast<DILocation *>(L)) {}
DebugLoc DebugLoc::get(unsigned Line, unsigned Col, const MDNode *Scope,
   if (!Scope)
     return DebugLoc();
 
   return DILocation::get(Scope->getContext(), Line, Col,
                          const_cast<MDNode *>(Scope),
                          const_cast<MDNode *>(InlinedAt));
 }
 
+DebugLoc DebugLoc::appendInlinedAt(DebugLoc DL, DILocation *InlinedAt,
+                                   LLVMContext &Ctx,
+                                   DenseMap<const MDNode *, MDNode *> &Cache,
+                                   bool ReplaceLast) {
+  SmallVector<DILocation *, 3> InlinedAtLocations;
+  DILocation *Last = InlinedAt;
+  DILocation *CurInlinedAt = DL;
+
+  // Gather all the inlined-at nodes.
+  while (DILocation *IA = CurInlinedAt->getInlinedAt()) {
+    // Skip any we've already built nodes for.
+    if (auto *Found = Cache[IA]) {
+      Last = cast<DILocation>(Found);
+      break;
+    }
+
+    if (ReplaceLast && !IA->getInlinedAt())
+      break;
+    InlinedAtLocations.push_back(IA);
+    CurInlinedAt = IA;
+  }
+
+  // Starting from the top, rebuild the nodes to point to the new inlined-at
+  // location (then rebuilding the rest of the chain behind it) and update the
+  // map of already-constructed inlined-at nodes.
+  for (const DILocation *MD : reverse(InlinedAtLocations))
+    Cache[MD] = Last = DILocation::getDistinct(
+        Ctx, MD->getLine(), MD->getColumn(), MD->getScope(), Last);
+
+  return Last;
+}
+
+/// Reparent \c Scope from \c OrigSP to \c NewSP.
+static DIScope *reparentScope(LLVMContext &Ctx, DIScope *Scope,
+                              DISubprogram *OrigSP, DISubprogram *NewSP,
+                              DenseMap<const MDNode *, MDNode *> &Cache) {
+  SmallVector<DIScope *, 3> ScopeChain;
+  DIScope *Last = NewSP;
+  DIScope *CurScope = Scope;
+  do {
+    if (auto *SP = dyn_cast<DISubprogram>(CurScope)) {
+      // Don't rewrite this scope chain if it doesn't lead to the replaced SP.
+      if (SP != OrigSP)
+        return Scope;
+      Cache.insert({OrigSP, NewSP});
+      break;
+    }
+    if (auto *Found = Cache[CurScope]) {
+      Last = cast<DIScope>(Found);
+      break;
+    }
+    ScopeChain.push_back(CurScope);
+  } while ((CurScope = CurScope->getScope().resolve()));
+
+  // Starting from the top, rebuild the nodes to point to the new inlined-at
+  // location (then rebuilding the rest of the chain behind it) and update the
+  // map of already-constructed inlined-at nodes.
+  for (const DIScope *MD : reverse(ScopeChain)) {
+    if (auto *LB = dyn_cast<DILexicalBlock>(MD))
+      Cache[MD] = Last = DILexicalBlock::getDistinct(
+          Ctx, Last, LB->getFile(), LB->getLine(), LB->getColumn());
+    else if (auto *LB = dyn_cast<DILexicalBlockFile>(MD))
+      Cache[MD] = Last = DILexicalBlockFile::getDistinct(
+          Ctx, Last, LB->getFile(), LB->getDiscriminator());
+    else
+      llvm_unreachable("illegal parent scope");
+  }
+  return Last;
+}
+
+void DebugLoc::reparentDebugInfo(Instruction &I, DISubprogram *OrigSP,
+                                 DISubprogram *NewSP,
+                                 DenseMap<const MDNode *, MDNode *> &Cache) {
+  auto DL = I.getDebugLoc();
+  if (!OrigSP || !NewSP || !DL)
+    return;
+
+  // Reparent the debug location.
+  auto &Ctx = I.getContext();
+  DILocation *InlinedAt = DL->getInlinedAt();
+  if (InlinedAt) {
+    while (auto *IA = InlinedAt->getInlinedAt())
+      InlinedAt = IA;
+    auto NewScope =
+        reparentScope(Ctx, InlinedAt->getScope(), OrigSP, NewSP, Cache);
+    InlinedAt =
+        DebugLoc::get(InlinedAt->getLine(), InlinedAt->getColumn(), NewScope);
+  }
+  I.setDebugLoc(
+      DebugLoc::get(DL.getLine(), DL.getCol(),
+                    reparentScope(Ctx, DL->getScope(), OrigSP, NewSP, Cache),
+                    DebugLoc::appendInlinedAt(DL, InlinedAt, Ctx, Cache,
+                                              ReplaceLastInlinedAt)));
+
+  // Fix up debug variables to point to NewSP.
+  auto reparentVar = [&](DILocalVariable *Var) {
+    return DILocalVariable::getDistinct(
+        Ctx,
+        cast<DILocalScope>(
+            reparentScope(Ctx, Var->getScope(), OrigSP, NewSP, Cache)),
+        Var->getName(), Var->getFile(), Var->getLine(), Var->getType(),
+        Var->getArg(), Var->getFlags(), Var->getAlignInBits());
+  };
+  if (auto *DbgValue = dyn_cast<DbgValueInst>(&I)) {
+    auto *Var = DbgValue->getVariable();
+    I.setOperand(2, MetadataAsValue::get(Ctx, reparentVar(Var)));
+  } else if (auto *DbgDeclare = dyn_cast<DbgDeclareInst>(&I)) {
+    auto *Var = DbgDeclare->getVariable();
+    I.setOperand(1, MetadataAsValue::get(Ctx, reparentVar(Var)));
+  }
+}
+
+
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
 LLVM_DUMP_METHOD void DebugLoc::dump() const {
   if (!Loc)
     return;
 
   dbgs() << getLine();
   if (getCol() != 0)
     dbgs() << ',' << getCol();

llvm/trunk/lib/IR/Verifier.cpp

class Verifier : public InstVisitor<Verifier>, VerifierSupport {
   ///
   /// This allows us to do efficient dominance checks for the case when an
   /// instruction has an operand that is an instruction in the same block.
   SmallPtrSet<Instruction *, 16> InstsInThisBlock;
 
   /// \brief Keep track of the metadata nodes that have been checked already.
   SmallPtrSet<const Metadata *, 32> MDNodes;
 
+  /// Keep track which DISubprogram is attached to which function.
+  DenseMap<const DISubprogram *, const Function *> DISubprogramAttachments;
+
   /// Track all DICompileUnits visited.
   SmallPtrSet<const Metadata *, 2> CUVisited;
 
   /// \brief The result type for a landingpad.
   Type *LandingPadResultTy;
 
   /// \brief Whether we've seen a call to @llvm.localescape in this function
   /// already.
for (const StringMapEntry<Comdat> &SMEC : M.getComdatSymbolTable())
       visitComdat(SMEC.getValue());
 
     visitModuleFlags(M);
     visitModuleIdents(M);
 
     verifyCompileUnits();
 
     verifyDeoptimizeCallingConvs();
-
+    DISubprogramAttachments.clear();
     return !Broken;
   }
 
 private:
   // Verification methods...
   void visitGlobalValue(const GlobalValue &GV);
   void visitGlobalVariable(const GlobalVariable &GV);
   void visitGlobalAlias(const GlobalAlias &GA);
if (F.isMaterializable()) {
 
     unsigned NumDebugAttachments = 0, NumProfAttachments = 0;
     // Visit metadata attachments.
     for (const auto &I : MDs) {
       // Verify that the attachment is legal.
       switch (I.first) {
       default:
         break;
-      case LLVMContext::MD_dbg:
+      case LLVMContext::MD_dbg: {
         ++NumDebugAttachments;
         AssertDI(NumDebugAttachments == 1,
                  "function must have a single !dbg attachment", &F, I.second);
         AssertDI(isa<DISubprogram>(I.second),
                  "function !dbg attachment must be a subprogram", &F, I.second);
+        auto *SP = cast<DISubprogram>(I.second);
+        const Function *&AttachedTo = DISubprogramAttachments[SP];
+        AssertDI(!AttachedTo || AttachedTo == &F,
+                 "DISubprogram attached to more than one function", SP, &F);
+        AttachedTo = &F;
         break;
+      }
       case LLVMContext::MD_prof:
         ++NumProfAttachments;
         Assert(NumProfAttachments == 1,
                "function must have a single !prof attachment", &F, I.second);
         break;
       }
 
       // Verify the metadata itself.

llvm/trunk/lib/Transforms/Utils/CloneFunction.cpp

 #include <map>
 using namespace llvm;
 
 /// See comments in Cloning.h.
 BasicBlock *llvm::CloneBasicBlock(const BasicBlock *BB,
                                   ValueToValueMapTy &VMap,
                                   const Twine &NameSuffix, Function *F,
                                   ClonedCodeInfo *CodeInfo) {
+  DenseMap<const MDNode *, MDNode *> Cache;
   BasicBlock *NewBB = BasicBlock::Create(BB->getContext(), "", F);
   if (BB->hasName()) NewBB->setName(BB->getName()+NameSuffix);
 
   bool hasCalls = false, hasDynamicAllocas = false, hasStaticAllocas = false;
   
   // Loop over all instructions, and copy them over.
   for (BasicBlock::const_iterator II = BB->begin(), IE = BB->end();
        II != IE; ++II) {
     Instruction *NewInst = II->clone();
+    if (F && F->getSubprogram())
+      DebugLoc::reparentDebugInfo(*NewInst, BB->getParent()->getSubprogram(),
+                                  F->getSubprogram(), Cache);
     if (II->hasName())
       NewInst->setName(II->getName()+NameSuffix);
     NewBB->getInstList().push_back(NewInst);
     VMap[&*II] = NewInst; // Add instruction map to value.
 
     hasCalls |= (isa<CallInst>(II) && !isa<DbgInfoIntrinsic>(II));
     if (const AllocaInst *AI = dyn_cast<AllocaInst>(II)) {
       if (isa<ConstantInt>(AI->getArraySize()))
#endif
   }
 
   NewFunc->setAttributes(
       AttributeList::get(NewFunc->getContext(), OldAttrs.getFnAttributes(),
                          OldAttrs.getRetAttributes(), NewArgAttrs));
 
   SmallVector<std::pair<unsigned, MDNode *>, 1> MDs;
   OldFunc->getAllMetadata(MDs);
-  for (auto MD : MDs)
-    NewFunc->addMetadata(
-        MD.first,
-        *MapMetadata(MD.second, VMap,
+  for (auto MD : MDs) {
+    MDNode *NewMD;
+    bool MustCloneSP =
+        (MD.first == LLVMContext::MD_dbg && OldFunc->getParent() &&
+         OldFunc->getParent() == NewFunc->getParent());
+    if (MustCloneSP) {
+      auto *SP = cast<DISubprogram>(MD.second);
+      NewMD = DISubprogram::getDistinct(
+          NewFunc->getContext(), SP->getScope(), SP->getName(),
+          NewFunc->getName(), SP->getFile(), SP->getLine(), SP->getType(),
+          SP->isLocalToUnit(), SP->isDefinition(), SP->getScopeLine(),
+          SP->getContainingType(), SP->getVirtuality(), SP->getVirtualIndex(),
+          SP->getThisAdjustment(), SP->getFlags(), SP->isOptimized(),
+          SP->getUnit(), SP->getTemplateParams(), SP->getDeclaration(),
+          SP->getVariables(), SP->getThrownTypes());
+    } else
+      NewMD =
+          MapMetadata(MD.second, VMap,
                       ModuleLevelChanges ? RF_None : RF_NoModuleLevelChanges,
-                     TypeMapper, Materializer));
+                      TypeMapper, Materializer);
+    NewFunc->addMetadata(MD.first, *NewMD);
+  }
 
   // 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.
   //
   for (Function::const_iterator BI = OldFunc->begin(), BE = OldFunc->end();
        BI != BE; ++BI) {
     const BasicBlock &BB = *BI;

llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp

for (User *U : AI->users()) {
     if (U->getType() != Int8PtrTy) continue;
     if (U->stripPointerCasts() != AI) continue;
     if (isUsedByLifetimeMarker(U))
       return true;
   }
   return false;
 }
 
-/// Rebuild the entire inlined-at chain for this instruction so that the top of
-/// the chain now is inlined-at the new call site.
-static DebugLoc
-updateInlinedAtInfo(const DebugLoc &DL, DILocation *InlinedAtNode,
-                    LLVMContext &Ctx,
-                    DenseMap<const DILocation *, DILocation *> &IANodes) {
-  SmallVector<DILocation *, 3> InlinedAtLocations;
-  DILocation *Last = InlinedAtNode;
-  DILocation *CurInlinedAt = DL;
-
-  // Gather all the inlined-at nodes
-  while (DILocation *IA = CurInlinedAt->getInlinedAt()) {
-    // Skip any we've already built nodes for
-    if (DILocation *Found = IANodes[IA]) {
-      Last = Found;
-      break;
-    }
-
-    InlinedAtLocations.push_back(IA);
-    CurInlinedAt = IA;
-  }
-
-  // Starting from the top, rebuild the nodes to point to the new inlined-at
-  // location (then rebuilding the rest of the chain behind it) and update the
-  // map of already-constructed inlined-at nodes.
-  for (const DILocation *MD : reverse(InlinedAtLocations)) {
-    Last = IANodes[MD] = DILocation::getDistinct(
-        Ctx, MD->getLine(), MD->getColumn(), MD->getScope(), Last);
-  }
-
-  // And finally create the normal location for this instruction, referring to
-  // the new inlined-at chain.
-  return DebugLoc::get(DL.getLine(), DL.getCol(), DL.getScope(), Last);
-}
-
 /// Return the result of AI->isStaticAlloca() if AI were moved to the entry
 /// block. Allocas used in inalloca calls and allocas of dynamic array size
 /// cannot be static.
 static bool allocaWouldBeStaticInEntry(const AllocaInst *AI ) {
   return isa<Constant>(AI->getArraySize()) && !AI->isUsedWithInAlloca();
 }
 
 /// Update inlined instructions' line numbers to
static void fixupLineNumbers(Function *Fn, Function::iterator FI,
   // same location.
   InlinedAtNode = DILocation::getDistinct(
       Ctx, InlinedAtNode->getLine(), InlinedAtNode->getColumn(),
       InlinedAtNode->getScope(), InlinedAtNode->getInlinedAt());
 
   // Cache the inlined-at nodes as they're built so they are reused, without
   // this every instruction's inlined-at chain would become distinct from each
   // other.
-  DenseMap<const DILocation *, DILocation *> IANodes;
+  DenseMap<const MDNode *, MDNode *> IANodes;
 
   for (; FI != Fn->end(); ++FI) {
     for (BasicBlock::iterator BI = FI->begin(), BE = FI->end();
          BI != BE; ++BI) {
       if (DebugLoc DL = BI->getDebugLoc()) {
-        BI->setDebugLoc(
-            updateInlinedAtInfo(DL, InlinedAtNode, BI->getContext(), IANodes));
+        auto IA = DebugLoc::appendInlinedAt(DL, InlinedAtNode, BI->getContext(),
+                                            IANodes);
+        auto IDL = DebugLoc::get(DL.getLine(), DL.getCol(), DL.getScope(), IA);
+        BI->setDebugLoc(IDL);
         continue;
       }
 
       if (CalleeHasDebugInfo)
         continue;
       
       // If the inlined instruction has no line number, make it look as if it
       // originates from the call location. This is important for

llvm/trunk/test/Verifier/metadata-function-dbg.ll

 ; RUN: not llvm-as %s -disable-output 2>&1 | FileCheck %s
 
 ; CHECK:      function declaration may not have a !dbg attachment
 declare !dbg !4 void @f1()
 
-define void @f2() !dbg !4 {
+; CHECK:      function must have a single !dbg attachment
+define void @f2() !dbg !4 !dbg !4 {
   unreachable
 }
 
-; CHECK:      function must have a single !dbg attachment
-define void @f3() !dbg !4 !dbg !4 {
+; CHECK:      DISubprogram attached to more than one function
+define void @f3() !dbg !4 {
+  unreachable
+}
+
+; CHECK:      DISubprogram attached to more than one function
+define void @f4() !dbg !4 {
   unreachable
 }
 
 ; CHECK-NOT:  !dbg
 ; CHECK:      function !dbg attachment must be a subprogram
 ; CHECK-NEXT: void ()* @bar
 ; CHECK-NEXT: !{{[0-9]+}} = !{}
-define void @bar() !dbg !6 {
+define void @bar() !dbg !3 {
   unreachable
 }
 
 !llvm.module.flags = !{!0}
 !0 = !{i32 2, !"Debug Info Version", i32 3}
 
 !llvm.dbg.cu = !{!1}
 !1 = distinct !DICompileUnit(language: DW_LANG_C99, file: !2)
 !2 = !DIFile(filename: "t.c", directory: "/path/to/dir")
+!3 = !{}
 !4 = distinct !DISubprogram(name: "foo", scope: !1, file: !2, unit: !1)
-!6 = !{}

llvm/trunk/unittests/Transforms/Utils/Cloning.cpp

void CreateOldFunctionBodyAndDI() {
     IBuilder.SetInsertPoint(Entry);
     DebugLoc Loc = DebugLoc::get(3, 2, Subprogram);
     IBuilder.SetCurrentDebugLocation(Loc);
     AllocaInst* Alloca = IBuilder.CreateAlloca(IntegerType::getInt32Ty(C));
     IBuilder.SetCurrentDebugLocation(DebugLoc::get(4, 2, Subprogram));
     Value* AllocaContent = IBuilder.getInt32(1);
     Instruction* Store = IBuilder.CreateStore(AllocaContent, Alloca);
     IBuilder.SetCurrentDebugLocation(DebugLoc::get(5, 2, Subprogram));
-    Instruction* Terminator = IBuilder.CreateRetVoid();
 
     // Create a local variable around the alloca
     auto *IntType = DBuilder.createBasicType("int", 32, dwarf::DW_ATE_signed);
     auto *E = DBuilder.createExpression();
     auto *Variable =
         DBuilder.createAutoVariable(Subprogram, "x", File, 5, IntType, true);
     auto *DL = DILocation::get(Subprogram->getContext(), 5, 0, Subprogram);
     DBuilder.insertDeclare(Alloca, Variable, E, DL, Store);
     DBuilder.insertDbgValueIntrinsic(AllocaContent, 0, Variable, E, DL,
-                                     Terminator);
-    // Finalize the debug info
+                                     Entry);
+    // Also create an inlined variable.
+    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);
+    auto *Scope = DBuilder.createLexicalBlock(
+        DBuilder.createLexicalBlockFile(InlinedSP, File), File, 1, 1);
+    auto InlinedDL =
+        DebugLoc::get(9, 4, Scope, DebugLoc::get(5, 2, Subprogram));
+    IBuilder.SetCurrentDebugLocation(InlinedDL);
+    DBuilder.insertDeclare(Alloca, InlinedVar, E, InlinedDL, Store);
+    IBuilder.CreateStore(IBuilder.getInt32(2), Alloca);
+    // Finalize the debug info.
     DBuilder.finalize();
+    IBuilder.CreateRetVoid();
 
-
-    // Create another, empty, compile unit
+    // Create another, empty, compile unit.
     DIBuilder DBuilder2(*M);
     DBuilder2.createCompileUnit(dwarf::DW_LANG_C99,
                                 DBuilder.createFile("extra.c", "/file/dir"),
                                 "CloneFunc", false, "", 0);
     DBuilder2.finalize();
   }
 
   void CreateNewFunc() {

 TEST_F(CloneFunc, NewFunctionCreated) {
   EXPECT_NE(OldFunc, NewFunc);
 }
 
 // Test that a new subprogram entry was added and is pointing to the new
 // function, while the original subprogram still points to the old one.
 TEST_F(CloneFunc, Subprogram) {
   EXPECT_FALSE(verifyModule(*M));
-
-  unsigned SubprogramCount = Finder->subprogram_count();
-  EXPECT_EQ(1U, SubprogramCount);
-
-  auto Iter = Finder->subprograms().begin();
-  auto *Sub = cast<DISubprogram>(*Iter);
-
-  EXPECT_TRUE(Sub == OldFunc->getSubprogram());
-  EXPECT_TRUE(Sub == NewFunc->getSubprogram());
+  EXPECT_EQ(3U, Finder->subprogram_count());
+  EXPECT_NE(NewFunc->getSubprogram(), OldFunc->getSubprogram());
 }
 
 // Test that instructions in the old function still belong to it in the
 // metadata, while instruction in the new function belong to the new one.
 TEST_F(CloneFunc, InstructionOwnership) {
   EXPECT_FALSE(verifyModule(*M));
 
   inst_iterator OldIter = inst_begin(OldFunc);
if (OldI.hasMetadata()) {
       const DebugLoc& OldDL = OldI.getDebugLoc();
       const DebugLoc& NewDL = NewI.getDebugLoc();
 
       // Verify that the debug location data is the same
       EXPECT_EQ(OldDL.getLine(), NewDL.getLine());
       EXPECT_EQ(OldDL.getCol(), NewDL.getCol());
 
       // But that they belong to different functions
-      auto *OldSubprogram = cast<DISubprogram>(OldDL.getScope());
-      auto *NewSubprogram = cast<DISubprogram>(NewDL.getScope());
+      auto *OldSubprogram = cast<DISubprogram>(OldDL.getInlinedAtScope());
+      auto *NewSubprogram = cast<DISubprogram>(NewDL.getInlinedAtScope());
       EXPECT_EQ(OldFunc->getSubprogram(), OldSubprogram);
       EXPECT_EQ(NewFunc->getSubprogram(), NewSubprogram);
     }
 
     ++OldIter;
     ++NewIter;
   }
   EXPECT_EQ(OldEnd, OldIter);
if (DbgDeclareInst* OldIntrin = dyn_cast<DbgDeclareInst>(&OldI)) {
 
       // Old address must belong to the old function
       EXPECT_EQ(OldFunc, cast<AllocaInst>(OldIntrin->getAddress())->
                          getParent()->getParent());
       // New address must belong to the new function
       EXPECT_EQ(NewFunc, cast<AllocaInst>(NewIntrin->getAddress())->
                          getParent()->getParent());
 
-      // Old variable must belong to the old function
+      if (!OldIntrin->getDebugLoc()->getInlinedAt()) {
+        // Old variable must belong to the old function.
         EXPECT_EQ(OldFunc->getSubprogram(),
                   cast<DISubprogram>(OldIntrin->getVariable()->getScope()));
-      // New variable must belong to the New function
+        // New variable must belong to the new function.
         EXPECT_EQ(NewFunc->getSubprogram(),
                   cast<DISubprogram>(NewIntrin->getVariable()->getScope()));
+      }
     } else if (DbgValueInst* OldIntrin = dyn_cast<DbgValueInst>(&OldI)) {
       DbgValueInst* NewIntrin = dyn_cast<DbgValueInst>(&NewI);
       EXPECT_TRUE(NewIntrin);
 
-      // Old variable must belong to the old function
+      if (!OldIntrin->getDebugLoc()->getInlinedAt()) {
+        // Old variable must belong to the old function.
         EXPECT_EQ(OldFunc->getSubprogram(),
                   cast<DISubprogram>(OldIntrin->getVariable()->getScope()));
-      // New variable must belong to the New function
+        // New variable must belong to the new function.
         EXPECT_EQ(NewFunc->getSubprogram(),
                   cast<DISubprogram>(NewIntrin->getVariable()->getScope()));
       }
+    }
 
     ++OldIter;
     ++NewIter;
   }
 }
 
 class CloneModule : public ::testing::Test {
 protected: