Re: [PATCH] Update source location for PRE inserted stmt

[Dehao Chen <dehao at google dot com>]

On Thu, Nov 15, 2012 at 8:46 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>
> > But UNKNOWN_LOCATION is effectively wrong as well.  If other
> > optimizations move the statements above the inserted instruction, then
> > the new instruction ends up inheriting whatever location happens to be
> > in the previous statement.
>
> That's correct, UNKNOWN_LOCATION isn't a panacea either, although it can help
> in some cases.  The only safe thing to do is to put the exact source location
> of the statement, i.e. the location of the source construct for which the
> statement has been generated.
>
> > Fixing the location when the statement is inserted will reduce this
> > randomness.  I'm not sure I understand why you think this will break
> > coverage.  The examples given in this thread were helped by this
> > location fixing.
>
> What do you call randomness exactly?  GDB jumpiness?  I'm a little wary of
> fixing GDB jumpiness by distorting the debug info.  Ian has clearly outlined
> the correct approach to addressing it.

The randomness here means that if we set UNKNOWN_LOCATION to insn, it
can get source location anywhere. Even with some small code layout
changes, the location for that insn could change. I would hope that in
the future, we add an assertion when emitting instruction to enforce
that INSN_LOCATION is never UNKNOWN_LOCATION. So when generate new
instructions/stmts, if we can surely know where it is coming from, it
is fine. Otherwise, instead of using UNKNOWN_LOCATION, we will set its
location to where it is inserted. How does that sound?

Thanks,
Dehao

>
>
> Note that I didn't specifically reply to Dehao's patch here, which might be
> acceptable in the end, only to David's seemingly general statement about the
> "natural way" of putting a location on these statements.
>
> --
> Eric Botcazou