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.