[llvm] r237237 - [DebugInfo] Debug locations for constant SD nodes

[David Blaikie]

On Wed, Jun 3, 2015 at 6:18 AM, Sergey Dmitrouk <sdmitrouk at accesssoftek.com>
wrote:

> On Tue, Jun 02, 2015 at 03:48:23PM -0700, David Blaikie wrote:
> > > Not sure how to handle this yet, a couple of obvious tries causes test
> > > failures, will look for something better.
> >
> > Given the debug info size/quality regression here - perhaps we could
> > disable this again, temporarily, until we have an answer?
>
> Looks like we don't need to do this.  I was wrong thinking that the fix
> causes tests to fail, there were debugging aid kind of changes left in
> the code, which I forgot to revert before running tests.
>
> Please take a look at the attached patch.
>

Looks good - please commit when ready.

Optional thoughts you can roll into the commit or we can handle in post
commit:

* The CHECK lines might be a bit more verbose/selectively chosen to be more
self documenting (perhaps CHECK for a .loc of line 9, then CHECK-NOT for a
.loc, then CHECK for the movl $1 - essentially saying "ensure that the
constant 1 is attributed to line 9")
* You could drop the "getDebugLoc" from the condition you're adding, since
it doesn't matter whether it has one or not if we're just removing it anyway
* The comment's english is a bit confusing. Specifically in the last clause
of the first sentence. Maybe: "Remove the debug location from the node as
the node is about to be used in a location which may differ from the
original debug location" ? I'm not sure if that's accurate or not, though.

Just some thoughts - thanks again!

- David



>
> Thanks,
> Sergey
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150604/e811f3c4/attachment.html>