[PATCH] D40413: [CodeExtractor] Add debug locations for new call and branch instrs.

[Dehao Chen via Phabricator via llvm-commits]

danielcdh added inline comments.


================
Comment at: test/Transforms/CodeExtractor/PartialInlineDebug.ll:30
+; CHECK-LABEL: define internal void @callee.1_if.then
+; CHECK: br label %if.then, !dbg ![[DBG3:[0-9]+]]
+; : ![[DBG1]] = !DILocation(line: 10, column: 7,
----------------
fhahn wrote:
> danielcdh wrote:
> > I'm wondering if we inline callee.1_if.then back into caller, what would the inline stack be for this instruction. Looks like it will be:
> > 
> > test.c:10 (callee.1_if.then)
> > test.c:10 (callee)
> > test.c:5 (caller)
> > 
> > But from the PGO point of view, the correct inline stack should be:
> > 
> > test.c:10 (callee)
> > test.c:5 (caller)
> > 
> > Not sure if PGO-desired inline stack is possible with partial inlining with or without this patch.
> > 
> > Maybe we should just disable partial inlining when the binary is to be used to collect SamplePGO profile, unless it's designed for flattened profile. +wmi who's evaluating flattened profile.
> Great, thanks for having a look! Currently, partial inlining is disabled by default, but it sounds like we have to handle this issue before enabling it or disable it with PGO for now.
> 
> Is the problem for PGO with splitting callee into 2 functions the following: all samples would be (incorrectly) attributed to `callee` and not split between `callee` and `callee.1_if.then`?
On the contrary, in the profile, we want to have all samples be attributed to callee instead of split between 2 parts. But as callee .1_if.then is in a separate function, this does not seem possible.


https://reviews.llvm.org/D40413