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

[Keith Walker]

Sergey,

Thanks for looking at the cause of why the line information for the constant
was being associated with the wrong line; at least I now understand why it
is occurring ..... did your experimentations suggested whether there was any
simple solution for this case?

Keith

> -----Original Message-----
> From: Sergey Dmitrouk [mailto:sdmitrouk at accesssoftek.com]
> Sent: 01 May 2015 19:05
> To: Keith Walker; Renato Golin
> Cc: LLVM Commits
> Subject: Re: [llvm] r235989 - Reapply r235977 "[DebugInfo] Add debug
> locations to constant SD nodes"
> 
> Thanks for spotting this Keith, the intension is to get better debug
> locations on ARM.  My English can be quite misleading...
> 
> On Fri, May 01, 2015 at 08:26:33AM -0700, Renato Golin wrote:
> > On 1 May 2015 at 15:53, Keith Walker <kwalker at arm.com> wrote:
> > > But after this change there is a "spurious" line entry for the
> instruction
> > > at 0x18 loading the constant 0 which is incorrectly associated with
> the line
> > > 6.
> >
> > From the size of the change, this looks like a bad assumption in one
> > of the unrelated SDLocs.
> 
> Likely in general, but not true in this case.  Here
> SelectionDAG::getConstant() checks for constant with the same value and
> returns previously used node for the line above (there is zero offset in
> store instruction for the first assignment).
> 
> In other words, debug locations are not part of node profile used by
> FoldingSet and I'm not sure whether it's safe to change this even just
> for
> constant nodes.  It will prohibit identical nodes elimination, although
> debug locations should become correct.  I'll try this out.
> 
> > Why haven't you added "SDLoc DL," to the end of the list of arguments?
> > It'd have made the patch a lot smaller and simpler.
> 
> I didn't want to add to existing inconsistency in order of arguments
> among methods of SelectionDAG.  Most of them (not all) tend to have
> DebugLoc
> argument before argument of type EVT, so I did the same.
> 
> Another reason is default arguments, SDLoc is mandatory argument and
> can't be placed after defaulted ones.  This left me with the choice
> before/after EVT.
> 
> Best regards,
> Sergey