[PATCH] D85085: Fix debug_loc offset difference with basic block sections

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D85085#2192340 <https://reviews.llvm.org/D85085#2192340>, @tmsriram wrote:

> In D85085#2191679 <https://reviews.llvm.org/D85085#2191679>, @dblaikie wrote:
>
>> Trying to reproduce this (by taking the .ll test case and compiling it with clang -c) I don't get any debug_loc section or DW_AT_locations. Is there something I'm missing about how to reproduce this?
>>
>> (trying to see what the non-bb-sections location description looks like - if it depends on basic block ordering, it's probably incorrect anyway, and might be worth fixing so it doesn't depend on that non-guaranteed property & then there'll be less divergence between bb-sections and not)
>
> I see why debug_loc is not produced in the default case.  This is what happens:
>
> + For the default case (no bbsections),  DebugLoc in  https://github.com/llvm/llvm-project/blob/master/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp#L1744  , it has exactly one value that contains the whole function scope as the start label is lfunc_begin and the end label is the end of the function.   
> + Such location lists are handled specially here when the scope is the whole function : https://github.com/llvm/llvm-project/blob/master/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp#L1841  and the debug_loc is not generated.

OK ^ and with bb sections do we see the same behavior? If not, why not?

> Diffing the asm by forcing a debug_loc entry,

Not sure what you mean by this ^ could you explain further?

> what happens here is that a DW_AT_location is replaced with a DW_AT_const_value.   I don't understand this too well but this seems like an optimization to me.
>
> Does this make sense?

I'm still a bit confused. I think if non-bb-sections doesn't need a location list, then bb-sections shouldn't either. Perhaps that issue should be fixed first, if it's an issue?

In any case/any order - perhaps this patch is still reflective of a bug that exists even in this ^ issue is fixed, but maybe it's that the test case isn't sufficient to demonstrate the remaining bug - perhaps the test case could be complicated a bit further to ensure it's not testing a behavior that shouldn't be happening for other reasons anyway?


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

https://reviews.llvm.org/D85085