[PATCH] D79379: Don't add function to import list if it's defined in the same module

Tue May 5 13:31:06 PDT 2020

lxfind marked an inline comment as done.
lxfind added inline comments.

================
Comment at: llvm/include/llvm/ProfileData/SampleProf.h:541
           const Function *Callee = M->getFunction(getFuncName(TS.getKey()));
           if (!Callee || !Callee->getSubprogram())
             S.insert(getGUID(TS.getKey()));
----------------
wenlei wrote:
> lxfind wrote:
> > @wenlei Do you know why we are checking for getSubprogram() here?
> I didn't notice this, but it looks fishy to me. We could have declaration with `!dbg` (see `metadata-function-dbg.ll`) which we should still import. We could also have definition without `!dbg`.. I'm not sure why we check `getSubprogram()` - I guess we can verify by trying to contrive a case that this logic would miss for importing.. @wmi is this intentional?
@wenlei  I was able to reproduce both of the cases you described, that a function isn't added to import list when it doesn't have !dbg annotation, indicating that we shouldn't rely on getSubProgram here. This is probably also intended to mimic the behavior of isDeclaration. 
I will fix this part as well, add a test, while waiting for @wmi  to confirm.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79379/new/

https://reviews.llvm.org/D79379