Re: [patch] Privatize gimplify_ctx structure.

[Andrew MacLeod <amacleod at redhat dot com>]

On 11/20/2013 10:51 AM, Richard Biener wrote:
> On Wed, Nov 20, 2013 at 4:06 PM, Trevor Saunders <tsaunders@mozilla.com> 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 <jakub@redhat.com> 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_gimplify_context shouldn't have an argument.
> > > > Can't we use stack_vec<gimplify_context, 30> for that?  Though that would
> > > > mean a global var constructor and destructor, so alternatively just use
> > > > a normal vec and .create(30) it somewhere during initialization?
> > > only with gimplify_context *, otherwise things will break during re-allocation.
> > hm? it seems like the only member of gimplify_ctx that can't just be
> > memcpyd is the prev pointer which presumably could go away if you have a
> > vec of all the contexts.
> Callers have a pointer to gimplify_context AFAIK.

No one except gimplify.c can have a pointer to the gimplify_context, so 
its contained to within gimplify.c.  Pretty much everything is based off 
the current context pointer (gimplify_ctxp).  There are places where the 
address of a field within that context structure is passed to another 
routine.  If that routine then eventually triggered another 
push/pop_context call, the address underneath could be changed... and 
chaos ensues.

I don't know if that does happen, but it is a possibility and I dont see 
the need to find out.  So a simple allocation scheme has the minimal 
impact on the code, and  my preference is leave it as it is, or 
otherwise do the simple linked list malloc-ing only as necessary....

Want me to change it, or leave it as is?

Andrew