Re: [PATCH 00/19] cleanup of memory stats prototypes

2017-08-01 Thread Richard Biener
On Tue, Aug 1, 2017 at 2:11 AM, Martin Sebor  wrote:
> On 07/31/2017 05:11 PM, Trevor Saunders wrote:
>>
>> On Mon, Jul 31, 2017 at 04:58:15PM -0600, Martin Sebor wrote:
>>>
>>> On 07/31/2017 03:34 PM, Trevor Saunders wrote:

 On Mon, Jul 31, 2017 at 02:56:40PM -0600, Martin Sebor wrote:
>
> On 07/27/2017 02:30 AM, tbsaunde+...@tbsaunde.org wrote:
>>
>> From: Trevor Saunders 
>>
>> The preC++ way of passing information about the call site of a
>> function was to
>> use a macro that passed __file__, __LINE__, and __FUNCTION__ to a
>> function with
>> the same name with _stat appended to it.  The way this is now done
>> with C++ is
>> to have arguments where the default value is __LINE__, __FILE__, and
>> __FUNCTION__ in the caller.  This has the significant advantage that
>> if you
>> look for "^function (" you find the correct function, where in the C
>> way of
>> doing things you need to realize its a macro and check the definition
>> of the
>> macro to see what to look for next.  So this removes a layer of
>> indirection,
>> and makes things somewhat more consistant in using the C++ way of
>> doing things.
>
>
> So that's what these things are for! :)
>
> I must be missing something either about the macros or about your
> changes.  The way I read the changes it seems that it's no longer
> possible to call, say,
>
>   t = make_node (code);
>
> and have __FILE__, __LINE__, and __FUNCTION__ passed as arguments
> behind the scenes.
>
> Instead, to achieve that, make_node has to be called like so
>
>   t = make_node (code PASS_MEM_STAT);
>
> Otherwise calling make_node() with just one argument will end up
> calling it with the defaults.


 calling it with the defaults will do the same thing the macro used to
 do, so its fine unless you want to pass in a location that you got as an
 argument.
>>>
>>>
>>> Sorry, I'm still not sure I understand.  Please bear with me
>>> if I'm just being slow.
>>>
>>> When GATHER_STATISTICS is defined, make_node_stat is declared
>>> like so in current GCC (without your patch):
>>>
>>>   extern tree
>>>   make_node_stat (enum tree_code , const char * _loc_name,
>>>   int _loc_line,  const char * _loc_function);
>>>
>>> and the make_node macro like so:
>>>
>>>   #define make_node(t) make_node_stat (t MEM_STAT_INFO)
>>>
>>> with MEM_STAT_INFO being defined as follows:
>>>
>>>   #define MEM_STAT_INFO , ALONE_MEM_STAT_INFO
>>>   #define ALONE_MEM_STAT_INFO __FILE__, __LINE__, __FUNCTION__
>>>
>>> So a call to
>>>
>>>   t = make_node (code);
>>>
>>> expands to
>>>
>>>   t = make_node_stat (code, __FILE__, __LINE__, __FUNCTION__);
>>>
>>> If I'm reading your patch correctly (please correct me if/where
>>> I'm not) then it treats the same call as just an ordinary call
>>> to the make_node function with no macro expansion taking place
>>> at the call  site.  I.e., it will be made with the _loc_name,
>>> _loc_line, and _loc_function arguments set to whatever their
>>> defaults are where the function is declared.  To have the call
>>> do the same thing as before it will have to be made with explicit
>>> arguments, e.g., like so:
>>>
>>>   t = make_node (code MEM_STAT_INFO);
>>>
>>> That seems like a (significant) difference.
>>>
 I'm guessing you are missing that the functions when used as the default
 argument provide the location of the call site of the function, not the
 location of the prototype.  Which is not the most obvious thing.
>>>
>>>
>>> I am indeed missing that.  How do/can they do that when that's
>>> not how C++ 98 default arguments work?  (It's only possible with
>>> GCC's __builtin_{FILE,LINE,FUNCTION} but not without these
>>> extensions.)
>>
>>
>> They use the extensions if available, and otherwise provide the wrong
>> values.  However that's fine because this is just for profiling gcc
>> itself so demanding you use gcc 4.8 or greater, or bootstrap the
>> compiler you are going to profile isn't a big deal.
>
>
> Ah, okay, thanks for your patience with me.  I didn't realize
> some language features/extensions get enabled as GCC bootstraps.
>
>>
>> perhaps we should just make --enable-gather-detailed-mem-statss require
>> a recent gcc instead of supporting it but providing bad data for some
>> things?

My plan was to remove the fallback (so you'd get no data).  But yes,
a configure test for the extension would work as well of course.

Note it only affects non-bootstrapped builds or stage1 so the configure
test would be a bit tricky?

Of course I never use mem-stats for a bootstrapped compiler.

Richard.

>
> That would make sense to me.  Either that, or documenting this
> caveat as a known limitation somewhere users of the option will
> notice (or both).
>
> Martin


Re: [PATCH 00/19] cleanup of memory stats prototypes

2017-07-31 Thread Martin Sebor

On 07/31/2017 05:11 PM, Trevor Saunders wrote:

On Mon, Jul 31, 2017 at 04:58:15PM -0600, Martin Sebor wrote:

On 07/31/2017 03:34 PM, Trevor Saunders wrote:

On Mon, Jul 31, 2017 at 02:56:40PM -0600, Martin Sebor wrote:

On 07/27/2017 02:30 AM, tbsaunde+...@tbsaunde.org wrote:

From: Trevor Saunders 

The preC++ way of passing information about the call site of a function was to
use a macro that passed __file__, __LINE__, and __FUNCTION__ to a function with
the same name with _stat appended to it.  The way this is now done with C++ is
to have arguments where the default value is __LINE__, __FILE__, and
__FUNCTION__ in the caller.  This has the significant advantage that if you
look for "^function (" you find the correct function, where in the C way of
doing things you need to realize its a macro and check the definition of the
macro to see what to look for next.  So this removes a layer of indirection,
and makes things somewhat more consistant in using the C++ way of doing things.


So that's what these things are for! :)

I must be missing something either about the macros or about your
changes.  The way I read the changes it seems that it's no longer
possible to call, say,

  t = make_node (code);

and have __FILE__, __LINE__, and __FUNCTION__ passed as arguments
behind the scenes.

Instead, to achieve that, make_node has to be called like so

  t = make_node (code PASS_MEM_STAT);

Otherwise calling make_node() with just one argument will end up
calling it with the defaults.


calling it with the defaults will do the same thing the macro used to
do, so its fine unless you want to pass in a location that you got as an
argument.


Sorry, I'm still not sure I understand.  Please bear with me
if I'm just being slow.

When GATHER_STATISTICS is defined, make_node_stat is declared
like so in current GCC (without your patch):

  extern tree
  make_node_stat (enum tree_code , const char * _loc_name,
  int _loc_line,  const char * _loc_function);

and the make_node macro like so:

  #define make_node(t) make_node_stat (t MEM_STAT_INFO)

with MEM_STAT_INFO being defined as follows:

  #define MEM_STAT_INFO , ALONE_MEM_STAT_INFO
  #define ALONE_MEM_STAT_INFO __FILE__, __LINE__, __FUNCTION__

So a call to

  t = make_node (code);

expands to

  t = make_node_stat (code, __FILE__, __LINE__, __FUNCTION__);

If I'm reading your patch correctly (please correct me if/where
I'm not) then it treats the same call as just an ordinary call
to the make_node function with no macro expansion taking place
at the call  site.  I.e., it will be made with the _loc_name,
_loc_line, and _loc_function arguments set to whatever their
defaults are where the function is declared.  To have the call
do the same thing as before it will have to be made with explicit
arguments, e.g., like so:

  t = make_node (code MEM_STAT_INFO);

That seems like a (significant) difference.


I'm guessing you are missing that the functions when used as the default
argument provide the location of the call site of the function, not the
location of the prototype.  Which is not the most obvious thing.


I am indeed missing that.  How do/can they do that when that's
not how C++ 98 default arguments work?  (It's only possible with
GCC's __builtin_{FILE,LINE,FUNCTION} but not without these
extensions.)


They use the extensions if available, and otherwise provide the wrong
values.  However that's fine because this is just for profiling gcc
itself so demanding you use gcc 4.8 or greater, or bootstrap the
compiler you are going to profile isn't a big deal.


Ah, okay, thanks for your patience with me.  I didn't realize
some language features/extensions get enabled as GCC bootstraps.



perhaps we should just make --enable-gather-detailed-mem-statss require
a recent gcc instead of supporting it but providing bad data for some
things?


That would make sense to me.  Either that, or documenting this
caveat as a known limitation somewhere users of the option will
notice (or both).

Martin


Re: [PATCH 00/19] cleanup of memory stats prototypes

2017-07-31 Thread Trevor Saunders
On Mon, Jul 31, 2017 at 04:58:15PM -0600, Martin Sebor wrote:
> On 07/31/2017 03:34 PM, Trevor Saunders wrote:
> > On Mon, Jul 31, 2017 at 02:56:40PM -0600, Martin Sebor wrote:
> > > On 07/27/2017 02:30 AM, tbsaunde+...@tbsaunde.org wrote:
> > > > From: Trevor Saunders 
> > > > 
> > > > The preC++ way of passing information about the call site of a function 
> > > > was to
> > > > use a macro that passed __file__, __LINE__, and __FUNCTION__ to a 
> > > > function with
> > > > the same name with _stat appended to it.  The way this is now done with 
> > > > C++ is
> > > > to have arguments where the default value is __LINE__, __FILE__, and
> > > > __FUNCTION__ in the caller.  This has the significant advantage that if 
> > > > you
> > > > look for "^function (" you find the correct function, where in the C 
> > > > way of
> > > > doing things you need to realize its a macro and check the definition 
> > > > of the
> > > > macro to see what to look for next.  So this removes a layer of 
> > > > indirection,
> > > > and makes things somewhat more consistant in using the C++ way of doing 
> > > > things.
> > > 
> > > So that's what these things are for! :)
> > > 
> > > I must be missing something either about the macros or about your
> > > changes.  The way I read the changes it seems that it's no longer
> > > possible to call, say,
> > > 
> > >   t = make_node (code);
> > > 
> > > and have __FILE__, __LINE__, and __FUNCTION__ passed as arguments
> > > behind the scenes.
> > > 
> > > Instead, to achieve that, make_node has to be called like so
> > > 
> > >   t = make_node (code PASS_MEM_STAT);
> > > 
> > > Otherwise calling make_node() with just one argument will end up
> > > calling it with the defaults.
> > 
> > calling it with the defaults will do the same thing the macro used to
> > do, so its fine unless you want to pass in a location that you got as an
> > argument.
> 
> Sorry, I'm still not sure I understand.  Please bear with me
> if I'm just being slow.
> 
> When GATHER_STATISTICS is defined, make_node_stat is declared
> like so in current GCC (without your patch):
> 
>   extern tree
>   make_node_stat (enum tree_code , const char * _loc_name,
>   int _loc_line,  const char * _loc_function);
> 
> and the make_node macro like so:
> 
>   #define make_node(t) make_node_stat (t MEM_STAT_INFO)
> 
> with MEM_STAT_INFO being defined as follows:
> 
>   #define MEM_STAT_INFO , ALONE_MEM_STAT_INFO
>   #define ALONE_MEM_STAT_INFO __FILE__, __LINE__, __FUNCTION__
> 
> So a call to
> 
>   t = make_node (code);
> 
> expands to
> 
>   t = make_node_stat (code, __FILE__, __LINE__, __FUNCTION__);
> 
> If I'm reading your patch correctly (please correct me if/where
> I'm not) then it treats the same call as just an ordinary call
> to the make_node function with no macro expansion taking place
> at the call  site.  I.e., it will be made with the _loc_name,
> _loc_line, and _loc_function arguments set to whatever their
> defaults are where the function is declared.  To have the call
> do the same thing as before it will have to be made with explicit
> arguments, e.g., like so:
> 
>   t = make_node (code MEM_STAT_INFO);
> 
> That seems like a (significant) difference.
> 
> > I'm guessing you are missing that the functions when used as the default
> > argument provide the location of the call site of the function, not the
> > location of the prototype.  Which is not the most obvious thing.
> 
> I am indeed missing that.  How do/can they do that when that's
> not how C++ 98 default arguments work?  (It's only possible with
> GCC's __builtin_{FILE,LINE,FUNCTION} but not without these
> extensions.)

They use the extensions if available, and otherwise provide the wrong
values.  However that's fine because this is just for profiling gcc
itself so demanding you use gcc 4.8 or greater, or bootstrap the
compiler you are going to profile isn't a big deal.

perhaps we should just make --enable-gather-detailed-mem-statss require
a recent gcc instead of supporting it but providing bad data for some
things?

Trev

> 
> Martin
> 


Re: [PATCH 00/19] cleanup of memory stats prototypes

2017-07-31 Thread Martin Sebor

On 07/31/2017 03:34 PM, Trevor Saunders wrote:

On Mon, Jul 31, 2017 at 02:56:40PM -0600, Martin Sebor wrote:

On 07/27/2017 02:30 AM, tbsaunde+...@tbsaunde.org wrote:

From: Trevor Saunders 

The preC++ way of passing information about the call site of a function was to
use a macro that passed __file__, __LINE__, and __FUNCTION__ to a function with
the same name with _stat appended to it.  The way this is now done with C++ is
to have arguments where the default value is __LINE__, __FILE__, and
__FUNCTION__ in the caller.  This has the significant advantage that if you
look for "^function (" you find the correct function, where in the C way of
doing things you need to realize its a macro and check the definition of the
macro to see what to look for next.  So this removes a layer of indirection,
and makes things somewhat more consistant in using the C++ way of doing things.


So that's what these things are for! :)

I must be missing something either about the macros or about your
changes.  The way I read the changes it seems that it's no longer
possible to call, say,

  t = make_node (code);

and have __FILE__, __LINE__, and __FUNCTION__ passed as arguments
behind the scenes.

Instead, to achieve that, make_node has to be called like so

  t = make_node (code PASS_MEM_STAT);

Otherwise calling make_node() with just one argument will end up
calling it with the defaults.


calling it with the defaults will do the same thing the macro used to
do, so its fine unless you want to pass in a location that you got as an
argument.


Sorry, I'm still not sure I understand.  Please bear with me
if I'm just being slow.

When GATHER_STATISTICS is defined, make_node_stat is declared
like so in current GCC (without your patch):

  extern tree
  make_node_stat (enum tree_code , const char * _loc_name,
  int _loc_line,  const char * _loc_function);

and the make_node macro like so:

  #define make_node(t) make_node_stat (t MEM_STAT_INFO)

with MEM_STAT_INFO being defined as follows:

  #define MEM_STAT_INFO , ALONE_MEM_STAT_INFO
  #define ALONE_MEM_STAT_INFO __FILE__, __LINE__, __FUNCTION__

So a call to

  t = make_node (code);

expands to

  t = make_node_stat (code, __FILE__, __LINE__, __FUNCTION__);

If I'm reading your patch correctly (please correct me if/where
I'm not) then it treats the same call as just an ordinary call
to the make_node function with no macro expansion taking place
at the call  site.  I.e., it will be made with the _loc_name,
_loc_line, and _loc_function arguments set to whatever their
defaults are where the function is declared.  To have the call
do the same thing as before it will have to be made with explicit
arguments, e.g., like so:

  t = make_node (code MEM_STAT_INFO);

That seems like a (significant) difference.


I'm guessing you are missing that the functions when used as the default
argument provide the location of the call site of the function, not the
location of the prototype.  Which is not the most obvious thing.


I am indeed missing that.  How do/can they do that when that's
not how C++ 98 default arguments work?  (It's only possible with
GCC's __builtin_{FILE,LINE,FUNCTION} but not without these
extensions.)

Martin



Re: [PATCH 00/19] cleanup of memory stats prototypes

2017-07-31 Thread Trevor Saunders
On Mon, Jul 31, 2017 at 02:56:40PM -0600, Martin Sebor wrote:
> On 07/27/2017 02:30 AM, tbsaunde+...@tbsaunde.org wrote:
> > From: Trevor Saunders 
> > 
> > The preC++ way of passing information about the call site of a function was 
> > to
> > use a macro that passed __file__, __LINE__, and __FUNCTION__ to a function 
> > with
> > the same name with _stat appended to it.  The way this is now done with C++ 
> > is
> > to have arguments where the default value is __LINE__, __FILE__, and
> > __FUNCTION__ in the caller.  This has the significant advantage that if you
> > look for "^function (" you find the correct function, where in the C way of
> > doing things you need to realize its a macro and check the definition of the
> > macro to see what to look for next.  So this removes a layer of indirection,
> > and makes things somewhat more consistant in using the C++ way of doing 
> > things.
> 
> So that's what these things are for! :)
> 
> I must be missing something either about the macros or about your
> changes.  The way I read the changes it seems that it's no longer
> possible to call, say,
> 
>   t = make_node (code);
> 
> and have __FILE__, __LINE__, and __FUNCTION__ passed as arguments
> behind the scenes.
> 
> Instead, to achieve that, make_node has to be called like so
> 
>   t = make_node (code PASS_MEM_STAT);
> 
> Otherwise calling make_node() with just one argument will end up
> calling it with the defaults.

calling it with the defaults will do the same thing the macro used to
do, so its fine unless you want to pass in a location that you got as an
argument.

I'm guessing you are missing that the functions when used as the default
argument provide the location of the call site of the function, not the
location of the prototype.  Which is not the most obvious thing.

Trev

> 
> Martin
> 


Re: [PATCH 00/19] cleanup of memory stats prototypes

2017-07-31 Thread Martin Sebor

On 07/27/2017 02:30 AM, tbsaunde+...@tbsaunde.org wrote:

From: Trevor Saunders 

The preC++ way of passing information about the call site of a function was to
use a macro that passed __file__, __LINE__, and __FUNCTION__ to a function with
the same name with _stat appended to it.  The way this is now done with C++ is
to have arguments where the default value is __LINE__, __FILE__, and
__FUNCTION__ in the caller.  This has the significant advantage that if you
look for "^function (" you find the correct function, where in the C way of
doing things you need to realize its a macro and check the definition of the
macro to see what to look for next.  So this removes a layer of indirection,
and makes things somewhat more consistant in using the C++ way of doing things.


So that's what these things are for! :)

I must be missing something either about the macros or about your
changes.  The way I read the changes it seems that it's no longer
possible to call, say,

  t = make_node (code);

and have __FILE__, __LINE__, and __FUNCTION__ passed as arguments
behind the scenes.

Instead, to achieve that, make_node has to be called like so

  t = make_node (code PASS_MEM_STAT);

Otherwise calling make_node() with just one argument will end up
calling it with the defaults.

Martin



Re: [PATCH 00/19] cleanup of memory stats prototypes

2017-07-31 Thread Jeff Law
On 07/27/2017 02:43 AM, Richard Biener wrote:
> On Thu, Jul 27, 2017 at 10:30 AM,   wrote:
>> From: Trevor Saunders 
>>
>> The preC++ way of passing information about the call site of a function was 
>> to
>> use a macro that passed __file__, __LINE__, and __FUNCTION__ to a function 
>> with
>> the same name with _stat appended to it.  The way this is now done with C++ 
>> is
>> to have arguments where the default value is __LINE__, __FILE__, and
>> __FUNCTION__ in the caller.  This has the significant advantage that if you
>> look for "^function (" you find the correct function, where in the C way of
>> doing things you need to realize its a macro and check the definition of the
>> macro to see what to look for next.  So this removes a layer of indirection,
>> and makes things somewhat more consistant in using the C++ way of doing 
>> things.
>>
>> patches independently bootstrapped and regtested on ppc64le-linux-gnu.  I
>> successfully ran make all-gcc with --enable-gather-detailed-mem-stats, but
>> couldn't complete a bootstrap before the series was applied, because the
>> ddrs_table in tree-loop-distribution.c causes memory statistics gathering to 
>> crash before the series as well as after it.  ok?
> 
> Thanks!  This was on my list of things todo...
> 
> The series is ok.
Just wanted to relay a thanks from me too.  I always find it annoying to
remember which functions are _stat thingies when debugging.  No more!

Jeff


Re: [PATCH 00/19] cleanup of memory stats prototypes

2017-07-28 Thread Trevor Saunders
On Thu, Jul 27, 2017 at 10:43:09AM +0200, Richard Biener wrote:
> On Thu, Jul 27, 2017 at 10:30 AM,   wrote:
> > From: Trevor Saunders 
> >
> > The preC++ way of passing information about the call site of a function was 
> > to
> > use a macro that passed __file__, __LINE__, and __FUNCTION__ to a function 
> > with
> > the same name with _stat appended to it.  The way this is now done with C++ 
> > is
> > to have arguments where the default value is __LINE__, __FILE__, and
> > __FUNCTION__ in the caller.  This has the significant advantage that if you
> > look for "^function (" you find the correct function, where in the C way of
> > doing things you need to realize its a macro and check the definition of the
> > macro to see what to look for next.  So this removes a layer of indirection,
> > and makes things somewhat more consistant in using the C++ way of doing 
> > things.
> >
> > patches independently bootstrapped and regtested on ppc64le-linux-gnu.  I
> > successfully ran make all-gcc with --enable-gather-detailed-mem-stats, but
> > couldn't complete a bootstrap before the series was applied, because the
> > ddrs_table in tree-loop-distribution.c causes memory statistics gathering 
> > to crash before the series as well as after it.  ok?
> 
> Thanks!  This was on my list of things todo...

no problem, it kept annoying me to find things.

> The series is ok.
> 
> Did you catch all of MEM_STAT_INFO/ALONE_MEM_STAT_INFO so we can remove the
> non-C++ way from statistics.h?

 Looks like ALONE_MEM_STAT_INFO is now effectively unused, but a few odd
 uses of MEM_STAT_INFO remain.  I'll try and look at finishing that off.

 Trev



Re: [PATCH 00/19] cleanup of memory stats prototypes

2017-07-28 Thread Trevor Saunders
On Fri, Jul 28, 2017 at 06:31:05PM +0200, Bernhard Reutner-Fischer wrote:
> On 27 July 2017 10:43:09 CEST, Richard Biener  
> wrote:
> >On Thu, Jul 27, 2017 at 10:30 AM,   wrote:
> >> From: Trevor Saunders 
> >>
> >> The preC++ way of passing information about the call site of a
> >function was to
> >> use a macro that passed __file__, __LINE__, and __FUNCTION__ to a
> >function with
> >> the same name with _stat appended to it.  The way this is now done
> >with C++ is
> >> to have arguments where the default value is __LINE__, __FILE__, and
> >> __FUNCTION__ in the caller.  This has the significant advantage that
> >if you
> >> look for "^function (" you find the correct function, where in the C
> >way of
> >> doing things you need to realize its a macro and check the definition
> >of the
> >> macro to see what to look for next.  So this removes a layer of
> >indirection,
> >> and makes things somewhat more consistant in using the C++ way of
> >doing things.
> >>
> >> patches independently bootstrapped and regtested on
> >ppc64le-linux-gnu.  I
> >> successfully ran make all-gcc with
> >--enable-gather-detailed-mem-stats, but
> >> couldn't complete a bootstrap before the series was applied, because
> >the
> >> ddrs_table in tree-loop-distribution.c causes memory statistics
> >gathering to crash before the series as well as after it.  ok?
> >
> >Thanks!  This was on my list of things todo...
> >
> >The series is ok.
> 
> Maybe s/enum tree_code/tree_code/ in the lines you touch?

I'd rather leave that to a separate patch and just do it for all of gcc/
at once.

Trev



Re: [PATCH 00/19] cleanup of memory stats prototypes

2017-07-28 Thread Bernhard Reutner-Fischer
On 27 July 2017 10:43:09 CEST, Richard Biener  
wrote:
>On Thu, Jul 27, 2017 at 10:30 AM,   wrote:
>> From: Trevor Saunders 
>>
>> The preC++ way of passing information about the call site of a
>function was to
>> use a macro that passed __file__, __LINE__, and __FUNCTION__ to a
>function with
>> the same name with _stat appended to it.  The way this is now done
>with C++ is
>> to have arguments where the default value is __LINE__, __FILE__, and
>> __FUNCTION__ in the caller.  This has the significant advantage that
>if you
>> look for "^function (" you find the correct function, where in the C
>way of
>> doing things you need to realize its a macro and check the definition
>of the
>> macro to see what to look for next.  So this removes a layer of
>indirection,
>> and makes things somewhat more consistant in using the C++ way of
>doing things.
>>
>> patches independently bootstrapped and regtested on
>ppc64le-linux-gnu.  I
>> successfully ran make all-gcc with
>--enable-gather-detailed-mem-stats, but
>> couldn't complete a bootstrap before the series was applied, because
>the
>> ddrs_table in tree-loop-distribution.c causes memory statistics
>gathering to crash before the series as well as after it.  ok?
>
>Thanks!  This was on my list of things todo...
>
>The series is ok.

Maybe s/enum tree_code/tree_code/ in the lines you touch?

Thanks,
>
>Did you catch all of MEM_STAT_INFO/ALONE_MEM_STAT_INFO so we can remove
>the
>non-C++ way from statistics.h?



Re: [PATCH 00/19] cleanup of memory stats prototypes

2017-07-27 Thread Richard Biener
On Thu, Jul 27, 2017 at 10:30 AM,   wrote:
> From: Trevor Saunders 
>
> The preC++ way of passing information about the call site of a function was to
> use a macro that passed __file__, __LINE__, and __FUNCTION__ to a function 
> with
> the same name with _stat appended to it.  The way this is now done with C++ is
> to have arguments where the default value is __LINE__, __FILE__, and
> __FUNCTION__ in the caller.  This has the significant advantage that if you
> look for "^function (" you find the correct function, where in the C way of
> doing things you need to realize its a macro and check the definition of the
> macro to see what to look for next.  So this removes a layer of indirection,
> and makes things somewhat more consistant in using the C++ way of doing 
> things.
>
> patches independently bootstrapped and regtested on ppc64le-linux-gnu.  I
> successfully ran make all-gcc with --enable-gather-detailed-mem-stats, but
> couldn't complete a bootstrap before the series was applied, because the
> ddrs_table in tree-loop-distribution.c causes memory statistics gathering to 
> crash before the series as well as after it.  ok?

Thanks!  This was on my list of things todo...

The series is ok.

Did you catch all of MEM_STAT_INFO/ALONE_MEM_STAT_INFO so we can remove the
non-C++ way from statistics.h?

Richard.

> thanks
>
> Trev
>
> p.s. the issue with ddrs_table is that it ends up trying to add to the memory
> stats hash table from a global constructor when that hash table hasn't yet 
> been
> constructed.
>
>
> Trevor Saunders (19):
>   use c++ instead of make_node_stat
>   use c++ instead of _stat for copy_node_stat
>   use cxx instead of make_tree_binfo_stat
>   use c++ for make_int_cst_stat
>   use c++ instead of buildN_stat{,_loc}
>   use c++ instead of {make,grow}_tree_vec_stat
>   replace gimple_alloc_stat with c++
>   use c++ instead of build_decl_stat
>   use c++ instead of build_vl_exp_stat
>   use c++ for tree_cons_stat
>   remove unused build_var_debug_value prototype
>   use C++ for {make,build}_vector_stat
>   use c++ for build_tree_list{,_vec}_stat
>   replace rtx_alloc_stat with c++
>   replace shallow_copy_rtx_stat with c++
>   simplify the bitmap alloc_stat functions with c++
>   use c++ for bitmap_initialize
>   use c++ for gimple_build_debug_bind{,_source}
>   use c++ for fold_buildN_loc
>
>  gcc/bitmap.c  |   8 ++--
>  gcc/bitmap.h  |  17 +++-
>  gcc/cp/lex.c  |   4 +-
>  gcc/emit-rtl.c|   2 +-
>  gcc/fold-const.c  |  14 +++
>  gcc/fold-const.h  |  24 +---
>  gcc/fortran/resolve.c |   2 +-
>  gcc/gengenrtl.c   |   2 +-
>  gcc/gimple.c  |   8 ++--
>  gcc/gimple.h  |  11 ++
>  gcc/rtl.c |   4 +-
>  gcc/rtl.h |   6 +--
>  gcc/tree.c|  62 ++---
>  gcc/tree.h| 106 
> ++
>  14 files changed, 109 insertions(+), 161 deletions(-)
>
> --
> 2.11.0
>


[PATCH 00/19] cleanup of memory stats prototypes

2017-07-27 Thread tbsaunde+gcc
From: Trevor Saunders 

The preC++ way of passing information about the call site of a function was to
use a macro that passed __file__, __LINE__, and __FUNCTION__ to a function with
the same name with _stat appended to it.  The way this is now done with C++ is
to have arguments where the default value is __LINE__, __FILE__, and
__FUNCTION__ in the caller.  This has the significant advantage that if you
look for "^function (" you find the correct function, where in the C way of
doing things you need to realize its a macro and check the definition of the
macro to see what to look for next.  So this removes a layer of indirection,
and makes things somewhat more consistant in using the C++ way of doing things.

patches independently bootstrapped and regtested on ppc64le-linux-gnu.  I
successfully ran make all-gcc with --enable-gather-detailed-mem-stats, but
couldn't complete a bootstrap before the series was applied, because the
ddrs_table in tree-loop-distribution.c causes memory statistics gathering to 
crash before the series as well as after it.  ok?

thanks

Trev

p.s. the issue with ddrs_table is that it ends up trying to add to the memory
stats hash table from a global constructor when that hash table hasn't yet been
constructed.


Trevor Saunders (19):
  use c++ instead of make_node_stat
  use c++ instead of _stat for copy_node_stat
  use cxx instead of make_tree_binfo_stat
  use c++ for make_int_cst_stat
  use c++ instead of buildN_stat{,_loc}
  use c++ instead of {make,grow}_tree_vec_stat
  replace gimple_alloc_stat with c++
  use c++ instead of build_decl_stat
  use c++ instead of build_vl_exp_stat
  use c++ for tree_cons_stat
  remove unused build_var_debug_value prototype
  use C++ for {make,build}_vector_stat
  use c++ for build_tree_list{,_vec}_stat
  replace rtx_alloc_stat with c++
  replace shallow_copy_rtx_stat with c++
  simplify the bitmap alloc_stat functions with c++
  use c++ for bitmap_initialize
  use c++ for gimple_build_debug_bind{,_source}
  use c++ for fold_buildN_loc

 gcc/bitmap.c  |   8 ++--
 gcc/bitmap.h  |  17 +++-
 gcc/cp/lex.c  |   4 +-
 gcc/emit-rtl.c|   2 +-
 gcc/fold-const.c  |  14 +++
 gcc/fold-const.h  |  24 +---
 gcc/fortran/resolve.c |   2 +-
 gcc/gengenrtl.c   |   2 +-
 gcc/gimple.c  |   8 ++--
 gcc/gimple.h  |  11 ++
 gcc/rtl.c |   4 +-
 gcc/rtl.h |   6 +--
 gcc/tree.c|  62 ++---
 gcc/tree.h| 106 ++
 14 files changed, 109 insertions(+), 161 deletions(-)

-- 
2.11.0