[llvm-commits] [llvm] r103135 - /llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

[Devang Patel]

Dan,

On Thu, May 6, 2010 at 10:24 AM, Dan Gohman <gohman at apple.com> wrote:
>
> On May 6, 2010, at 9:39 AM, Devang Patel wrote:
>
>> Dan,
>>
>> On Wed, May 5, 2010 at 5:32 PM, Dan Gohman <gohman at apple.com> wrote:
>>>
>>> I took a simple program and compiled it with -g -emit-llvm. I edited
>>> the output and deleted the !dbg parts from some of the instructions in
>>> the middle. I then ran CodeGen and produced an object file, and then
>>> checked that the debug info for those instructions was in fact set
>>> to "unknown".
>>
>> What exactly did you see as "unknown" ? some random value ? 0,0 ?
>
> It's not random. The dwarf standard in section 6.2.2 says this, in the
> description of the "line" field:
>
>  The compiler may emit the value 0 in cases where an instruction cannot
>  be attributed to any source line.
>
> This is exactly what the code is now doing.
>
>>
>> IMO, lack of location info is better then "unknown" location info. For
>> example, while single stepping instructions, "unknown" location
>> instructs the debugger to take you to "unknown" location which is not
>> at all helpful to end user. Where as lack of location info for a
>> instruction  lets the debugger to not take any action or assume
>> current last known location. Either way better then jumping to
>> "unknown" location in source file.  Plus, "unknown" location increases
>> line table size without any significant benefit.
>
> The problem is that the previous code didn't ever indicate a lack of
> location info. It just extended the range for the location info of
> the most recently processed instruction that had a location.

Compiler is not extending the location range here in beginScope().
(May be I should rename it to avoid confusion. Compiler is only
emitting markers, (labels or .loc/.file entries) that are referenced
by line table, for known locations.

> If a debugger or other consumer thinks it's a good idea to resolve an
> unknown location by hunting around for the nearest known location, it
> can still do so. The only difference is that now it really is up to
> the consumer to do this; the compiler is no longer hard-coding it.

In optimized code if performance analyzing tools identify that
"unknown" location is my performance bottleneck in my source then I'll
be confused. If you expect these tools to interpret "unknown"
appropriately then these tools will have to track down nearest known
location. If the these tools are not updated/smart enough then end
user suffers.  So you're increasing likely hood of inferior end user
experience. Because standards say we can do so if we want.

>>
>>> I don't know how to write a test for this.
>>
>> You'd see extra labels in .s file.
>
> Ok, I'll see what I can do.
>
>> I need a good reason to keep this change.
>
> I offer the rationale I gave in the commit message. The compiler should
> either emit accurate debug info or no debug info. If it has lost track of
> the location for a particular instruction, it's a QOI issue. But if it
> then covers up the problem by assigning that instruction the location
> from some other instruction which may not be related, it's being actively
> misleading.

If the tools (compiler, debugger, performance analyzer, ...) can
provide better end user experience by saying "this instruction has
unknown location" instead of "I do not have location info for this
instruction" then I'm all for it. If not then we're doing extra work,
expecting extra work from other tools, bloating line tables.
-
Devang