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

[David Blaikie via llvm-commits]

On Mon, May 8, 2017 at 1:45 PM Adrian Prantl via Phabricator <
reviews at reviews.llvm.org> wrote:

> 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.
>

Not sure what you mean by "public" API - and in any case, if there's no use
currently in-tree, I think an assert would be more appropriate. Then once
there's a clear use case it can be supported, tested, etc?


>
>
> ================
> 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
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170508/3c3be2cc/attachment.html>