[PATCH] Support using sample profiles with partial debug info.

[Diego Novillo]

On Tue, Oct 21, 2014 at 3:38 PM, David Blaikie <dblaikie at gmail.com> wrote:

> ================
> Comment at: lib/Transforms/Scalar/SampleProfile.cpp:662
> @@ +661,3 @@
> +    // leading back to \p F.
> +    for (Function::iterator I = F.begin(), E = F.end(); I != E; ++I) {
> +      BasicBlock *B = I;
> ----------------
> Please use range-based-for loops.
>
>
The file already uses iterator-based loops. OK if I update all the loops in
a subsequent patch?

================
> Comment at: lib/Transforms/Scalar/SampleProfile.cpp:671
> @@ +670,3 @@
> +        if (Subprogram.describes(&F))
> +          return Subprogram.getLineNumber();
> +      }
> ----------------
> If you find an instruction with DebugLoc, you don't need to keep searching
> - no matter what its subprogram node is.
>

Ah, so you want a failure if Subprogram.describes(&F) is false. Got it.


> Here's the invariant that I believe now holds:
> If a function has debug info, the scope chain of all instructions in that
> function will lead to the function and nothing else.
>
> So once you find one instruction with a debug loc, you don't need to
> examine any others - if it leads to this function, you're done, if it
> doesn't then this function doesn't have debug info.
>

Done.


>
> ================
> Comment at: test/Transforms/SampleProfile/loc-tracking-only.ll:3
> @@ +2,3 @@
> +
> +; This test is identical to calls.ll but it is missing the llvm.dbg.cu
> +; annotation. This emulates the effect of the user having only used
> ----------------
> Worth updating calls.ll itself instead of adding a new test? If you don't
> depend on the llvm.dbg.cu at all, it doesn't seem worthwhile to have two
> different tests, one with it and one without it.
>

Ah, yes. Perhaps that's easiest. The other tests already cover the case
where llvm.dbg.cu is present.


Thanks. Diego.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141021/9178bd9f/attachment.html>