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

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

On Sun, Nov 4, 2012 at 8:07 AM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Wed, Oct 31, 2012 at 8:02 PM, Xinliang David Li <davidxl@google.com> wrote:
>> Dehao's patch will make the debugging of the following code (-g -O2)
>> less jumpy.   After the testing of x > 0, it should go to line 'a++'.
>>  Without the fix, when stepping through 'abc', the lines covered are
>> 6, 4, 11, 13. With the fix it should be 6, 9, 11, 13 -- much better.
>
> I am not convinced.  Btw, you do not comment my example at all.

Because I was not sure what the example was trying to illustrate.
Let's make it more concrete:

Original program:

if (...)
{
    .. = i + j;
}
else
{
   // (1)
   ..
}

... = i + j;  // (2)

After PRE:

if (...)
{
   t = i + j;
   .. = t;
}
else
{
    // insertion point at 1
    t = i + j;   // New statement
   ..
}

.. = t; // (2)


What is the 'different operation/component' you are referring to in the example?


> For less jumpiness no line number for inserted stmts works as well.


Those compiler generated statements do not have source origins, but
they need to have debug location information attached so that
information collected for them can be correlated with program
constructs (such as CFG). One of the natural way is to use the source
location associated with the program point where they are inserted.

David

>
> Richard.
>
>> David
>>
>>
>>
>>
>> 1. int x;
>>
>> 2. __attribute__((noinline)) int abc (int *a)
>> 3. {
>> 4.  int ret = 0;
>> 5.
>> 6.  if (x > 0)
>> 7.    ret += *a;
>> 8.  else
>> 9.    a++;
>> 10.
>> 11.  ret += *a;
>> 12.  return ret;
>> 13 }
>>
>>
>> int main()
>> {
>>   int a = 0;
>>
>>    x = -1;
>>    return abc ( &a);
>>
>> }
>>
>> On Wed, Oct 31, 2012 at 2:34 AM, Richard Biener
>> <richard.guenther@gmail.com> wrote:
>>> On Wed, Oct 31, 2012 at 12:57 AM, Xinliang David Li <davidxl@google.com> wrote:
>>>> It will make the location info for the newly synthesized stmt more
>>>> deterministic, I think.
>>>
>>> Maybe, but it will increase the jumpiness in the debugger without actually
>>> being accurate, no?  For example if the partially redundant expression is
>>>
>>>   i + j;
>>>
>>> then when computed at the insertion point the values of i and j do not
>>> necessarily reflect the computed value!  Instead we may compute the
>>> result of i + j using completely different components / operation.
>>>
>>> Thus I think inserted expressions should not have any debug information
>>> at all because they do not correspond to a source line.
>>>
>>> Richard.
>>>
>>>> David
>>>>
>>>> On Tue, Oct 30, 2012 at 4:38 PM, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
>>>>> On Wed, Oct 31, 2012 at 12:00 AM, Dehao Chen wrote:
>>>>>> This patch aims to improve debugging of optimized code. It ensures
>>>>>> that PRE inserted statements have the same source location as the
>>>>>> statement at the insertion point, instead of UNKNOWN_LOCATION.
>>>>>
>>>>> Wrong patch attached.
>>>>>
>>>>> However, is it really better to have the location of the insertion
>>>>> point than to have UNKNOWN_LOCATION? It's not where the value is
>>>>> computed in the source program...
>>>>>
>>>>> Ciao!
>>>>> Steven