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

[Dehao Chen via Phabricator via llvm-commits]

danielcdh accepted this revision.
danielcdh added a comment.

The patch LGTM. Notes about SamplePGO use cases below, which may be addressed by separate patch/discussion, if necessary.



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


https://reviews.llvm.org/D40413