Re: [patch] Privatize gimplify_ctx structure.

2013-11-20 Thread Andrew MacLeod
On 11/20/2013 02:58 PM, Jeff Law wrote: On 11/20/13 12:18, Andrew MacLeod wrote: And per Jakubs suggestion, I'll use XNEW... along with the changelog oversite. Assuming that all works, and no regressions, OK? Yup. jeff Bootstrapped on x86_64-unknown-linux-gnu with no new regressions. The

Re: [patch] Privatize gimplify_ctx structure.

2013-11-20 Thread David Malcolm
On Wed, 2013-11-20 at 12:58 -0700, Jeff Law wrote: > On 11/20/13 12:18, Andrew MacLeod wrote: > > On 11/20/2013 01:40 PM, Jeff Law wrote: > >> On 11/20/13 09:47, Andrew MacLeod wrote: > >>> * gimplify.h (gimplify_hasher : typed_free_remove, struct > >>> gimplify_ctx): > >>> Move to gimplify

Re: [patch] Privatize gimplify_ctx structure.

2013-11-20 Thread Jeff Law
On 11/20/13 12:18, Andrew MacLeod wrote: On 11/20/2013 01:40 PM, Jeff Law wrote: On 11/20/13 09:47, Andrew MacLeod wrote: * gimplify.h (gimplify_hasher : typed_free_remove, struct gimplify_ctx): Move to gimplify.c. * gimplify.c (gimplify_hasher:typed_free_remove): Relocate here.

Re: [patch] Privatize gimplify_ctx structure.

2013-11-20 Thread Andrew MacLeod
On 11/20/2013 01:40 PM, Jeff Law wrote: On 11/20/13 09:47, Andrew MacLeod wrote: * gimplify.h (gimplify_hasher : typed_free_remove, struct gimplify_ctx): Move to gimplify.c. * gimplify.c (gimplify_hasher:typed_free_remove): Relocate here. (struct gimplify_ctx): Relocate here and

Re: [patch] Privatize gimplify_ctx structure.

2013-11-20 Thread Jeff Law
On 11/20/13 11:59, Diego Novillo wrote: On Wed, Nov 20, 2013 at 1:40 PM, Jeff Law wrote: Do our coding standards allow using default arguments: extern void push_gimplify_context (bool in_ssa = false, bool rhs_cond_ok = false); Yes, as long as they are not

Re: [patch] Privatize gimplify_ctx structure.

2013-11-20 Thread Diego Novillo
On Wed, Nov 20, 2013 at 1:40 PM, Jeff Law wrote: > Do our coding standards allow using default arguments: > > extern void push_gimplify_context (bool in_ssa = false, >bool rhs_cond_ok = false); Yes, as long as they are not expensive to construct (so, PODs most

Re: [patch] Privatize gimplify_ctx structure.

2013-11-20 Thread Jakub Jelinek
On Wed, Nov 20, 2013 at 11:47:42AM -0500, Andrew MacLeod wrote: > + static inline struct gimplify_ctx * > + ctx_alloc (void) > + { > + struct gimplify_ctx * c = ctx_pool; > + > + if (c) > + ctx_pool = c->prev_context; > + else > + c = (struct gimplify_ctx *) xmalloc (sizeof (struct g

Re: [patch] Privatize gimplify_ctx structure.

2013-11-20 Thread Jeff Law
On 11/20/13 09:47, Andrew MacLeod wrote: * gimplify.h (gimplify_hasher : typed_free_remove, struct gimplify_ctx): Move to gimplify.c. * gimplify.c (gimplify_hasher:typed_free_remove): Relocate here. (struct gimplify_ctx): Relocate here and add 'malloced' field.

Re: [patch] Privatize gimplify_ctx structure.

2013-11-20 Thread Jeff Law
On 11/20/13 06:56, Andrew MacLeod wrote: On 11/20/2013 08:44 AM, Jakub Jelinek wrote: On Wed, Nov 20, 2013 at 08:35:28AM -0500, Andrew MacLeod wrote: I also hacked up the compiler to report what the 'top' of the stack was for a compilation unit. I then ran it through a bootstrap and full testsu

Re: [patch] Privatize gimplify_ctx structure.

2013-11-20 Thread Jeff Law
On 11/20/13 06:44, Jakub Jelinek wrote: On Wed, Nov 20, 2013 at 08:35:28AM -0500, Andrew MacLeod wrote: I also hacked up the compiler to report what the 'top' of the stack was for a compilation unit. I then ran it through a bootstrap and full testsuite run of all languages, and looked for the ma

Re: [patch] Privatize gimplify_ctx structure.

2013-11-20 Thread Andrew MacLeod
On 11/20/2013 11:30 AM, Andrew MacLeod wrote: On 11/20/2013 10:51 AM, Richard Biener wrote: On Wed, Nov 20, 2013 at 4:06 PM, Trevor Saunders wrote: On Wed, Nov 20, 2013 at 03:18:07PM +0100, Richard Biener wrote: On Wed, Nov 20, 2013 at 3:16 PM, Jakub Jelinek wrote: On Wed, Nov 20, 2013 at 0

Re: [patch] Privatize gimplify_ctx structure.

2013-11-20 Thread Andrew MacLeod
On 11/20/2013 10:51 AM, Richard Biener wrote: On Wed, Nov 20, 2013 at 4:06 PM, Trevor Saunders wrote: On Wed, Nov 20, 2013 at 03:18:07PM +0100, Richard Biener wrote: On Wed, Nov 20, 2013 at 3:16 PM, Jakub Jelinek wrote: On Wed, Nov 20, 2013 at 03:12:34PM +0100, Richard Biener wrote: The lim

Re: [patch] Privatize gimplify_ctx structure.

2013-11-20 Thread Richard Biener
On Wed, Nov 20, 2013 at 4:06 PM, Trevor Saunders wrote: > On Wed, Nov 20, 2013 at 03:18:07PM +0100, Richard Biener wrote: >> On Wed, Nov 20, 2013 at 3:16 PM, Jakub Jelinek wrote: >> > On Wed, Nov 20, 2013 at 03:12:34PM +0100, Richard Biener wrote: >> >> The limit looks reasonable, but you could h

Re: [patch] Privatize gimplify_ctx structure.

2013-11-20 Thread Richard Biener
On Wed, Nov 20, 2013 at 3:34 PM, Andrew MacLeod wrote: > On 11/20/2013 09:12 AM, Richard Biener wrote: >> >> On Wed, Nov 20, 2013 at 2:35 PM, Andrew MacLeod >> wrote: >>> >>> This has been bugging me since I moved it out of gimple.h to gimplify.h. >>> >>> The gimplify context structure was expose

Re: [patch] Privatize gimplify_ctx structure.

2013-11-20 Thread Trevor Saunders
On Wed, Nov 20, 2013 at 03:18:07PM +0100, Richard Biener wrote: > On Wed, Nov 20, 2013 at 3:16 PM, Jakub Jelinek wrote: > > On Wed, Nov 20, 2013 at 03:12:34PM +0100, Richard Biener wrote: > >> The limit looks reasonable, but you could have used a simple linked > >> list (and never free). Also bei

Re: [patch] Privatize gimplify_ctx structure.

2013-11-20 Thread Andrew MacLeod
On 11/20/2013 09:12 AM, Richard Biener wrote: On Wed, Nov 20, 2013 at 2:35 PM, Andrew MacLeod wrote: This has been bugging me since I moved it out of gimple.h to gimplify.h. The gimplify context structure was exposed in the header file to allow a few other files to push and pop contexts off th

Re: [patch] Privatize gimplify_ctx structure.

2013-11-20 Thread Richard Biener
On Wed, Nov 20, 2013 at 3:16 PM, Jakub Jelinek wrote: > On Wed, Nov 20, 2013 at 03:12:34PM +0100, Richard Biener wrote: >> The limit looks reasonable, but you could have used a simple linked >> list (and never free). Also being able to pop a random context >> looks fragile ... that is, pop_gimpl

Re: [patch] Privatize gimplify_ctx structure.

2013-11-20 Thread Jakub Jelinek
On Wed, Nov 20, 2013 at 03:12:34PM +0100, Richard Biener wrote: > The limit looks reasonable, but you could have used a simple linked > list (and never free). Also being able to pop a random context > looks fragile ... that is, pop_gimplify_context shouldn't have an argument. Can't we use stack_

Re: [patch] Privatize gimplify_ctx structure.

2013-11-20 Thread Richard Biener
On Wed, Nov 20, 2013 at 2:35 PM, Andrew MacLeod wrote: > This has been bugging me since I moved it out of gimple.h to gimplify.h. > > The gimplify context structure was exposed in the header file to allow a few > other files to push and pop contexts off the gimplification stack. > Unfortunately, t

Re: [patch] Privatize gimplify_ctx structure.

2013-11-20 Thread Andrew MacLeod
On 11/20/2013 08:44 AM, Jakub Jelinek wrote: On Wed, Nov 20, 2013 at 08:35:28AM -0500, Andrew MacLeod wrote: I also hacked up the compiler to report what the 'top' of the stack was for a compilation unit. I then ran it through a bootstrap and full testsuite run of all languages, and looked for t

Re: [patch] Privatize gimplify_ctx structure.

2013-11-20 Thread Jakub Jelinek
On Wed, Nov 20, 2013 at 08:35:28AM -0500, Andrew MacLeod wrote: > I also hacked up the compiler to report what the 'top' of the stack > was for a compilation unit. I then ran it through a bootstrap and > full testsuite run of all languages, and looked for the maximum > number of context structs in

Re: [patch] Privatize gimplify_ctx structure.

2013-11-20 Thread Andrew MacLeod
On 11/20/2013 08:35 AM, Andrew MacLeod wrote: Bootstraps on x86_64-unknown-linux-gnu with no new regressions. OK for mainline? Andrew And I've already fixed the typo in the changelog :-) I had it open in a window but hadn't saved it when I created the patch. Andrew * gimplify.