[PATCH] D32975: Make it illegal for two Functions to point to the same DISubprogram

[Adrian Prantl via Phabricator via llvm-commits]

aprantl marked 3 inline comments as done.
aprantl added inline comments.


================
Comment at: lib/Transforms/Utils/CloneFunction.cpp:131
+        (MD.first == LLVMContext::MD_dbg && OldFunc->getParent() &&
+         OldFunc->getParent() == NewFunc->getParent());
+    if (MustCloneSP) {
----------------
dblaikie wrote:
> 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?
This is public LLVM API, so I think we need to brace for every possibility here.


================
Comment at: unittests/Transforms/Utils/Cloning.cpp:391-392
       // 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);
----------------
dblaikie wrote:
> 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)
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.


https://reviews.llvm.org/D32975