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

[Xinliang David Li <davidxl at google dot com>]

On Sun, Nov 25, 2012 at 4:40 AM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Thu, Nov 15, 2012 at 5:46 PM, 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.
>
> There may not be a source location for a generated statement, the computed
> value might not have been computed in the source at any point even.  Consider
> re-association of an expression and then a re-associated part becoming
> partially redundant.
>
>  if ()
>    tem = a + b;
>  tem2 = a + c  + b;
>
> after re-assoc + PRE:
>
>  if ()
>    tem = a + b;
> else
>    tem' = a + b;  // which sloc here?

If there was a else branch, the proposed patch by Dehao will use the
source location of the previous stmt 'tem' = a + b' was inserted
after.

I did not see any special source location handling in
tree-ssa-reassoc.c -- looks like the same principle is used there --
the new assignment after the operand shuffling will inherit the source
location of the placeholder gimple stmt.


> tem'' = PHI <tem, tem'>  //  or here?  on the args?
> tem2 = tem'' + c;
>
> so what source location would you use for the inserted expression?  If
> UNKNOWN is not
> correct here then I think we need an explicit NOWHERE?

Can that break line coverage info too, say, when source location get
stripped from reassociated expressions?

David




>
>>> 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.
>>
>> 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.
>
> Richard.
>
>> --
>> Eric Botcazou