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

Sergey Dmitrouk sdmitrouk at accesssoftek.com
Fri May 1 11:05:03 PDT 2015


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



More information about the llvm-commits mailing list