[llvm] r235989 - Reapply r235977 "[DebugInfo] Add debug locations to constant SD nodes"

[Keith Walker]

Sergey,

You have my vote (for what it's worth) on making the change in the
constructor in order to restore the previous behaviour without necessarily
reverting all the API changes.

However other more experienced people who know the code in this area might
have a different view.

Keith

> -----Original Message-----
> From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-
> bounces at cs.uiuc.edu] On Behalf Of Sergey Dmitrouk
> Sent: 07 May 2015 11:07
> To: David Blaikie
> Cc: llvm-commits at cs.uiuc.edu
> Subject: Re: [llvm] r235989 - Reapply r235977 "[DebugInfo] Add debug
> locations to constant SD nodes"
> 
> On Wed, May 06, 2015 at 04:05:54PM -0700, David Blaikie wrote:
> >    This seems to produce some regressions, for example:
> >
> >    https://www.diffchecker.com/p2ladh2w
> >
> >    The constant used in both the inlined function and outside the
> inlined
> >    function is attributed to the first use (the inlined function), but
> then
> >    the codegen for the inlined function's constant ends up morphed
> into a
> >    setg and inc, so the constant used outside the inlined function
> ends up
> >    after some of the outer code, but attributed to it...
> >
> >    GDB doesn't seem to trip over this (but in theory, stepping through
> this
> >    code should cause the debugger to go in/out of the inlined function
> more
> >    than once) but does seem incorrect.
> 
> It's basically different manifestation of the issue noted by Keith.
> First ConstantSDNode for some value gets cached and used in all
> appearances
> of the same constant, as debug location is bound to ConstantSDNode all
> such
> constants get location of the first one.
> 
> Strictly saying this is not a regression, it just happened to be more or
> less correct before this change, but it was more of a luck than
> intention.
> 
> Do you want me to disable the change until I figure out how to solve
> this issue with constants?  Not reverting the commit, just dropping
> using
> location argument in ConstantSDNode constructor, so that API changes
> will still be there.
> 
> >    Also, this causes substantial increases to debug info size,
> especially for
> >    -gmlt, due to many of these discontiguous ranges... that seems
> unfortunate
> >    (though some of them may be legitimate).
> 
> Not sure this will go away completely, looks like it's either this or
> wrong
> locations, but if constants will get correct locations, ranges should
> become more contiguous.
> 
> --
> Sergey
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits