[cfe-dev] Track locations of individual loads in -Rpass remarks (Re: [polly] Add diagnostic remark for ReportVariantBasePtr)

[Tobias Grosser]

On 28/07/2014 20:10, Eric Christopher wrote:
> On Mon, Jul 28, 2014 at 10:38 AM, Diego Novillo <dnovillo at google.com> wrote:
>> On Thu, Jul 24, 2014 at 4:16 PM, Diego Novillo <dnovillo at google.com> wrote:
>>> On Mon, Jul 21, 2014 at 9:36 AM, Tobias Grosser <tobias at grosser.es> wrote:
>>>
>>>> Diego, you just recently run the -gcolumn-info test cases on your internal
>>>> code base and on the gdb test suite. Would it be difficult for
>>>> you to create similar information for the attached patch to get an idea of
>>>> how much overhead this patch would cause?
>>>
>>> A full comparison build is now running. I should have results soon. In the
>>> meantime, I tried it on a target that generates a 30Mb shared object:
>>>
>>> Without your patch, I see these segment sizes:
>>>
>>>       text      data       bss     debug debug.dwo    symtab    strtab
>>> other     total filename
>>>    8505260    444768    308624  12140854         0   1226952   5631742
>>> 1632669  29588116 file.so
>>>
>>>
>>> With your patch:
>>>       text      data       bss     debug debug.dwo    symtab    strtab
>>> other     total filename
>>>    8505260    444768    308624  12565490         0   1226952   5631742
>>> 1632669  30012748 file.so
>>>
>>> That's a 3.5% growth in debug info size, for a total size increase of ~1%.
>>>
>>> I'll send numbers over a full build as soon as I get them.
>>
>> I've got full numbers now. The extreme I chose was an outlier. I've
>> done full builds over our code base. The increase on average debug
>> sizes is 0.1%. The total file size increase is ~0%.
>>
>> I don't think your change will make a substantial difference in debug
>> info sizes. At least, not in what I've observed.
>>
>
> I'd have been surprised if it had, but thanks for running the numbers.

Thanks Diego for testing. This is very much appreciated. It is good to 
run this on a large code basis to gain confidence in this change.

> Tobi: Go ahead and commit it if you'd like. Please go ahead and watch
> the bots to make sure there aren't any failures (they'll likely be
> spurious, but we should watch out for them.

Thanks. As I am not a debug system expert, it is good to confirm this is 
a reasonable approach.

> One bit of review on the patch, looks like there are extra braces
> since the bit after the conditional is a single line?

Right, I also added a test case and updated three other test cases
which checked full debug informations and where the column information
of loads now changed.

r214162

Thanks,
Tobias