[patch] Privatize gimplify_ctx structure.

Richard Biener richard.guenther@gmail.com
Wed Nov 20 15:42:00 GMT 2013


On Wed, Nov 20, 2013 at 2:35 PM, Andrew MacLeod <amacleod@redhat.com> 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, the struct contains a hash-table template, which means it
> also required hash-table.h in order to even parse the header.  No file other
> than gimplify.c actually uses what is in the structure, so exposing it seems
> wrong.
>
> This patch primarily changes push_gimplify_context () and
> pop_gimplify_context () to grab a struct gimplify_ctx off a local stack pool
> rather than have each file's local function know about the structure and
> allocate it on their stack.  I didn't want to use malloc/free or there would
> be a lot of churn.  Its open to debate what the best approach is, but I
> decided to simply allocate a bunch on a static array local to gimplify.c.
> If it goes past the end of the local array, malloc the required structure.
> Its pretty simplistic, but functional and doesn't required any GTY stuff or
> other machinery.
>
> 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
> use at one time.  Normally only 1 or 2 were ever in use, but its topped out
> at 7 during some of the openmp tests.  So my current limit of 30 will likely
> never been reached.
>
> only 2 fields were ever set by anyone outside of gimplify.c, the 'into_ssa'
> and 'allow_rhs_cond_expr' fields, and they were always set
> immediately after pushing the context onto the stack... So I added them as
> parameters to the push and gave them default values.
>
> And thats it... this patch moves the hash function and structure completely
> within gimplify.c so no one needs to know anything about it, and removes the
> hash-table.h include prerequisite for parsing.
>
> Bootstraps on x86_64-unknown-linux-gnu with no new regressions. OK for
> mainline?

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.

Richard.



> Andrew
>
>



More information about the Gcc-patches mailing list