On Wed, Nov 20, 2013 at 2:35 PM, Andrew MacLeod <amacl...@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 > >