[patch] Privatize gimplify_ctx structure.

[patch] Privatize gimplify_ctx structure.

[Andrew MacLeod]

[-- Attachment #1: Type: text/plain, Size: 2035 bytes --]

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?

Andrew



[-- Attachment #2: ctx.patch --]
[-- Type: text/x-patch, Size: 24671 bytes --]



	* 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.
	(gimplify_ctxp): Make static.
	(ctx_alloc, ctx_free): Manage a pool of struct gimplify_ctx.
	(push_gimplify_context)): Add default parameters and allocate a 
	struct from the pool.
	(pop_gimplify_context): Free a struct back to the pool.
	(gimplify_scan_omp_clauses, gimplify_omp_parallel, gimplify_omp_task,
	gimplify_omp_workshare, gimplify_transaction, gimplify_body): Don't
	use a local struct gimplify_ctx.
	* gimplify-me.c (force_gimple_operand_1, gimple_regimplify_operands):
	Likewise.
	* omp-low.c (lower_omp_sections, lower_omp_single, lower_omp_master,
	lower_omp_ordered, lower_omp_critical, lower_omp_for,
	create_task_copyfn, lower_omp_taskreg, lower_omp_target,
	lower_omp_teams, execute_lower_omp): Likewise.
	* gimple-fold.c (gimplify_and_update_call_from_tree): Likewise.
	* tree-inline.c (optimize_inline_calls): Likewise.

Index: gimplify.h
===================================================================
*** gimplify.h	(revision 205035)
--- gimplify.h	(working copy)
*************** enum gimplify_status {
*** 48,86 ****
    GS_OK		= 0,	/* We did something, maybe more to do.  */
    GS_ALL_DONE	= 1	/* The expression is fully gimplified.  */
  };
- /* Gimplify hashtable helper.  */
  
! struct gimplify_hasher : typed_free_remove <elt_t>
! {
!   typedef elt_t value_type;
!   typedef elt_t compare_type;
!   static inline hashval_t hash (const value_type *);
!   static inline bool equal (const value_type *, const compare_type *);
! };
! 
! struct gimplify_ctx
! {
!   struct gimplify_ctx *prev_context;
! 
!   vec<gimple> bind_expr_stack;
!   tree temps;
!   gimple_seq conditional_cleanups;
!   tree exit_label;
!   tree return_temp;
! 
!   vec<tree> case_labels;
!   /* The formal temporary table.  Should this be persistent?  */
!   hash_table <gimplify_hasher> temp_htab;
! 
!   int conditions;
!   bool save_stack;
!   bool into_ssa;
!   bool allow_rhs_cond_expr;
!   bool in_cleanup_point_expr;
! };
! 
! extern struct gimplify_ctx *gimplify_ctxp;
! extern void push_gimplify_context (struct gimplify_ctx *);
  extern void pop_gimplify_context (gimple);
  extern gimple gimple_current_bind_expr (void);
  extern vec<gimple> gimple_bind_expr_stack (void);
--- 48,56 ----
    GS_OK		= 0,	/* We did something, maybe more to do.  */
    GS_ALL_DONE	= 1	/* The expression is fully gimplified.  */
  };
  
! extern void push_gimplify_context (bool in_ssa = false,
! 				   bool rhs_cond_ok = false);
  extern void pop_gimplify_context (gimple);
  extern gimple gimple_current_bind_expr (void);
  extern vec<gimple> gimple_bind_expr_stack (void);
Index: gimplify.c
===================================================================
*** gimplify.c	(revision 205035)
--- gimplify.c	(working copy)
*************** enum omp_region_type
*** 89,94 ****
--- 89,126 ----
    ORT_TARGET = 32
  };
  
+ /* Gimplify hashtable helper.  */
+ 
+ struct gimplify_hasher : typed_free_remove <elt_t>
+ {
+   typedef elt_t value_type;
+   typedef elt_t compare_type;
+   static inline hashval_t hash (const value_type *);
+   static inline bool equal (const value_type *, const compare_type *);
+ };
+ 
+ struct gimplify_ctx
+ {
+   struct gimplify_ctx *prev_context;
+ 
+   vec<gimple> bind_expr_stack;
+   tree temps;
+   gimple_seq conditional_cleanups;
+   tree exit_label;
+   tree return_temp;
+ 
+   vec<tree> case_labels;
+   /* The formal temporary table.  Should this be persistent?  */
+   hash_table <gimplify_hasher> temp_htab;
+ 
+   int conditions;
+   bool save_stack;
+   bool into_ssa;
+   bool allow_rhs_cond_expr;
+   bool in_cleanup_point_expr;
+   bool malloced;
+ };
+ 
  struct gimplify_omp_ctx
  {
    struct gimplify_omp_ctx *outer_context;
*************** struct gimplify_omp_ctx
*** 100,109 ****
    bool combined_loop;
  };
  
! struct gimplify_ctx *gimplify_ctxp;
  static struct gimplify_omp_ctx *gimplify_omp_ctxp;
  
- 
  /* Forward declaration.  */
  static enum gimplify_status gimplify_compound_expr (tree *, gimple_seq *, bool);
  
--- 132,140 ----
    bool combined_loop;
  };
  
! static struct gimplify_ctx *gimplify_ctxp;
  static struct gimplify_omp_ctx *gimplify_omp_ctxp;
  
  /* Forward declaration.  */
  static enum gimplify_status gimplify_compound_expr (tree *, gimple_seq *, bool);
  
*************** gimplify_seq_add_seq (gimple_seq *dst_p,
*** 134,147 ****
    gsi_insert_seq_after_without_update (&si, src, GSI_NEW_STMT);
  }
  
  /* Set up a context for the gimplifier.  */
  
  void
! push_gimplify_context (struct gimplify_ctx *c)
  {
!   memset (c, '\0', sizeof (*c));
    c->prev_context = gimplify_ctxp;
    gimplify_ctxp = c;
  }
  
  /* Tear down a context for the gimplifier.  If BODY is non-null, then
--- 165,225 ----
    gsi_insert_seq_after_without_update (&si, src, GSI_NEW_STMT);
  }
  
+ 
+ /* Manage a pool of gimplify_ctx structs to act as a stack of allocated memory
+    for push and pop of contexts.  If requirements exceed the size of the pool,
+    then draw on malloc to temporarily make additional structs.  */
+ 
+ #define POOL_LIM 30
+ 
+ static struct gimplify_ctx pool[POOL_LIM];
+ static unsigned pool_idx = 0;
+ 
+ /* Return a gimplify context struct from the pool.  */
+ 
+ static inline struct gimplify_ctx *
+ ctx_alloc (void)
+ {
+   struct gimplify_ctx * c;
+   if (pool_idx < POOL_LIM)
+     {
+       c = pool + pool_idx++;
+       memset (c, '\0', sizeof (*c));
+     }
+   else
+     {
+       c = (struct gimplify_ctx *) xmalloc (sizeof (struct gimplify_ctx));
+       memset (c, '\0', sizeof (*c));
+       c->malloced = true;
+     }
+   return c;
+ }
+ 
+ /* put gimplify context C back into the pool.  */
+ 
+ static inline void
+ ctx_free (struct gimplify_ctx *c)
+ {
+   if (c->malloced)
+     free (c);
+   else
+     {
+       gcc_checking_assert (pool_idx != 0);
+       --pool_idx;
+     }
+ }
+ 
  /* Set up a context for the gimplifier.  */
  
  void
! push_gimplify_context (bool in_ssa, bool rhs_cond_ok)
  {
!   struct gimplify_ctx *c = ctx_alloc ();
! 
    c->prev_context = gimplify_ctxp;
    gimplify_ctxp = c;
+   gimplify_ctxp->into_ssa = in_ssa;
+   gimplify_ctxp->allow_rhs_cond_expr = rhs_cond_ok;
  }
  
  /* Tear down a context for the gimplifier.  If BODY is non-null, then
*************** pop_gimplify_context (gimple body)
*** 168,173 ****
--- 246,252 ----
  
    if (c->temp_htab.is_created ())
      c->temp_htab.dispose ();
+   ctx_free (c);
  }
  
  /* Push a GIMPLE_BIND tuple onto the stack of bindings.  */
*************** gimplify_scan_omp_clauses (tree *list_p,
*** 5726,5732 ****
  			   enum omp_region_type region_type)
  {
    struct gimplify_omp_ctx *ctx, *outer_ctx;
-   struct gimplify_ctx gctx;
    tree c;
  
    ctx = new_omp_context (region_type);
--- 5805,5810 ----
*************** gimplify_scan_omp_clauses (tree *list_p,
*** 5863,5869 ****
  	      omp_add_variable (ctx, OMP_CLAUSE_REDUCTION_PLACEHOLDER (c),
  				GOVD_LOCAL | GOVD_SEEN);
  	      gimplify_omp_ctxp = ctx;
! 	      push_gimplify_context (&gctx);
  
  	      OMP_CLAUSE_REDUCTION_GIMPLE_INIT (c) = NULL;
  	      OMP_CLAUSE_REDUCTION_GIMPLE_MERGE (c) = NULL;
--- 5941,5947 ----
  	      omp_add_variable (ctx, OMP_CLAUSE_REDUCTION_PLACEHOLDER (c),
  				GOVD_LOCAL | GOVD_SEEN);
  	      gimplify_omp_ctxp = ctx;
! 	      push_gimplify_context ();
  
  	      OMP_CLAUSE_REDUCTION_GIMPLE_INIT (c) = NULL;
  	      OMP_CLAUSE_REDUCTION_GIMPLE_MERGE (c) = NULL;
*************** gimplify_scan_omp_clauses (tree *list_p,
*** 5872,5878 ****
  		  		&OMP_CLAUSE_REDUCTION_GIMPLE_INIT (c));
  	      pop_gimplify_context
  		(gimple_seq_first_stmt (OMP_CLAUSE_REDUCTION_GIMPLE_INIT (c)));
! 	      push_gimplify_context (&gctx);
  	      gimplify_and_add (OMP_CLAUSE_REDUCTION_MERGE (c),
  		  		&OMP_CLAUSE_REDUCTION_GIMPLE_MERGE (c));
  	      pop_gimplify_context
--- 5950,5956 ----
  		  		&OMP_CLAUSE_REDUCTION_GIMPLE_INIT (c));
  	      pop_gimplify_context
  		(gimple_seq_first_stmt (OMP_CLAUSE_REDUCTION_GIMPLE_INIT (c)));
! 	      push_gimplify_context ();
  	      gimplify_and_add (OMP_CLAUSE_REDUCTION_MERGE (c),
  		  		&OMP_CLAUSE_REDUCTION_GIMPLE_MERGE (c));
  	      pop_gimplify_context
*************** gimplify_scan_omp_clauses (tree *list_p,
*** 5886,5892 ****
  		   && OMP_CLAUSE_LASTPRIVATE_STMT (c))
  	    {
  	      gimplify_omp_ctxp = ctx;
! 	      push_gimplify_context (&gctx);
  	      if (TREE_CODE (OMP_CLAUSE_LASTPRIVATE_STMT (c)) != BIND_EXPR)
  		{
  		  tree bind = build3 (BIND_EXPR, void_type_node, NULL,
--- 5964,5970 ----
  		   && OMP_CLAUSE_LASTPRIVATE_STMT (c))
  	    {
  	      gimplify_omp_ctxp = ctx;
! 	      push_gimplify_context ();
  	      if (TREE_CODE (OMP_CLAUSE_LASTPRIVATE_STMT (c)) != BIND_EXPR)
  		{
  		  tree bind = build3 (BIND_EXPR, void_type_node, NULL,
*************** gimplify_omp_parallel (tree *expr_p, gim
*** 6309,6322 ****
    tree expr = *expr_p;
    gimple g;
    gimple_seq body = NULL;
-   struct gimplify_ctx gctx;
  
    gimplify_scan_omp_clauses (&OMP_PARALLEL_CLAUSES (expr), pre_p,
  			     OMP_PARALLEL_COMBINED (expr)
  			     ? ORT_COMBINED_PARALLEL
  			     : ORT_PARALLEL);
  
!   push_gimplify_context (&gctx);
  
    g = gimplify_and_return_first (OMP_PARALLEL_BODY (expr), &body);
    if (gimple_code (g) == GIMPLE_BIND)
--- 6387,6399 ----
    tree expr = *expr_p;
    gimple g;
    gimple_seq body = NULL;
  
    gimplify_scan_omp_clauses (&OMP_PARALLEL_CLAUSES (expr), pre_p,
  			     OMP_PARALLEL_COMBINED (expr)
  			     ? ORT_COMBINED_PARALLEL
  			     : ORT_PARALLEL);
  
!   push_gimplify_context ();
  
    g = gimplify_and_return_first (OMP_PARALLEL_BODY (expr), &body);
    if (gimple_code (g) == GIMPLE_BIND)
*************** gimplify_omp_task (tree *expr_p, gimple_
*** 6346,6359 ****
    tree expr = *expr_p;
    gimple g;
    gimple_seq body = NULL;
-   struct gimplify_ctx gctx;
  
    gimplify_scan_omp_clauses (&OMP_TASK_CLAUSES (expr), pre_p,
  			     find_omp_clause (OMP_TASK_CLAUSES (expr),
  					      OMP_CLAUSE_UNTIED)
  			     ? ORT_UNTIED_TASK : ORT_TASK);
  
!   push_gimplify_context (&gctx);
  
    g = gimplify_and_return_first (OMP_TASK_BODY (expr), &body);
    if (gimple_code (g) == GIMPLE_BIND)
--- 6423,6435 ----
    tree expr = *expr_p;
    gimple g;
    gimple_seq body = NULL;
  
    gimplify_scan_omp_clauses (&OMP_TASK_CLAUSES (expr), pre_p,
  			     find_omp_clause (OMP_TASK_CLAUSES (expr),
  					      OMP_CLAUSE_UNTIED)
  			     ? ORT_UNTIED_TASK : ORT_TASK);
  
!   push_gimplify_context ();
  
    g = gimplify_and_return_first (OMP_TASK_BODY (expr), &body);
    if (gimple_code (g) == GIMPLE_BIND)
*************** gimplify_omp_workshare (tree *expr_p, gi
*** 6751,6758 ****
    gimplify_scan_omp_clauses (&OMP_CLAUSES (expr), pre_p, ort);
    if (ort == ORT_TARGET || ort == ORT_TARGET_DATA)
      {
!       struct gimplify_ctx gctx;
!       push_gimplify_context (&gctx);
        gimple g = gimplify_and_return_first (OMP_BODY (expr), &body);
        if (gimple_code (g) == GIMPLE_BIND)
  	pop_gimplify_context (g);
--- 6827,6833 ----
    gimplify_scan_omp_clauses (&OMP_CLAUSES (expr), pre_p, ort);
    if (ort == ORT_TARGET || ort == ORT_TARGET_DATA)
      {
!       push_gimplify_context ();
        gimple g = gimplify_and_return_first (OMP_BODY (expr), &body);
        if (gimple_code (g) == GIMPLE_BIND)
  	pop_gimplify_context (g);
*************** gimplify_transaction (tree *expr_p, gimp
*** 6987,6993 ****
    tree expr = *expr_p, temp, tbody = TRANSACTION_EXPR_BODY (expr);
    gimple g;
    gimple_seq body = NULL;
-   struct gimplify_ctx gctx;
    int subcode = 0;
  
    /* Wrap the transaction body in a BIND_EXPR so we have a context
--- 7062,7067 ----
*************** gimplify_transaction (tree *expr_p, gimp
*** 7000,7006 ****
        TRANSACTION_EXPR_BODY (expr) = bind;
      }
  
!   push_gimplify_context (&gctx);
    temp = voidify_wrapper_expr (*expr_p, NULL);
  
    g = gimplify_and_return_first (TRANSACTION_EXPR_BODY (expr), &body);
--- 7074,7080 ----
        TRANSACTION_EXPR_BODY (expr) = bind;
      }
  
!   push_gimplify_context ();
    temp = voidify_wrapper_expr (*expr_p, NULL);
  
    g = gimplify_and_return_first (TRANSACTION_EXPR_BODY (expr), &body);
*************** gimplify_body (tree fndecl, bool do_parm
*** 8358,8364 ****
    location_t saved_location = input_location;
    gimple_seq parm_stmts, seq;
    gimple outer_bind;
-   struct gimplify_ctx gctx;
    struct cgraph_node *cgn;
  
    timevar_push (TV_TREE_GIMPLIFY);
--- 8432,8437 ----
*************** gimplify_body (tree fndecl, bool do_parm
*** 8368,8374 ****
    default_rtl_profile ();
  
    gcc_assert (gimplify_ctxp == NULL);
!   push_gimplify_context (&gctx);
  
    if (flag_openmp)
      {
--- 8441,8447 ----
    default_rtl_profile ();
  
    gcc_assert (gimplify_ctxp == NULL);
!   push_gimplify_context ();
  
    if (flag_openmp)
      {
Index: gimplify-me.c
===================================================================
*** gimplify-me.c	(revision 205035)
--- gimplify-me.c	(working copy)
*************** force_gimple_operand_1 (tree expr, gimpl
*** 45,51 ****
  			gimple_predicate gimple_test_f, tree var)
  {
    enum gimplify_status ret;
-   struct gimplify_ctx gctx;
    location_t saved_location;
  
    *stmts = NULL;
--- 45,50 ----
*************** force_gimple_operand_1 (tree expr, gimpl
*** 57,72 ****
        && (*gimple_test_f) (expr))
      return expr;
  
!   push_gimplify_context (&gctx);
!   gimplify_ctxp->into_ssa = gimple_in_ssa_p (cfun);
!   gimplify_ctxp->allow_rhs_cond_expr = true;
    saved_location = input_location;
    input_location = UNKNOWN_LOCATION;
  
    if (var)
      {
!       if (gimplify_ctxp->into_ssa
! 	  && is_gimple_reg (var))
  	var = make_ssa_name (var, NULL);
        expr = build2 (MODIFY_EXPR, TREE_TYPE (var), var, expr);
      }
--- 56,68 ----
        && (*gimple_test_f) (expr))
      return expr;
  
!   push_gimplify_context (gimple_in_ssa_p (cfun), true);
    saved_location = input_location;
    input_location = UNKNOWN_LOCATION;
  
    if (var)
      {
!       if (gimple_in_ssa_p (cfun) && is_gimple_reg (var))
  	var = make_ssa_name (var, NULL);
        expr = build2 (MODIFY_EXPR, TREE_TYPE (var), var, expr);
      }
*************** gimple_regimplify_operands (gimple stmt,
*** 160,169 ****
    tree lhs;
    gimple_seq pre = NULL;
    gimple post_stmt = NULL;
-   struct gimplify_ctx gctx;
  
!   push_gimplify_context (&gctx);
!   gimplify_ctxp->into_ssa = gimple_in_ssa_p (cfun);
  
    switch (gimple_code (stmt))
      {
--- 156,163 ----
    tree lhs;
    gimple_seq pre = NULL;
    gimple post_stmt = NULL;
  
!   push_gimplify_context (gimple_in_ssa_p (cfun));
  
    switch (gimple_code (stmt))
      {
Index: omp-low.c
===================================================================
*** omp-low.c	(revision 205035)
--- omp-low.c	(working copy)
*************** lower_omp_sections (gimple_stmt_iterator
*** 8351,8361 ****
    gimple_stmt_iterator tgsi;
    gimple stmt, new_stmt, bind, t;
    gimple_seq ilist, dlist, olist, new_body;
-   struct gimplify_ctx gctx;
  
    stmt = gsi_stmt (*gsi_p);
  
!   push_gimplify_context (&gctx);
  
    dlist = NULL;
    ilist = NULL;
--- 8351,8360 ----
    gimple_stmt_iterator tgsi;
    gimple stmt, new_stmt, bind, t;
    gimple_seq ilist, dlist, olist, new_body;
  
    stmt = gsi_stmt (*gsi_p);
  
!   push_gimplify_context ();
  
    dlist = NULL;
    ilist = NULL;
*************** lower_omp_single (gimple_stmt_iterator *
*** 8561,8569 ****
    tree block;
    gimple t, bind, single_stmt = gsi_stmt (*gsi_p);
    gimple_seq bind_body, bind_body_tail = NULL, dlist;
-   struct gimplify_ctx gctx;
  
!   push_gimplify_context (&gctx);
  
    block = make_node (BLOCK);
    bind = gimple_build_bind (NULL, NULL, block);
--- 8560,8567 ----
    tree block;
    gimple t, bind, single_stmt = gsi_stmt (*gsi_p);
    gimple_seq bind_body, bind_body_tail = NULL, dlist;
  
!   push_gimplify_context ();
  
    block = make_node (BLOCK);
    bind = gimple_build_bind (NULL, NULL, block);
*************** lower_omp_master (gimple_stmt_iterator *
*** 8621,8629 ****
    gimple stmt = gsi_stmt (*gsi_p), bind;
    location_t loc = gimple_location (stmt);
    gimple_seq tseq;
-   struct gimplify_ctx gctx;
  
!   push_gimplify_context (&gctx);
  
    block = make_node (BLOCK);
    bind = gimple_build_bind (NULL, NULL, block);
--- 8619,8626 ----
    gimple stmt = gsi_stmt (*gsi_p), bind;
    location_t loc = gimple_location (stmt);
    gimple_seq tseq;
  
!   push_gimplify_context ();
  
    block = make_node (BLOCK);
    bind = gimple_build_bind (NULL, NULL, block);
*************** lower_omp_ordered (gimple_stmt_iterator
*** 8688,8696 ****
  {
    tree block;
    gimple stmt = gsi_stmt (*gsi_p), bind, x;
-   struct gimplify_ctx gctx;
  
!   push_gimplify_context (&gctx);
  
    block = make_node (BLOCK);
    bind = gimple_build_bind (NULL, NULL, block);
--- 8685,8692 ----
  {
    tree block;
    gimple stmt = gsi_stmt (*gsi_p), bind, x;
  
!   push_gimplify_context ();
  
    block = make_node (BLOCK);
    bind = gimple_build_bind (NULL, NULL, block);
*************** lower_omp_critical (gimple_stmt_iterator
*** 8734,8740 ****
    gimple stmt = gsi_stmt (*gsi_p), bind;
    location_t loc = gimple_location (stmt);
    gimple_seq tbody;
-   struct gimplify_ctx gctx;
  
    name = gimple_omp_critical_name (stmt);
    if (name)
--- 8730,8735 ----
*************** lower_omp_critical (gimple_stmt_iterator
*** 8787,8793 ****
        unlock = build_call_expr_loc (loc, unlock, 0);
      }
  
!   push_gimplify_context (&gctx);
  
    block = make_node (BLOCK);
    bind = gimple_build_bind (NULL, NULL, block);
--- 8782,8788 ----
        unlock = build_call_expr_loc (loc, unlock, 0);
      }
  
!   push_gimplify_context ();
  
    block = make_node (BLOCK);
    bind = gimple_build_bind (NULL, NULL, block);
*************** lower_omp_for (gimple_stmt_iterator *gsi
*** 8877,8885 ****
    gimple stmt = gsi_stmt (*gsi_p), new_stmt;
    gimple_seq omp_for_body, body, dlist;
    size_t i;
-   struct gimplify_ctx gctx;
  
!   push_gimplify_context (&gctx);
  
    lower_omp (gimple_omp_for_pre_body_ptr (stmt), ctx);
  
--- 8872,8879 ----
    gimple stmt = gsi_stmt (*gsi_p), new_stmt;
    gimple_seq omp_for_body, body, dlist;
    size_t i;
  
!   push_gimplify_context ();
  
    lower_omp (gimple_omp_for_pre_body_ptr (stmt), ctx);
  
*************** create_task_copyfn (gimple task_stmt, om
*** 9094,9100 ****
    bool record_needs_remap = false, srecord_needs_remap = false;
    splay_tree_node n;
    struct omp_taskcopy_context tcctx;
-   struct gimplify_ctx gctx;
    location_t loc = gimple_location (task_stmt);
  
    child_fn = gimple_omp_task_copy_fn (task_stmt);
--- 9088,9093 ----
*************** create_task_copyfn (gimple task_stmt, om
*** 9107,9113 ****
      DECL_CONTEXT (t) = child_fn;
  
    /* Populate the function.  */
!   push_gimplify_context (&gctx);
    push_cfun (child_cfun);
  
    bind = build3 (BIND_EXPR, void_type_node, NULL, NULL, NULL);
--- 9100,9106 ----
      DECL_CONTEXT (t) = child_fn;
  
    /* Populate the function.  */
!   push_gimplify_context ();
    push_cfun (child_cfun);
  
    bind = build3 (BIND_EXPR, void_type_node, NULL, NULL, NULL);
*************** lower_omp_taskreg (gimple_stmt_iterator
*** 9387,9393 ****
    gimple stmt = gsi_stmt (*gsi_p);
    gimple par_bind, bind, dep_bind = NULL;
    gimple_seq par_body, olist, ilist, par_olist, par_rlist, par_ilist, new_body;
-   struct gimplify_ctx gctx, dep_gctx;
    location_t loc = gimple_location (stmt);
  
    clauses = gimple_omp_taskreg_clauses (stmt);
--- 9380,9385 ----
*************** lower_omp_taskreg (gimple_stmt_iterator
*** 9412,9418 ****
    if (gimple_code (stmt) == GIMPLE_OMP_TASK
        && find_omp_clause (clauses, OMP_CLAUSE_DEPEND))
      {
!       push_gimplify_context (&dep_gctx);
        dep_bind = gimple_build_bind (NULL, NULL, make_node (BLOCK));
        lower_depend_clauses (stmt, &dep_ilist, &dep_olist);
      }
--- 9404,9410 ----
    if (gimple_code (stmt) == GIMPLE_OMP_TASK
        && find_omp_clause (clauses, OMP_CLAUSE_DEPEND))
      {
!       push_gimplify_context ();
        dep_bind = gimple_build_bind (NULL, NULL, make_node (BLOCK));
        lower_depend_clauses (stmt, &dep_ilist, &dep_olist);
      }
*************** lower_omp_taskreg (gimple_stmt_iterator
*** 9420,9426 ****
    if (ctx->srecord_type)
      create_task_copyfn (stmt, ctx);
  
!   push_gimplify_context (&gctx);
  
    par_olist = NULL;
    par_ilist = NULL;
--- 9412,9418 ----
    if (ctx->srecord_type)
      create_task_copyfn (stmt, ctx);
  
!   push_gimplify_context ();
  
    par_olist = NULL;
    par_ilist = NULL;
*************** lower_omp_target (gimple_stmt_iterator *
*** 9510,9516 ****
    gimple stmt = gsi_stmt (*gsi_p);
    gimple tgt_bind = NULL, bind;
    gimple_seq tgt_body = NULL, olist, ilist, new_body;
-   struct gimplify_ctx gctx;
    location_t loc = gimple_location (stmt);
    int kind = gimple_omp_target_kind (stmt);
    unsigned int map_cnt = 0;
--- 9502,9507 ----
*************** lower_omp_target (gimple_stmt_iterator *
*** 9525,9531 ****
      tgt_body = gimple_omp_body (stmt);
    child_fn = ctx->cb.dst_fn;
  
!   push_gimplify_context (&gctx);
  
    for (c = clauses; c ; c = OMP_CLAUSE_CHAIN (c))
      switch (OMP_CLAUSE_CODE (c))
--- 9516,9522 ----
      tgt_body = gimple_omp_body (stmt);
    child_fn = ctx->cb.dst_fn;
  
!   push_gimplify_context ();
  
    for (c = clauses; c ; c = OMP_CLAUSE_CHAIN (c))
      switch (OMP_CLAUSE_CODE (c))
*************** static void
*** 9811,9818 ****
  lower_omp_teams (gimple_stmt_iterator *gsi_p, omp_context *ctx)
  {
    gimple teams_stmt = gsi_stmt (*gsi_p);
!   struct gimplify_ctx gctx;
!   push_gimplify_context (&gctx);
  
    tree block = make_node (BLOCK);
    gimple bind = gimple_build_bind (NULL, NULL, block);
--- 9802,9808 ----
  lower_omp_teams (gimple_stmt_iterator *gsi_p, omp_context *ctx)
  {
    gimple teams_stmt = gsi_stmt (*gsi_p);
!   push_gimplify_context ();
  
    tree block = make_node (BLOCK);
    gimple bind = gimple_build_bind (NULL, NULL, block);
*************** execute_lower_omp (void)
*** 10105,10114 ****
  
    if (all_contexts->root)
      {
-       struct gimplify_ctx gctx;
- 
        if (task_shared_vars)
! 	push_gimplify_context (&gctx);
        lower_omp (&body, NULL);
        if (task_shared_vars)
  	pop_gimplify_context (NULL);
--- 10095,10102 ----
  
    if (all_contexts->root)
      {
        if (task_shared_vars)
! 	push_gimplify_context ();
        lower_omp (&body, NULL);
        if (task_shared_vars)
  	pop_gimplify_context (NULL);
Index: gimple-fold.c
===================================================================
*** gimple-fold.c	(revision 205035)
--- gimple-fold.c	(working copy)
*************** gimplify_and_update_call_from_tree (gimp
*** 608,614 ****
    gimple stmt, new_stmt;
    gimple_stmt_iterator i;
    gimple_seq stmts = NULL;
-   struct gimplify_ctx gctx;
    gimple laststore;
    tree reaching_vuse;
  
--- 608,613 ----
*************** gimplify_and_update_call_from_tree (gimp
*** 616,623 ****
  
    gcc_assert (is_gimple_call (stmt));
  
!   push_gimplify_context (&gctx);
!   gctx.into_ssa = gimple_in_ssa_p (cfun);
  
    lhs = gimple_call_lhs (stmt);
    if (lhs == NULL_TREE)
--- 615,621 ----
  
    gcc_assert (is_gimple_call (stmt));
  
!   push_gimplify_context (gimple_in_ssa_p (cfun));
  
    lhs = gimple_call_lhs (stmt);
    if (lhs == NULL_TREE)
Index: tree-inline.c
===================================================================
*** tree-inline.c	(revision 205035)
--- tree-inline.c	(working copy)
*************** optimize_inline_calls (tree fn)
*** 4518,4524 ****
    copy_body_data id;
    basic_block bb;
    int last = n_basic_blocks_for_fn (cfun);
-   struct gimplify_ctx gctx;
    bool inlined_p = false;
  
    /* Clear out ID.  */
--- 4518,4523 ----
*************** optimize_inline_calls (tree fn)
*** 4539,4545 ****
    id.transform_lang_insert_block = NULL;
    id.statements_to_fold = pointer_set_create ();
  
!   push_gimplify_context (&gctx);
  
    /* We make no attempts to keep dominance info up-to-date.  */
    free_dominance_info (CDI_DOMINATORS);
--- 4538,4544 ----
    id.transform_lang_insert_block = NULL;
    id.statements_to_fold = pointer_set_create ();
  
!   push_gimplify_context ();
  
    /* We make no attempts to keep dominance info up-to-date.  */
    free_dominance_info (CDI_DOMINATORS);

Re: [patch] Privatize gimplify_ctx structure.

[Andrew MacLeod]

[-- Attachment #1: Type: text/plain, Size: 286 bytes --]

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

[-- Attachment #2: ctx.change --]
[-- Type: text/plain, Size: 1071 bytes --]


	* 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.
	(gimplify_ctxp): Make static.
	(ctx_alloc, ctx_free): Manage a pool of struct gimplify_ctx.
	(push_gimplify_context): Add default parameters and allocate a 
	struct from the pool.
	(pop_gimplify_context): Free a struct back to the pool.
	(gimplify_scan_omp_clauses, gimplify_omp_parallel, gimplify_omp_task,
	gimplify_omp_workshare, gimplify_transaction, gimplify_body): Don't
	use a local 'struct gimplify_ctx'.
	* gimplify-me.c (force_gimple_operand_1, gimple_regimplify_operands):
	Likewise.
	* omp-low.c (lower_omp_sections, lower_omp_single, lower_omp_master,
	lower_omp_ordered, lower_omp_critical, lower_omp_for,
	create_task_copyfn, lower_omp_taskreg, lower_omp_target,
	lower_omp_teams, execute_lower_omp): Likewise.
	* gimple-fold.c (gimplify_and_update_call_from_tree): Likewise.
	* tree-inline.c (optimize_inline_calls): Likewise.

Re: [patch] Privatize gimplify_ctx structure.

[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 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.

???  You can trivially create a testcase where it will be reached
(and you certainly should, so that you actually test the fallback).

Say:

/* { dg-do compile } */
/* { dg-options "-fopenmp" } */
#define P1 _Pragma ("omp parallel")
#define P10 P1 P1 P1 P1 P1 P1 P1 P1 P1 P1
#define P100 P10 P10 P10 P10 P10 P10 P10 P10 P10 P10
void bar (void);
void
foo (void)
{
  P100
    bar ();
}

	Jakub

Re: [patch] Privatize gimplify_ctx structure.

[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 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.
> ???  You can trivially create a testcase where it will be reached
> (and you certainly should, so that you actually test the fallback).

Not something we're likely to see every day, but excellent!  a testcase 
is good :-) I'll add this as a testcase.

Do you think the current limit of 30 is reasonable?

Andrew

Re: [patch] Privatize gimplify_ctx structure.

[Richard Biener]

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
>
>

Re: [patch] Privatize gimplify_ctx structure.

[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_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?

	Jakub

Re: [patch] Privatize gimplify_ctx structure.

[Richard Biener]

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.

Richard.

>         Jakub

Re: [patch] Privatize gimplify_ctx structure.

[Andrew MacLeod]

On 11/20/2013 09:12 AM, Richard Biener wrote:
> 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.
>
>
The argument to pop_gimplify_context isn't a context pointer, its a 
gimple statement and if present calls declare_vars on the temps in the 
body...  the routine always pops the current context.  So that doesn't 
seem too fragile?

I had considered just a linked list and never freeing, but thought that 
policy might be frowned upon and trigger false positives on memory 
allocation analysis tools  :-)   I'm happy to do that tho, its quite 
trivial.


Andrew

Re: [patch] Privatize gimplify_ctx structure.

[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 <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.

Trev

> 
> Richard.
> 
> >         Jakub

Re: [patch] Privatize gimplify_ctx structure.

[Richard Biener]

On Wed, Nov 20, 2013 at 3:34 PM, Andrew MacLeod <amacleod@redhat.com> wrote:
> On 11/20/2013 09:12 AM, Richard Biener wrote:
>>
>> 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.
>>
>>
> The argument to pop_gimplify_context isn't a context pointer, its a gimple
> statement and if present calls declare_vars on the temps in the body...  the
> routine always pops the current context.  So that doesn't seem too fragile?

Oh... I see.

> I had considered just a linked list and never freeing, but thought that
> policy might be frowned upon and trigger false positives on memory
> allocation analysis tools  :-)   I'm happy to do that tho, its quite
> trivial.

;)

Richard.

>
> Andrew
>
>

Re: [patch] Privatize gimplify_ctx structure.

[Richard Biener]

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.

Richard.

> Trev
>
>>
>> Richard.
>>
>> >         Jakub

Re: [patch] Privatize gimplify_ctx structure.

[Andrew MacLeod]

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

Re: [patch] Privatize gimplify_ctx structure.

[Andrew MacLeod]

[-- Attachment #1: Type: text/plain, Size: 2269 bytes --]

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 
>> <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?
In fact, I like the code better for the linked list... its even simpler  
I've attached that patch.   Its currently bootstrapping and I'll run the 
tests.   And it wouldn't need a special testcase since there are no edge 
cases.

Which is your preference?

Andrew

[-- Attachment #2: ctx2.patch --]
[-- Type: text/x-patch, Size: 24336 bytes --]



	* 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.
	(gimplify_ctxp): Make static.
	(ctx_pool, ctx_alloc, ctx_free): Manage a list of struct gimplify_ctx.
	(push_gimplify_context): Add default parameters and allocate a 
	struct from the pool.
	(pop_gimplify_context): Free a struct back to the pool.
	(gimplify_scan_omp_clauses, gimplify_omp_parallel, gimplify_omp_task,
	gimplify_omp_workshare, gimplify_transaction, gimplify_body): Don't
	use a local 'struct gimplify_ctx'.
	* gimplify-me.c (force_gimple_operand_1, gimple_regimplify_operands):
	Likewise.
	* omp-low.c (lower_omp_sections, lower_omp_single, lower_omp_master,
	lower_omp_ordered, lower_omp_critical, lower_omp_for,
	create_task_copyfn, lower_omp_taskreg, lower_omp_target,
	lower_omp_teams, execute_lower_omp): Likewise.
	* gimple-fold.c (gimplify_and_update_call_from_tree): Likewise.
	* tree-inline.c (optimize_inline_calls): Likewise.

Index: gimplify.h
===================================================================
*** gimplify.h	(revision 205035)
--- gimplify.h	(working copy)
*************** enum gimplify_status {
*** 48,86 ****
    GS_OK		= 0,	/* We did something, maybe more to do.  */
    GS_ALL_DONE	= 1	/* The expression is fully gimplified.  */
  };
- /* Gimplify hashtable helper.  */
  
! struct gimplify_hasher : typed_free_remove <elt_t>
! {
!   typedef elt_t value_type;
!   typedef elt_t compare_type;
!   static inline hashval_t hash (const value_type *);
!   static inline bool equal (const value_type *, const compare_type *);
! };
! 
! struct gimplify_ctx
! {
!   struct gimplify_ctx *prev_context;
! 
!   vec<gimple> bind_expr_stack;
!   tree temps;
!   gimple_seq conditional_cleanups;
!   tree exit_label;
!   tree return_temp;
! 
!   vec<tree> case_labels;
!   /* The formal temporary table.  Should this be persistent?  */
!   hash_table <gimplify_hasher> temp_htab;
! 
!   int conditions;
!   bool save_stack;
!   bool into_ssa;
!   bool allow_rhs_cond_expr;
!   bool in_cleanup_point_expr;
! };
! 
! extern struct gimplify_ctx *gimplify_ctxp;
! extern void push_gimplify_context (struct gimplify_ctx *);
  extern void pop_gimplify_context (gimple);
  extern gimple gimple_current_bind_expr (void);
  extern vec<gimple> gimple_bind_expr_stack (void);
--- 48,56 ----
    GS_OK		= 0,	/* We did something, maybe more to do.  */
    GS_ALL_DONE	= 1	/* The expression is fully gimplified.  */
  };
  
! extern void push_gimplify_context (bool in_ssa = false,
! 				   bool rhs_cond_ok = false);
  extern void pop_gimplify_context (gimple);
  extern gimple gimple_current_bind_expr (void);
  extern vec<gimple> gimple_bind_expr_stack (void);
Index: gimplify.c
===================================================================
*** gimplify.c	(revision 205035)
--- gimplify.c	(working copy)
*************** enum omp_region_type
*** 89,94 ****
--- 89,125 ----
    ORT_TARGET = 32
  };
  
+ /* Gimplify hashtable helper.  */
+ 
+ struct gimplify_hasher : typed_free_remove <elt_t>
+ {
+   typedef elt_t value_type;
+   typedef elt_t compare_type;
+   static inline hashval_t hash (const value_type *);
+   static inline bool equal (const value_type *, const compare_type *);
+ };
+ 
+ struct gimplify_ctx
+ {
+   struct gimplify_ctx *prev_context;
+ 
+   vec<gimple> bind_expr_stack;
+   tree temps;
+   gimple_seq conditional_cleanups;
+   tree exit_label;
+   tree return_temp;
+ 
+   vec<tree> case_labels;
+   /* The formal temporary table.  Should this be persistent?  */
+   hash_table <gimplify_hasher> temp_htab;
+ 
+   int conditions;
+   bool save_stack;
+   bool into_ssa;
+   bool allow_rhs_cond_expr;
+   bool in_cleanup_point_expr;
+ };
+ 
  struct gimplify_omp_ctx
  {
    struct gimplify_omp_ctx *outer_context;
*************** struct gimplify_omp_ctx
*** 100,109 ****
    bool combined_loop;
  };
  
! struct gimplify_ctx *gimplify_ctxp;
  static struct gimplify_omp_ctx *gimplify_omp_ctxp;
  
- 
  /* Forward declaration.  */
  static enum gimplify_status gimplify_compound_expr (tree *, gimple_seq *, bool);
  
--- 131,139 ----
    bool combined_loop;
  };
  
! static struct gimplify_ctx *gimplify_ctxp;
  static struct gimplify_omp_ctx *gimplify_omp_ctxp;
  
  /* Forward declaration.  */
  static enum gimplify_status gimplify_compound_expr (tree *, gimple_seq *, bool);
  
*************** gimplify_seq_add_seq (gimple_seq *dst_p,
*** 134,147 ****
    gsi_insert_seq_after_without_update (&si, src, GSI_NEW_STMT);
  }
  
  /* Set up a context for the gimplifier.  */
  
  void
! push_gimplify_context (struct gimplify_ctx *c)
  {
!   memset (c, '\0', sizeof (*c));
    c->prev_context = gimplify_ctxp;
    gimplify_ctxp = c;
  }
  
  /* Tear down a context for the gimplifier.  If BODY is non-null, then
--- 164,211 ----
    gsi_insert_seq_after_without_update (&si, src, GSI_NEW_STMT);
  }
  
+ 
+ /* pointer to a list of gimplify_ctx structs to be used as a stack of allocated
+    memory for push and pop of gimplify contexts.  */
+ 
+ static struct gimplify_ctx *ctx_pool = NULL;
+ 
+ /* Return a gimplify context struct from the pool.  */
+ 
+ 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 gimplify_ctx));
+ 
+   memset (c, '\0', sizeof (*c));
+   return c;
+ }
+ 
+ /* put gimplify context C back into the pool.  */
+ 
+ static inline void
+ ctx_free (struct gimplify_ctx *c)
+ {
+   c->prev_context = ctx_pool;
+   ctx_pool = c;
+ }
+ 
  /* Set up a context for the gimplifier.  */
  
  void
! push_gimplify_context (bool in_ssa, bool rhs_cond_ok)
  {
!   struct gimplify_ctx *c = ctx_alloc ();
! 
    c->prev_context = gimplify_ctxp;
    gimplify_ctxp = c;
+   gimplify_ctxp->into_ssa = in_ssa;
+   gimplify_ctxp->allow_rhs_cond_expr = rhs_cond_ok;
  }
  
  /* Tear down a context for the gimplifier.  If BODY is non-null, then
*************** pop_gimplify_context (gimple body)
*** 168,173 ****
--- 232,238 ----
  
    if (c->temp_htab.is_created ())
      c->temp_htab.dispose ();
+   ctx_free (c);
  }
  
  /* Push a GIMPLE_BIND tuple onto the stack of bindings.  */
*************** gimplify_scan_omp_clauses (tree *list_p,
*** 5726,5732 ****
  			   enum omp_region_type region_type)
  {
    struct gimplify_omp_ctx *ctx, *outer_ctx;
-   struct gimplify_ctx gctx;
    tree c;
  
    ctx = new_omp_context (region_type);
--- 5791,5796 ----
*************** gimplify_scan_omp_clauses (tree *list_p,
*** 5863,5869 ****
  	      omp_add_variable (ctx, OMP_CLAUSE_REDUCTION_PLACEHOLDER (c),
  				GOVD_LOCAL | GOVD_SEEN);
  	      gimplify_omp_ctxp = ctx;
! 	      push_gimplify_context (&gctx);
  
  	      OMP_CLAUSE_REDUCTION_GIMPLE_INIT (c) = NULL;
  	      OMP_CLAUSE_REDUCTION_GIMPLE_MERGE (c) = NULL;
--- 5927,5933 ----
  	      omp_add_variable (ctx, OMP_CLAUSE_REDUCTION_PLACEHOLDER (c),
  				GOVD_LOCAL | GOVD_SEEN);
  	      gimplify_omp_ctxp = ctx;
! 	      push_gimplify_context ();
  
  	      OMP_CLAUSE_REDUCTION_GIMPLE_INIT (c) = NULL;
  	      OMP_CLAUSE_REDUCTION_GIMPLE_MERGE (c) = NULL;
*************** gimplify_scan_omp_clauses (tree *list_p,
*** 5872,5878 ****
  		  		&OMP_CLAUSE_REDUCTION_GIMPLE_INIT (c));
  	      pop_gimplify_context
  		(gimple_seq_first_stmt (OMP_CLAUSE_REDUCTION_GIMPLE_INIT (c)));
! 	      push_gimplify_context (&gctx);
  	      gimplify_and_add (OMP_CLAUSE_REDUCTION_MERGE (c),
  		  		&OMP_CLAUSE_REDUCTION_GIMPLE_MERGE (c));
  	      pop_gimplify_context
--- 5936,5942 ----
  		  		&OMP_CLAUSE_REDUCTION_GIMPLE_INIT (c));
  	      pop_gimplify_context
  		(gimple_seq_first_stmt (OMP_CLAUSE_REDUCTION_GIMPLE_INIT (c)));
! 	      push_gimplify_context ();
  	      gimplify_and_add (OMP_CLAUSE_REDUCTION_MERGE (c),
  		  		&OMP_CLAUSE_REDUCTION_GIMPLE_MERGE (c));
  	      pop_gimplify_context
*************** gimplify_scan_omp_clauses (tree *list_p,
*** 5886,5892 ****
  		   && OMP_CLAUSE_LASTPRIVATE_STMT (c))
  	    {
  	      gimplify_omp_ctxp = ctx;
! 	      push_gimplify_context (&gctx);
  	      if (TREE_CODE (OMP_CLAUSE_LASTPRIVATE_STMT (c)) != BIND_EXPR)
  		{
  		  tree bind = build3 (BIND_EXPR, void_type_node, NULL,
--- 5950,5956 ----
  		   && OMP_CLAUSE_LASTPRIVATE_STMT (c))
  	    {
  	      gimplify_omp_ctxp = ctx;
! 	      push_gimplify_context ();
  	      if (TREE_CODE (OMP_CLAUSE_LASTPRIVATE_STMT (c)) != BIND_EXPR)
  		{
  		  tree bind = build3 (BIND_EXPR, void_type_node, NULL,
*************** gimplify_omp_parallel (tree *expr_p, gim
*** 6309,6322 ****
    tree expr = *expr_p;
    gimple g;
    gimple_seq body = NULL;
-   struct gimplify_ctx gctx;
  
    gimplify_scan_omp_clauses (&OMP_PARALLEL_CLAUSES (expr), pre_p,
  			     OMP_PARALLEL_COMBINED (expr)
  			     ? ORT_COMBINED_PARALLEL
  			     : ORT_PARALLEL);
  
!   push_gimplify_context (&gctx);
  
    g = gimplify_and_return_first (OMP_PARALLEL_BODY (expr), &body);
    if (gimple_code (g) == GIMPLE_BIND)
--- 6373,6385 ----
    tree expr = *expr_p;
    gimple g;
    gimple_seq body = NULL;
  
    gimplify_scan_omp_clauses (&OMP_PARALLEL_CLAUSES (expr), pre_p,
  			     OMP_PARALLEL_COMBINED (expr)
  			     ? ORT_COMBINED_PARALLEL
  			     : ORT_PARALLEL);
  
!   push_gimplify_context ();
  
    g = gimplify_and_return_first (OMP_PARALLEL_BODY (expr), &body);
    if (gimple_code (g) == GIMPLE_BIND)
*************** gimplify_omp_task (tree *expr_p, gimple_
*** 6346,6359 ****
    tree expr = *expr_p;
    gimple g;
    gimple_seq body = NULL;
-   struct gimplify_ctx gctx;
  
    gimplify_scan_omp_clauses (&OMP_TASK_CLAUSES (expr), pre_p,
  			     find_omp_clause (OMP_TASK_CLAUSES (expr),
  					      OMP_CLAUSE_UNTIED)
  			     ? ORT_UNTIED_TASK : ORT_TASK);
  
!   push_gimplify_context (&gctx);
  
    g = gimplify_and_return_first (OMP_TASK_BODY (expr), &body);
    if (gimple_code (g) == GIMPLE_BIND)
--- 6409,6421 ----
    tree expr = *expr_p;
    gimple g;
    gimple_seq body = NULL;
  
    gimplify_scan_omp_clauses (&OMP_TASK_CLAUSES (expr), pre_p,
  			     find_omp_clause (OMP_TASK_CLAUSES (expr),
  					      OMP_CLAUSE_UNTIED)
  			     ? ORT_UNTIED_TASK : ORT_TASK);
  
!   push_gimplify_context ();
  
    g = gimplify_and_return_first (OMP_TASK_BODY (expr), &body);
    if (gimple_code (g) == GIMPLE_BIND)
*************** gimplify_omp_workshare (tree *expr_p, gi
*** 6751,6758 ****
    gimplify_scan_omp_clauses (&OMP_CLAUSES (expr), pre_p, ort);
    if (ort == ORT_TARGET || ort == ORT_TARGET_DATA)
      {
!       struct gimplify_ctx gctx;
!       push_gimplify_context (&gctx);
        gimple g = gimplify_and_return_first (OMP_BODY (expr), &body);
        if (gimple_code (g) == GIMPLE_BIND)
  	pop_gimplify_context (g);
--- 6813,6819 ----
    gimplify_scan_omp_clauses (&OMP_CLAUSES (expr), pre_p, ort);
    if (ort == ORT_TARGET || ort == ORT_TARGET_DATA)
      {
!       push_gimplify_context ();
        gimple g = gimplify_and_return_first (OMP_BODY (expr), &body);
        if (gimple_code (g) == GIMPLE_BIND)
  	pop_gimplify_context (g);
*************** gimplify_transaction (tree *expr_p, gimp
*** 6987,6993 ****
    tree expr = *expr_p, temp, tbody = TRANSACTION_EXPR_BODY (expr);
    gimple g;
    gimple_seq body = NULL;
-   struct gimplify_ctx gctx;
    int subcode = 0;
  
    /* Wrap the transaction body in a BIND_EXPR so we have a context
--- 7048,7053 ----
*************** gimplify_transaction (tree *expr_p, gimp
*** 7000,7006 ****
        TRANSACTION_EXPR_BODY (expr) = bind;
      }
  
!   push_gimplify_context (&gctx);
    temp = voidify_wrapper_expr (*expr_p, NULL);
  
    g = gimplify_and_return_first (TRANSACTION_EXPR_BODY (expr), &body);
--- 7060,7066 ----
        TRANSACTION_EXPR_BODY (expr) = bind;
      }
  
!   push_gimplify_context ();
    temp = voidify_wrapper_expr (*expr_p, NULL);
  
    g = gimplify_and_return_first (TRANSACTION_EXPR_BODY (expr), &body);
*************** gimplify_body (tree fndecl, bool do_parm
*** 8358,8364 ****
    location_t saved_location = input_location;
    gimple_seq parm_stmts, seq;
    gimple outer_bind;
-   struct gimplify_ctx gctx;
    struct cgraph_node *cgn;
  
    timevar_push (TV_TREE_GIMPLIFY);
--- 8418,8423 ----
*************** gimplify_body (tree fndecl, bool do_parm
*** 8368,8374 ****
    default_rtl_profile ();
  
    gcc_assert (gimplify_ctxp == NULL);
!   push_gimplify_context (&gctx);
  
    if (flag_openmp)
      {
--- 8427,8433 ----
    default_rtl_profile ();
  
    gcc_assert (gimplify_ctxp == NULL);
!   push_gimplify_context ();
  
    if (flag_openmp)
      {
Index: gimplify-me.c
===================================================================
*** gimplify-me.c	(revision 205035)
--- gimplify-me.c	(working copy)
*************** force_gimple_operand_1 (tree expr, gimpl
*** 45,51 ****
  			gimple_predicate gimple_test_f, tree var)
  {
    enum gimplify_status ret;
-   struct gimplify_ctx gctx;
    location_t saved_location;
  
    *stmts = NULL;
--- 45,50 ----
*************** force_gimple_operand_1 (tree expr, gimpl
*** 57,72 ****
        && (*gimple_test_f) (expr))
      return expr;
  
!   push_gimplify_context (&gctx);
!   gimplify_ctxp->into_ssa = gimple_in_ssa_p (cfun);
!   gimplify_ctxp->allow_rhs_cond_expr = true;
    saved_location = input_location;
    input_location = UNKNOWN_LOCATION;
  
    if (var)
      {
!       if (gimplify_ctxp->into_ssa
! 	  && is_gimple_reg (var))
  	var = make_ssa_name (var, NULL);
        expr = build2 (MODIFY_EXPR, TREE_TYPE (var), var, expr);
      }
--- 56,68 ----
        && (*gimple_test_f) (expr))
      return expr;
  
!   push_gimplify_context (gimple_in_ssa_p (cfun), true);
    saved_location = input_location;
    input_location = UNKNOWN_LOCATION;
  
    if (var)
      {
!       if (gimple_in_ssa_p (cfun) && is_gimple_reg (var))
  	var = make_ssa_name (var, NULL);
        expr = build2 (MODIFY_EXPR, TREE_TYPE (var), var, expr);
      }
*************** gimple_regimplify_operands (gimple stmt,
*** 160,169 ****
    tree lhs;
    gimple_seq pre = NULL;
    gimple post_stmt = NULL;
-   struct gimplify_ctx gctx;
  
!   push_gimplify_context (&gctx);
!   gimplify_ctxp->into_ssa = gimple_in_ssa_p (cfun);
  
    switch (gimple_code (stmt))
      {
--- 156,163 ----
    tree lhs;
    gimple_seq pre = NULL;
    gimple post_stmt = NULL;
  
!   push_gimplify_context (gimple_in_ssa_p (cfun));
  
    switch (gimple_code (stmt))
      {
Index: omp-low.c
===================================================================
*** omp-low.c	(revision 205035)
--- omp-low.c	(working copy)
*************** lower_omp_sections (gimple_stmt_iterator
*** 8351,8361 ****
    gimple_stmt_iterator tgsi;
    gimple stmt, new_stmt, bind, t;
    gimple_seq ilist, dlist, olist, new_body;
-   struct gimplify_ctx gctx;
  
    stmt = gsi_stmt (*gsi_p);
  
!   push_gimplify_context (&gctx);
  
    dlist = NULL;
    ilist = NULL;
--- 8351,8360 ----
    gimple_stmt_iterator tgsi;
    gimple stmt, new_stmt, bind, t;
    gimple_seq ilist, dlist, olist, new_body;
  
    stmt = gsi_stmt (*gsi_p);
  
!   push_gimplify_context ();
  
    dlist = NULL;
    ilist = NULL;
*************** lower_omp_single (gimple_stmt_iterator *
*** 8561,8569 ****
    tree block;
    gimple t, bind, single_stmt = gsi_stmt (*gsi_p);
    gimple_seq bind_body, bind_body_tail = NULL, dlist;
-   struct gimplify_ctx gctx;
  
!   push_gimplify_context (&gctx);
  
    block = make_node (BLOCK);
    bind = gimple_build_bind (NULL, NULL, block);
--- 8560,8567 ----
    tree block;
    gimple t, bind, single_stmt = gsi_stmt (*gsi_p);
    gimple_seq bind_body, bind_body_tail = NULL, dlist;
  
!   push_gimplify_context ();
  
    block = make_node (BLOCK);
    bind = gimple_build_bind (NULL, NULL, block);
*************** lower_omp_master (gimple_stmt_iterator *
*** 8621,8629 ****
    gimple stmt = gsi_stmt (*gsi_p), bind;
    location_t loc = gimple_location (stmt);
    gimple_seq tseq;
-   struct gimplify_ctx gctx;
  
!   push_gimplify_context (&gctx);
  
    block = make_node (BLOCK);
    bind = gimple_build_bind (NULL, NULL, block);
--- 8619,8626 ----
    gimple stmt = gsi_stmt (*gsi_p), bind;
    location_t loc = gimple_location (stmt);
    gimple_seq tseq;
  
!   push_gimplify_context ();
  
    block = make_node (BLOCK);
    bind = gimple_build_bind (NULL, NULL, block);
*************** lower_omp_ordered (gimple_stmt_iterator
*** 8688,8696 ****
  {
    tree block;
    gimple stmt = gsi_stmt (*gsi_p), bind, x;
-   struct gimplify_ctx gctx;
  
!   push_gimplify_context (&gctx);
  
    block = make_node (BLOCK);
    bind = gimple_build_bind (NULL, NULL, block);
--- 8685,8692 ----
  {
    tree block;
    gimple stmt = gsi_stmt (*gsi_p), bind, x;
  
!   push_gimplify_context ();
  
    block = make_node (BLOCK);
    bind = gimple_build_bind (NULL, NULL, block);
*************** lower_omp_critical (gimple_stmt_iterator
*** 8734,8740 ****
    gimple stmt = gsi_stmt (*gsi_p), bind;
    location_t loc = gimple_location (stmt);
    gimple_seq tbody;
-   struct gimplify_ctx gctx;
  
    name = gimple_omp_critical_name (stmt);
    if (name)
--- 8730,8735 ----
*************** lower_omp_critical (gimple_stmt_iterator
*** 8787,8793 ****
        unlock = build_call_expr_loc (loc, unlock, 0);
      }
  
!   push_gimplify_context (&gctx);
  
    block = make_node (BLOCK);
    bind = gimple_build_bind (NULL, NULL, block);
--- 8782,8788 ----
        unlock = build_call_expr_loc (loc, unlock, 0);
      }
  
!   push_gimplify_context ();
  
    block = make_node (BLOCK);
    bind = gimple_build_bind (NULL, NULL, block);
*************** lower_omp_for (gimple_stmt_iterator *gsi
*** 8877,8885 ****
    gimple stmt = gsi_stmt (*gsi_p), new_stmt;
    gimple_seq omp_for_body, body, dlist;
    size_t i;
-   struct gimplify_ctx gctx;
  
!   push_gimplify_context (&gctx);
  
    lower_omp (gimple_omp_for_pre_body_ptr (stmt), ctx);
  
--- 8872,8879 ----
    gimple stmt = gsi_stmt (*gsi_p), new_stmt;
    gimple_seq omp_for_body, body, dlist;
    size_t i;
  
!   push_gimplify_context ();
  
    lower_omp (gimple_omp_for_pre_body_ptr (stmt), ctx);
  
*************** create_task_copyfn (gimple task_stmt, om
*** 9094,9100 ****
    bool record_needs_remap = false, srecord_needs_remap = false;
    splay_tree_node n;
    struct omp_taskcopy_context tcctx;
-   struct gimplify_ctx gctx;
    location_t loc = gimple_location (task_stmt);
  
    child_fn = gimple_omp_task_copy_fn (task_stmt);
--- 9088,9093 ----
*************** create_task_copyfn (gimple task_stmt, om
*** 9107,9113 ****
      DECL_CONTEXT (t) = child_fn;
  
    /* Populate the function.  */
!   push_gimplify_context (&gctx);
    push_cfun (child_cfun);
  
    bind = build3 (BIND_EXPR, void_type_node, NULL, NULL, NULL);
--- 9100,9106 ----
      DECL_CONTEXT (t) = child_fn;
  
    /* Populate the function.  */
!   push_gimplify_context ();
    push_cfun (child_cfun);
  
    bind = build3 (BIND_EXPR, void_type_node, NULL, NULL, NULL);
*************** lower_omp_taskreg (gimple_stmt_iterator
*** 9387,9393 ****
    gimple stmt = gsi_stmt (*gsi_p);
    gimple par_bind, bind, dep_bind = NULL;
    gimple_seq par_body, olist, ilist, par_olist, par_rlist, par_ilist, new_body;
-   struct gimplify_ctx gctx, dep_gctx;
    location_t loc = gimple_location (stmt);
  
    clauses = gimple_omp_taskreg_clauses (stmt);
--- 9380,9385 ----
*************** lower_omp_taskreg (gimple_stmt_iterator
*** 9412,9418 ****
    if (gimple_code (stmt) == GIMPLE_OMP_TASK
        && find_omp_clause (clauses, OMP_CLAUSE_DEPEND))
      {
!       push_gimplify_context (&dep_gctx);
        dep_bind = gimple_build_bind (NULL, NULL, make_node (BLOCK));
        lower_depend_clauses (stmt, &dep_ilist, &dep_olist);
      }
--- 9404,9410 ----
    if (gimple_code (stmt) == GIMPLE_OMP_TASK
        && find_omp_clause (clauses, OMP_CLAUSE_DEPEND))
      {
!       push_gimplify_context ();
        dep_bind = gimple_build_bind (NULL, NULL, make_node (BLOCK));
        lower_depend_clauses (stmt, &dep_ilist, &dep_olist);
      }
*************** lower_omp_taskreg (gimple_stmt_iterator
*** 9420,9426 ****
    if (ctx->srecord_type)
      create_task_copyfn (stmt, ctx);
  
!   push_gimplify_context (&gctx);
  
    par_olist = NULL;
    par_ilist = NULL;
--- 9412,9418 ----
    if (ctx->srecord_type)
      create_task_copyfn (stmt, ctx);
  
!   push_gimplify_context ();
  
    par_olist = NULL;
    par_ilist = NULL;
*************** lower_omp_target (gimple_stmt_iterator *
*** 9510,9516 ****
    gimple stmt = gsi_stmt (*gsi_p);
    gimple tgt_bind = NULL, bind;
    gimple_seq tgt_body = NULL, olist, ilist, new_body;
-   struct gimplify_ctx gctx;
    location_t loc = gimple_location (stmt);
    int kind = gimple_omp_target_kind (stmt);
    unsigned int map_cnt = 0;
--- 9502,9507 ----
*************** lower_omp_target (gimple_stmt_iterator *
*** 9525,9531 ****
      tgt_body = gimple_omp_body (stmt);
    child_fn = ctx->cb.dst_fn;
  
!   push_gimplify_context (&gctx);
  
    for (c = clauses; c ; c = OMP_CLAUSE_CHAIN (c))
      switch (OMP_CLAUSE_CODE (c))
--- 9516,9522 ----
      tgt_body = gimple_omp_body (stmt);
    child_fn = ctx->cb.dst_fn;
  
!   push_gimplify_context ();
  
    for (c = clauses; c ; c = OMP_CLAUSE_CHAIN (c))
      switch (OMP_CLAUSE_CODE (c))
*************** static void
*** 9811,9818 ****
  lower_omp_teams (gimple_stmt_iterator *gsi_p, omp_context *ctx)
  {
    gimple teams_stmt = gsi_stmt (*gsi_p);
!   struct gimplify_ctx gctx;
!   push_gimplify_context (&gctx);
  
    tree block = make_node (BLOCK);
    gimple bind = gimple_build_bind (NULL, NULL, block);
--- 9802,9808 ----
  lower_omp_teams (gimple_stmt_iterator *gsi_p, omp_context *ctx)
  {
    gimple teams_stmt = gsi_stmt (*gsi_p);
!   push_gimplify_context ();
  
    tree block = make_node (BLOCK);
    gimple bind = gimple_build_bind (NULL, NULL, block);
*************** execute_lower_omp (void)
*** 10105,10114 ****
  
    if (all_contexts->root)
      {
-       struct gimplify_ctx gctx;
- 
        if (task_shared_vars)
! 	push_gimplify_context (&gctx);
        lower_omp (&body, NULL);
        if (task_shared_vars)
  	pop_gimplify_context (NULL);
--- 10095,10102 ----
  
    if (all_contexts->root)
      {
        if (task_shared_vars)
! 	push_gimplify_context ();
        lower_omp (&body, NULL);
        if (task_shared_vars)
  	pop_gimplify_context (NULL);
Index: gimple-fold.c
===================================================================
*** gimple-fold.c	(revision 205035)
--- gimple-fold.c	(working copy)
*************** gimplify_and_update_call_from_tree (gimp
*** 608,614 ****
    gimple stmt, new_stmt;
    gimple_stmt_iterator i;
    gimple_seq stmts = NULL;
-   struct gimplify_ctx gctx;
    gimple laststore;
    tree reaching_vuse;
  
--- 608,613 ----
*************** gimplify_and_update_call_from_tree (gimp
*** 616,623 ****
  
    gcc_assert (is_gimple_call (stmt));
  
!   push_gimplify_context (&gctx);
!   gctx.into_ssa = gimple_in_ssa_p (cfun);
  
    lhs = gimple_call_lhs (stmt);
    if (lhs == NULL_TREE)
--- 615,621 ----
  
    gcc_assert (is_gimple_call (stmt));
  
!   push_gimplify_context (gimple_in_ssa_p (cfun));
  
    lhs = gimple_call_lhs (stmt);
    if (lhs == NULL_TREE)
Index: tree-inline.c
===================================================================
*** tree-inline.c	(revision 205035)
--- tree-inline.c	(working copy)
*************** optimize_inline_calls (tree fn)
*** 4518,4524 ****
    copy_body_data id;
    basic_block bb;
    int last = n_basic_blocks_for_fn (cfun);
-   struct gimplify_ctx gctx;
    bool inlined_p = false;
  
    /* Clear out ID.  */
--- 4518,4523 ----
*************** optimize_inline_calls (tree fn)
*** 4539,4545 ****
    id.transform_lang_insert_block = NULL;
    id.statements_to_fold = pointer_set_create ();
  
!   push_gimplify_context (&gctx);
  
    /* We make no attempts to keep dominance info up-to-date.  */
    free_dominance_info (CDI_DOMINATORS);
--- 4538,4544 ----
    id.transform_lang_insert_block = NULL;
    id.statements_to_fold = pointer_set_create ();
  
!   push_gimplify_context ();
  
    /* We make no attempts to keep dominance info up-to-date.  */
    free_dominance_info (CDI_DOMINATORS);

Re: [patch] Privatize gimplify_ctx structure.

[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 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.
>
> ???  You can trivially create a testcase where it will be reached
> (and you certainly should, so that you actually test the fallback).
I was going to suggest hacking things so that he only had 2, then do a 
bootstrap/comparison to test the fallback.  Obviously that would be for 
testing purposes only.

However, a test we can put into the suite is even better :-)
jeff

Re: [patch] Privatize gimplify_ctx structure.

[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 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.
>> ???  You can trivially create a testcase where it will be reached
>> (and you certainly should, so that you actually test the fallback).
>
> Not something we're likely to see every day, but excellent!  a testcase
> is good :-) I'll add this as a testcase.
>
> Do you think the current limit of 30 is reasonable?
Yes if you've verified the fall back works ;-)  You probably could even 
drop it to 10ish.
jeff

Re: [patch] Privatize gimplify_ctx structure.

[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.
> 	(gimplify_ctxp): Make static.
> 	(ctx_pool, ctx_alloc, ctx_free): Manage a list of struct gimplify_ctx.
> 	(push_gimplify_context): Add default parameters and allocate a
> 	struct from the pool.
> 	(pop_gimplify_context): Free a struct back to the pool.
> 	(gimplify_scan_omp_clauses, gimplify_omp_parallel, gimplify_omp_task,
> 	gimplify_omp_workshare, gimplify_transaction, gimplify_body): Don't
> 	use a local 'struct gimplify_ctx'.
> 	* gimplify-me.c (force_gimple_operand_1, gimple_regimplify_operands):
> 	Likewise.
> 	* omp-low.c (lower_omp_sections, lower_omp_single, lower_omp_master,
> 	lower_omp_ordered, lower_omp_critical, lower_omp_for,
> 	create_task_copyfn, lower_omp_taskreg, lower_omp_target,
> 	lower_omp_teams, execute_lower_omp): Likewise.
> 	* gimple-fold.c (gimplify_and_update_call_from_tree): Likewise.
> 	* tree-inline.c (optimize_inline_calls): Likewise.
I don't see the malloced field in gimplify_ctx.  ChangeLog from prior 
version?

Any reason not to use xcalloc to allocate & clear the memory in 
ctx_alloc.  Oh, I see, you want to clear the cached one too.  Nevermind.

Should we ever release the list of ctx pointers?

Do our coding standards allow using default arguments:

extern void push_gimplify_context (bool in_ssa = false,
                                    bool rhs_cond_ok = false);



Jeff

Re: [patch] Privatize gimplify_ctx structure.

[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 gimplify_ctx));

Use
	c = XNEW (struct gimplify_ctx);
instead?

	Jakub

Re: [patch] Privatize gimplify_ctx structure.

[Diego Novillo]

On Wed, Nov 20, 2013 at 1:40 PM, Jeff Law <law@redhat.com> 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 mostly).
http://gcc.gnu.org/codingconventions.html#Default


Diego.

Re: [patch] Privatize gimplify_ctx structure.

[Jeff Law]

On 11/20/13 11:59, Diego Novillo wrote:
> On Wed, Nov 20, 2013 at 1:40 PM, Jeff Law <law@redhat.com> 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 mostly).
> http://gcc.gnu.org/codingconventions.html#Default
Excellent.    I'd just been too lazy to look.

jeff

Re: [patch] Privatize gimplify_ctx structure.

[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 add 'malloced' field.
>>     (gimplify_ctxp): Make static.
>>     (ctx_pool, ctx_alloc, ctx_free): Manage a list of struct 
>> gimplify_ctx.
>>     (push_gimplify_context): Add default parameters and allocate a
>>     struct from the pool.
>>     (pop_gimplify_context): Free a struct back to the pool.
>>     (gimplify_scan_omp_clauses, gimplify_omp_parallel, 
>> gimplify_omp_task,
>>     gimplify_omp_workshare, gimplify_transaction, gimplify_body): Don't
>>     use a local 'struct gimplify_ctx'.
>>     * gimplify-me.c (force_gimple_operand_1, 
>> gimple_regimplify_operands):
>>     Likewise.
>>     * omp-low.c (lower_omp_sections, lower_omp_single, lower_omp_master,
>>     lower_omp_ordered, lower_omp_critical, lower_omp_for,
>>     create_task_copyfn, lower_omp_taskreg, lower_omp_target,
>>     lower_omp_teams, execute_lower_omp): Likewise.
>>     * gimple-fold.c (gimplify_and_update_call_from_tree): Likewise.
>>     * tree-inline.c (optimize_inline_calls): Likewise.
> I don't see the malloced field in gimplify_ctx.  ChangeLog from prior 
> version?
>
> Any reason not to use xcalloc to allocate & clear the memory in 
> ctx_alloc.  Oh, I see, you want to clear the cached one too. Nevermind.
>
> Should we ever release the list of ctx pointers?

There isn't much of a place to do it... I guess you could export a 
free_gimplify_stack () routine and  call at some point at the end of 
finalize_compilation_unit() or something if we think its an issue. Maybe 
the end of cgraphunit.c::expand_all_functions would be the best place... 
it frees its own vector of nodes there as well, and by that point we 
ought to be done.  Or is there a better place?

So something like:

Index: cgraphunit.c
===================================================================
*** cgraphunit.c    (revision 205035)
--- cgraphunit.c    (working copy)
*************** expand_all_functions (void)
*** 1866,1871 ****
--- 1866,1872 ----
       }
       }
     cgraph_process_new_functions ();
+   free_gimplify_stack ();

     free (order);


and

*** gimplify.c    2013-11-20 14:12:57.803369359 -0500
--- G.c    2013-11-20 14:15:32.023013391 -0500
*************** ctx_free (struct gimplify_ctx *c)
*** 195,200 ****
--- 195,215 ----
     ctx_pool = c;
   }

+ /* Free allocated ctx stack memory.  */
+
+ void
+ free_gimplify_stack (void)
+ {
+   struct gimplify_ctx *c;
+
+   while (c = ctx_pool)
+     {
+       ctx_pool = c->prev_context;
+       free (c);
+     }
+ }
+
+


So should we do that?

And per Jakubs suggestion, I'll use XNEW... along with the changelog 
oversite.

Assuming that all works, and no regressions,  OK?

Andrew

Re: [patch] Privatize gimplify_ctx structure.

[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.
>>>     (struct gimplify_ctx): Relocate here and add 'malloced' field.
>>>     (gimplify_ctxp): Make static.
>>>     (ctx_pool, ctx_alloc, ctx_free): Manage a list of struct
>>> gimplify_ctx.
>>>     (push_gimplify_context): Add default parameters and allocate a
>>>     struct from the pool.
>>>     (pop_gimplify_context): Free a struct back to the pool.
>>>     (gimplify_scan_omp_clauses, gimplify_omp_parallel,
>>> gimplify_omp_task,
>>>     gimplify_omp_workshare, gimplify_transaction, gimplify_body): Don't
>>>     use a local 'struct gimplify_ctx'.
>>>     * gimplify-me.c (force_gimple_operand_1,
>>> gimple_regimplify_operands):
>>>     Likewise.
>>>     * omp-low.c (lower_omp_sections, lower_omp_single, lower_omp_master,
>>>     lower_omp_ordered, lower_omp_critical, lower_omp_for,
>>>     create_task_copyfn, lower_omp_taskreg, lower_omp_target,
>>>     lower_omp_teams, execute_lower_omp): Likewise.
>>>     * gimple-fold.c (gimplify_and_update_call_from_tree): Likewise.
>>>     * tree-inline.c (optimize_inline_calls): Likewise.
>> I don't see the malloced field in gimplify_ctx.  ChangeLog from prior
>> version?
>>
>> Any reason not to use xcalloc to allocate & clear the memory in
>> ctx_alloc.  Oh, I see, you want to clear the cached one too. Nevermind.
>>
>> Should we ever release the list of ctx pointers?
>
> There isn't much of a place to do it... I guess you could export a
> free_gimplify_stack () routine and  call at some point at the end of
> finalize_compilation_unit() or something if we think its an issue. Maybe
> the end of cgraphunit.c::expand_all_functions would be the best place...
> it frees its own vector of nodes there as well, and by that point we
> ought to be done.  Or is there a better place?
>
> So something like:
>
> Index: cgraphunit.c
> ===================================================================
> *** cgraphunit.c    (revision 205035)
> --- cgraphunit.c    (working copy)
> *************** expand_all_functions (void)
> *** 1866,1871 ****
> --- 1866,1872 ----
>        }
>        }
>      cgraph_process_new_functions ();
> +   free_gimplify_stack ();
>
>      free (order);
>
>
> and
>
> *** gimplify.c    2013-11-20 14:12:57.803369359 -0500
> --- G.c    2013-11-20 14:15:32.023013391 -0500
> *************** ctx_free (struct gimplify_ctx *c)
> *** 195,200 ****
> --- 195,215 ----
>      ctx_pool = c;
>    }
>
> + /* Free allocated ctx stack memory.  */
> +
> + void
> + free_gimplify_stack (void)
> + {
> +   struct gimplify_ctx *c;
> +
> +   while (c = ctx_pool)
> +     {
> +       ctx_pool = c->prev_context;
> +       free (c);
> +     }
> + }
> +
> +
>
>
> So should we do that?
I think so. Otherwise we just end up adding to the memory leak noise 
from valgrind :-0

>
> And per Jakubs suggestion, I'll use XNEW... along with the changelog
> oversite.
>
> Assuming that all works, and no regressions,  OK?
Yup.
jeff

Re: [patch] Privatize gimplify_ctx structure.

[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.c.
> >>>     * gimplify.c (gimplify_hasher:typed_free_remove): Relocate here.
> >>>     (struct gimplify_ctx): Relocate here and add 'malloced' field.
> >>>     (gimplify_ctxp): Make static.
> >>>     (ctx_pool, ctx_alloc, ctx_free): Manage a list of struct
> >>> gimplify_ctx.
> >>>     (push_gimplify_context): Add default parameters and allocate a
> >>>     struct from the pool.
> >>>     (pop_gimplify_context): Free a struct back to the pool.
> >>>     (gimplify_scan_omp_clauses, gimplify_omp_parallel,
> >>> gimplify_omp_task,
> >>>     gimplify_omp_workshare, gimplify_transaction, gimplify_body): Don't
> >>>     use a local 'struct gimplify_ctx'.
> >>>     * gimplify-me.c (force_gimple_operand_1,
> >>> gimple_regimplify_operands):
> >>>     Likewise.
> >>>     * omp-low.c (lower_omp_sections, lower_omp_single, lower_omp_master,
> >>>     lower_omp_ordered, lower_omp_critical, lower_omp_for,
> >>>     create_task_copyfn, lower_omp_taskreg, lower_omp_target,
> >>>     lower_omp_teams, execute_lower_omp): Likewise.
> >>>     * gimple-fold.c (gimplify_and_update_call_from_tree): Likewise.
> >>>     * tree-inline.c (optimize_inline_calls): Likewise.
> >> I don't see the malloced field in gimplify_ctx.  ChangeLog from prior
> >> version?
> >>
> >> Any reason not to use xcalloc to allocate & clear the memory in
> >> ctx_alloc.  Oh, I see, you want to clear the cached one too. Nevermind.
> >>
> >> Should we ever release the list of ctx pointers?
> >
> > There isn't much of a place to do it... I guess you could export a
> > free_gimplify_stack () routine and  call at some point at the end of
> > finalize_compilation_unit() or something if we think its an issue. Maybe
> > the end of cgraphunit.c::expand_all_functions would be the best place...
> > it frees its own vector of nodes there as well, and by that point we
> > ought to be done.  Or is there a better place?
> >
> > So something like:
> >
> > Index: cgraphunit.c
> > ===================================================================
> > *** cgraphunit.c    (revision 205035)
> > --- cgraphunit.c    (working copy)
> > *************** expand_all_functions (void)
> > *** 1866,1871 ****
> > --- 1866,1872 ----
> >        }
> >        }
> >      cgraph_process_new_functions ();
> > +   free_gimplify_stack ();
> >
> >      free (order);
> >
> >
> > and
> >
> > *** gimplify.c    2013-11-20 14:12:57.803369359 -0500
> > --- G.c    2013-11-20 14:15:32.023013391 -0500
> > *************** ctx_free (struct gimplify_ctx *c)
> > *** 195,200 ****
> > --- 195,215 ----
> >      ctx_pool = c;
> >    }
> >
> > + /* Free allocated ctx stack memory.  */
> > +
> > + void
> > + free_gimplify_stack (void)
> > + {
> > +   struct gimplify_ctx *c;
> > +
> > +   while (c = ctx_pool)
> > +     {
> > +       ctx_pool = c->prev_context;
> > +       free (c);
> > +     }
> > + }
> > +
> > +
> >
> >
> > So should we do that?
> I think so. Otherwise we just end up adding to the memory leak noise 
> from valgrind :-0

This kind of thing is a real issue to the JIT library, of course.
But having a cleanup routine fixes that.
(it's also implicit global state, but that's another battle, I guess;
the JIT has a mutex for that).

Re: [patch] Privatize gimplify_ctx structure.

[Andrew MacLeod]

[-- Attachment #1: Type: text/plain, Size: 378 bytes --]

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 
attached final version was checked in as revision 205168

Andrew

[-- Attachment #2: ctx4.patch --]
[-- Type: text/x-patch, Size: 25366 bytes --]


	* gimplify.h (gimplify_hasher : typed_free_remove, struct gimplify_ctx):
	Move to gimplify.c.
	(free_gimplify_stack): Add prototype.
	* gimplify.c (gimplify_hasher:typed_free_remove): Relocate here.
	(struct gimplify_ctx): Relocate here.
	(gimplify_ctxp): Make static.
	(ctx_pool, ctx_alloc, ctx_free, free_gimplify_stack): New.  Manage a 
	list of struct gimplify_ctx.
	(push_gimplify_context): Add default parameters and allocate a struct
	from the pool.
	(pop_gimplify_context): Free a struct back to the pool.
	(gimplify_scan_omp_clauses, gimplify_omp_parallel, gimplify_omp_task,
	gimplify_omp_workshare, gimplify_transaction, gimplify_body): Don't
	use a local 'struct gimplify_ctx'.
	* cgraphunit.c (expand_all_functions): call free_gimplify_stack.
	* gimplify-me.c (force_gimple_operand_1, gimple_regimplify_operands):
	Don't use a local 'struct gimplify_ctx'.
	* omp-low.c (lower_omp_sections, lower_omp_single, lower_omp_master,
	lower_omp_ordered, lower_omp_critical, lower_omp_for,
	create_task_copyfn, lower_omp_taskreg, lower_omp_target,
	lower_omp_teams, execute_lower_omp): Likewise.
	* gimple-fold.c (gimplify_and_update_call_from_tree): Likewise.
	* tree-inline.c (optimize_inline_calls): Likewise.


Index: gimplify.h
===================================================================
*** gimplify.h	(revision 205035)
--- gimplify.h	(working copy)
*************** enum gimplify_status {
*** 48,86 ****
    GS_OK		= 0,	/* We did something, maybe more to do.  */
    GS_ALL_DONE	= 1	/* The expression is fully gimplified.  */
  };
- /* Gimplify hashtable helper.  */
  
! struct gimplify_hasher : typed_free_remove <elt_t>
! {
!   typedef elt_t value_type;
!   typedef elt_t compare_type;
!   static inline hashval_t hash (const value_type *);
!   static inline bool equal (const value_type *, const compare_type *);
! };
! 
! struct gimplify_ctx
! {
!   struct gimplify_ctx *prev_context;
! 
!   vec<gimple> bind_expr_stack;
!   tree temps;
!   gimple_seq conditional_cleanups;
!   tree exit_label;
!   tree return_temp;
! 
!   vec<tree> case_labels;
!   /* The formal temporary table.  Should this be persistent?  */
!   hash_table <gimplify_hasher> temp_htab;
! 
!   int conditions;
!   bool save_stack;
!   bool into_ssa;
!   bool allow_rhs_cond_expr;
!   bool in_cleanup_point_expr;
! };
! 
! extern struct gimplify_ctx *gimplify_ctxp;
! extern void push_gimplify_context (struct gimplify_ctx *);
  extern void pop_gimplify_context (gimple);
  extern gimple gimple_current_bind_expr (void);
  extern vec<gimple> gimple_bind_expr_stack (void);
--- 48,57 ----
    GS_OK		= 0,	/* We did something, maybe more to do.  */
    GS_ALL_DONE	= 1	/* The expression is fully gimplified.  */
  };
  
! extern void free_gimplify_stack (void);
! extern void push_gimplify_context (bool in_ssa = false,
! 				   bool rhs_cond_ok = false);
  extern void pop_gimplify_context (gimple);
  extern gimple gimple_current_bind_expr (void);
  extern vec<gimple> gimple_bind_expr_stack (void);
Index: gimplify.c
===================================================================
*** gimplify.c	(revision 205035)
--- gimplify.c	(working copy)
*************** enum omp_region_type
*** 89,94 ****
--- 89,125 ----
    ORT_TARGET = 32
  };
  
+ /* Gimplify hashtable helper.  */
+ 
+ struct gimplify_hasher : typed_free_remove <elt_t>
+ {
+   typedef elt_t value_type;
+   typedef elt_t compare_type;
+   static inline hashval_t hash (const value_type *);
+   static inline bool equal (const value_type *, const compare_type *);
+ };
+ 
+ struct gimplify_ctx
+ {
+   struct gimplify_ctx *prev_context;
+ 
+   vec<gimple> bind_expr_stack;
+   tree temps;
+   gimple_seq conditional_cleanups;
+   tree exit_label;
+   tree return_temp;
+ 
+   vec<tree> case_labels;
+   /* The formal temporary table.  Should this be persistent?  */
+   hash_table <gimplify_hasher> temp_htab;
+ 
+   int conditions;
+   bool save_stack;
+   bool into_ssa;
+   bool allow_rhs_cond_expr;
+   bool in_cleanup_point_expr;
+ };
+ 
  struct gimplify_omp_ctx
  {
    struct gimplify_omp_ctx *outer_context;
*************** struct gimplify_omp_ctx
*** 100,109 ****
    bool combined_loop;
  };
  
! struct gimplify_ctx *gimplify_ctxp;
  static struct gimplify_omp_ctx *gimplify_omp_ctxp;
  
- 
  /* Forward declaration.  */
  static enum gimplify_status gimplify_compound_expr (tree *, gimple_seq *, bool);
  
--- 131,139 ----
    bool combined_loop;
  };
  
! static struct gimplify_ctx *gimplify_ctxp;
  static struct gimplify_omp_ctx *gimplify_omp_ctxp;
  
  /* Forward declaration.  */
  static enum gimplify_status gimplify_compound_expr (tree *, gimple_seq *, bool);
  
*************** gimplify_seq_add_seq (gimple_seq *dst_p,
*** 134,147 ****
    gsi_insert_seq_after_without_update (&si, src, GSI_NEW_STMT);
  }
  
  /* Set up a context for the gimplifier.  */
  
  void
! push_gimplify_context (struct gimplify_ctx *c)
  {
!   memset (c, '\0', sizeof (*c));
    c->prev_context = gimplify_ctxp;
    gimplify_ctxp = c;
  }
  
  /* Tear down a context for the gimplifier.  If BODY is non-null, then
--- 164,226 ----
    gsi_insert_seq_after_without_update (&si, src, GSI_NEW_STMT);
  }
  
+ 
+ /* Pointer to a list of allocated gimplify_ctx structs to be used for pushing
+    and popping gimplify contexts.  */
+ 
+ static struct gimplify_ctx *ctx_pool = NULL;
+ 
+ /* Return a gimplify context struct from the pool.  */
+ 
+ static inline struct gimplify_ctx *
+ ctx_alloc (void)
+ {
+   struct gimplify_ctx * c = ctx_pool;
+ 
+   if (c)
+     ctx_pool = c->prev_context;
+   else
+     c = XNEW (struct gimplify_ctx);
+ 
+   memset (c, '\0', sizeof (*c));
+   return c;
+ }
+ 
+ /* Put gimplify context C back into the pool.  */
+ 
+ static inline void
+ ctx_free (struct gimplify_ctx *c)
+ {
+   c->prev_context = ctx_pool;
+   ctx_pool = c;
+ }
+ 
+ /* Free allocated ctx stack memory.  */
+ 
+ void
+ free_gimplify_stack (void)
+ {
+   struct gimplify_ctx *c;
+ 
+   while ((c = ctx_pool))
+     {
+       ctx_pool = c->prev_context;
+       free (c);
+     }
+ }
+ 
+ 
  /* Set up a context for the gimplifier.  */
  
  void
! push_gimplify_context (bool in_ssa, bool rhs_cond_ok)
  {
!   struct gimplify_ctx *c = ctx_alloc ();
! 
    c->prev_context = gimplify_ctxp;
    gimplify_ctxp = c;
+   gimplify_ctxp->into_ssa = in_ssa;
+   gimplify_ctxp->allow_rhs_cond_expr = rhs_cond_ok;
  }
  
  /* Tear down a context for the gimplifier.  If BODY is non-null, then
*************** pop_gimplify_context (gimple body)
*** 168,173 ****
--- 247,253 ----
  
    if (c->temp_htab.is_created ())
      c->temp_htab.dispose ();
+   ctx_free (c);
  }
  
  /* Push a GIMPLE_BIND tuple onto the stack of bindings.  */
*************** gimplify_scan_omp_clauses (tree *list_p,
*** 5726,5732 ****
  			   enum omp_region_type region_type)
  {
    struct gimplify_omp_ctx *ctx, *outer_ctx;
-   struct gimplify_ctx gctx;
    tree c;
  
    ctx = new_omp_context (region_type);
--- 5806,5811 ----
*************** gimplify_scan_omp_clauses (tree *list_p,
*** 5863,5869 ****
  	      omp_add_variable (ctx, OMP_CLAUSE_REDUCTION_PLACEHOLDER (c),
  				GOVD_LOCAL | GOVD_SEEN);
  	      gimplify_omp_ctxp = ctx;
! 	      push_gimplify_context (&gctx);
  
  	      OMP_CLAUSE_REDUCTION_GIMPLE_INIT (c) = NULL;
  	      OMP_CLAUSE_REDUCTION_GIMPLE_MERGE (c) = NULL;
--- 5942,5948 ----
  	      omp_add_variable (ctx, OMP_CLAUSE_REDUCTION_PLACEHOLDER (c),
  				GOVD_LOCAL | GOVD_SEEN);
  	      gimplify_omp_ctxp = ctx;
! 	      push_gimplify_context ();
  
  	      OMP_CLAUSE_REDUCTION_GIMPLE_INIT (c) = NULL;
  	      OMP_CLAUSE_REDUCTION_GIMPLE_MERGE (c) = NULL;
*************** gimplify_scan_omp_clauses (tree *list_p,
*** 5872,5878 ****
  		  		&OMP_CLAUSE_REDUCTION_GIMPLE_INIT (c));
  	      pop_gimplify_context
  		(gimple_seq_first_stmt (OMP_CLAUSE_REDUCTION_GIMPLE_INIT (c)));
! 	      push_gimplify_context (&gctx);
  	      gimplify_and_add (OMP_CLAUSE_REDUCTION_MERGE (c),
  		  		&OMP_CLAUSE_REDUCTION_GIMPLE_MERGE (c));
  	      pop_gimplify_context
--- 5951,5957 ----
  		  		&OMP_CLAUSE_REDUCTION_GIMPLE_INIT (c));
  	      pop_gimplify_context
  		(gimple_seq_first_stmt (OMP_CLAUSE_REDUCTION_GIMPLE_INIT (c)));
! 	      push_gimplify_context ();
  	      gimplify_and_add (OMP_CLAUSE_REDUCTION_MERGE (c),
  		  		&OMP_CLAUSE_REDUCTION_GIMPLE_MERGE (c));
  	      pop_gimplify_context
*************** gimplify_scan_omp_clauses (tree *list_p,
*** 5886,5892 ****
  		   && OMP_CLAUSE_LASTPRIVATE_STMT (c))
  	    {
  	      gimplify_omp_ctxp = ctx;
! 	      push_gimplify_context (&gctx);
  	      if (TREE_CODE (OMP_CLAUSE_LASTPRIVATE_STMT (c)) != BIND_EXPR)
  		{
  		  tree bind = build3 (BIND_EXPR, void_type_node, NULL,
--- 5965,5971 ----
  		   && OMP_CLAUSE_LASTPRIVATE_STMT (c))
  	    {
  	      gimplify_omp_ctxp = ctx;
! 	      push_gimplify_context ();
  	      if (TREE_CODE (OMP_CLAUSE_LASTPRIVATE_STMT (c)) != BIND_EXPR)
  		{
  		  tree bind = build3 (BIND_EXPR, void_type_node, NULL,
*************** gimplify_omp_parallel (tree *expr_p, gim
*** 6309,6322 ****
    tree expr = *expr_p;
    gimple g;
    gimple_seq body = NULL;
-   struct gimplify_ctx gctx;
  
    gimplify_scan_omp_clauses (&OMP_PARALLEL_CLAUSES (expr), pre_p,
  			     OMP_PARALLEL_COMBINED (expr)
  			     ? ORT_COMBINED_PARALLEL
  			     : ORT_PARALLEL);
  
!   push_gimplify_context (&gctx);
  
    g = gimplify_and_return_first (OMP_PARALLEL_BODY (expr), &body);
    if (gimple_code (g) == GIMPLE_BIND)
--- 6388,6400 ----
    tree expr = *expr_p;
    gimple g;
    gimple_seq body = NULL;
  
    gimplify_scan_omp_clauses (&OMP_PARALLEL_CLAUSES (expr), pre_p,
  			     OMP_PARALLEL_COMBINED (expr)
  			     ? ORT_COMBINED_PARALLEL
  			     : ORT_PARALLEL);
  
!   push_gimplify_context ();
  
    g = gimplify_and_return_first (OMP_PARALLEL_BODY (expr), &body);
    if (gimple_code (g) == GIMPLE_BIND)
*************** gimplify_omp_task (tree *expr_p, gimple_
*** 6346,6359 ****
    tree expr = *expr_p;
    gimple g;
    gimple_seq body = NULL;
-   struct gimplify_ctx gctx;
  
    gimplify_scan_omp_clauses (&OMP_TASK_CLAUSES (expr), pre_p,
  			     find_omp_clause (OMP_TASK_CLAUSES (expr),
  					      OMP_CLAUSE_UNTIED)
  			     ? ORT_UNTIED_TASK : ORT_TASK);
  
!   push_gimplify_context (&gctx);
  
    g = gimplify_and_return_first (OMP_TASK_BODY (expr), &body);
    if (gimple_code (g) == GIMPLE_BIND)
--- 6424,6436 ----
    tree expr = *expr_p;
    gimple g;
    gimple_seq body = NULL;
  
    gimplify_scan_omp_clauses (&OMP_TASK_CLAUSES (expr), pre_p,
  			     find_omp_clause (OMP_TASK_CLAUSES (expr),
  					      OMP_CLAUSE_UNTIED)
  			     ? ORT_UNTIED_TASK : ORT_TASK);
  
!   push_gimplify_context ();
  
    g = gimplify_and_return_first (OMP_TASK_BODY (expr), &body);
    if (gimple_code (g) == GIMPLE_BIND)
*************** gimplify_omp_workshare (tree *expr_p, gi
*** 6751,6758 ****
    gimplify_scan_omp_clauses (&OMP_CLAUSES (expr), pre_p, ort);
    if (ort == ORT_TARGET || ort == ORT_TARGET_DATA)
      {
!       struct gimplify_ctx gctx;
!       push_gimplify_context (&gctx);
        gimple g = gimplify_and_return_first (OMP_BODY (expr), &body);
        if (gimple_code (g) == GIMPLE_BIND)
  	pop_gimplify_context (g);
--- 6828,6834 ----
    gimplify_scan_omp_clauses (&OMP_CLAUSES (expr), pre_p, ort);
    if (ort == ORT_TARGET || ort == ORT_TARGET_DATA)
      {
!       push_gimplify_context ();
        gimple g = gimplify_and_return_first (OMP_BODY (expr), &body);
        if (gimple_code (g) == GIMPLE_BIND)
  	pop_gimplify_context (g);
*************** gimplify_transaction (tree *expr_p, gimp
*** 6987,6993 ****
    tree expr = *expr_p, temp, tbody = TRANSACTION_EXPR_BODY (expr);
    gimple g;
    gimple_seq body = NULL;
-   struct gimplify_ctx gctx;
    int subcode = 0;
  
    /* Wrap the transaction body in a BIND_EXPR so we have a context
--- 7063,7068 ----
*************** gimplify_transaction (tree *expr_p, gimp
*** 7000,7006 ****
        TRANSACTION_EXPR_BODY (expr) = bind;
      }
  
!   push_gimplify_context (&gctx);
    temp = voidify_wrapper_expr (*expr_p, NULL);
  
    g = gimplify_and_return_first (TRANSACTION_EXPR_BODY (expr), &body);
--- 7075,7081 ----
        TRANSACTION_EXPR_BODY (expr) = bind;
      }
  
!   push_gimplify_context ();
    temp = voidify_wrapper_expr (*expr_p, NULL);
  
    g = gimplify_and_return_first (TRANSACTION_EXPR_BODY (expr), &body);
*************** gimplify_body (tree fndecl, bool do_parm
*** 8358,8364 ****
    location_t saved_location = input_location;
    gimple_seq parm_stmts, seq;
    gimple outer_bind;
-   struct gimplify_ctx gctx;
    struct cgraph_node *cgn;
  
    timevar_push (TV_TREE_GIMPLIFY);
--- 8433,8438 ----
*************** gimplify_body (tree fndecl, bool do_parm
*** 8368,8374 ****
    default_rtl_profile ();
  
    gcc_assert (gimplify_ctxp == NULL);
!   push_gimplify_context (&gctx);
  
    if (flag_openmp)
      {
--- 8442,8448 ----
    default_rtl_profile ();
  
    gcc_assert (gimplify_ctxp == NULL);
!   push_gimplify_context ();
  
    if (flag_openmp)
      {
Index: cgraphunit.c
===================================================================
*** cgraphunit.c	(revision 205035)
--- cgraphunit.c	(working copy)
*************** along with GCC; see the file COPYING3.
*** 205,210 ****
--- 205,211 ----
  #include "context.h"
  #include "pass_manager.h"
  #include "tree-nested.h"
+ #include "gimplify.h"
  
  /* Queue of cgraph nodes scheduled to be added into cgraph.  This is a
     secondary queue used during optimization to accommodate passes that
*************** expand_all_functions (void)
*** 1866,1871 ****
--- 1867,1873 ----
  	}
      }
    cgraph_process_new_functions ();
+   free_gimplify_stack ();
  
    free (order);
  
Index: gimplify-me.c
===================================================================
*** gimplify-me.c	(revision 205035)
--- gimplify-me.c	(working copy)
*************** force_gimple_operand_1 (tree expr, gimpl
*** 45,51 ****
  			gimple_predicate gimple_test_f, tree var)
  {
    enum gimplify_status ret;
-   struct gimplify_ctx gctx;
    location_t saved_location;
  
    *stmts = NULL;
--- 45,50 ----
*************** force_gimple_operand_1 (tree expr, gimpl
*** 57,72 ****
        && (*gimple_test_f) (expr))
      return expr;
  
!   push_gimplify_context (&gctx);
!   gimplify_ctxp->into_ssa = gimple_in_ssa_p (cfun);
!   gimplify_ctxp->allow_rhs_cond_expr = true;
    saved_location = input_location;
    input_location = UNKNOWN_LOCATION;
  
    if (var)
      {
!       if (gimplify_ctxp->into_ssa
! 	  && is_gimple_reg (var))
  	var = make_ssa_name (var, NULL);
        expr = build2 (MODIFY_EXPR, TREE_TYPE (var), var, expr);
      }
--- 56,68 ----
        && (*gimple_test_f) (expr))
      return expr;
  
!   push_gimplify_context (gimple_in_ssa_p (cfun), true);
    saved_location = input_location;
    input_location = UNKNOWN_LOCATION;
  
    if (var)
      {
!       if (gimple_in_ssa_p (cfun) && is_gimple_reg (var))
  	var = make_ssa_name (var, NULL);
        expr = build2 (MODIFY_EXPR, TREE_TYPE (var), var, expr);
      }
*************** gimple_regimplify_operands (gimple stmt,
*** 160,169 ****
    tree lhs;
    gimple_seq pre = NULL;
    gimple post_stmt = NULL;
-   struct gimplify_ctx gctx;
  
!   push_gimplify_context (&gctx);
!   gimplify_ctxp->into_ssa = gimple_in_ssa_p (cfun);
  
    switch (gimple_code (stmt))
      {
--- 156,163 ----
    tree lhs;
    gimple_seq pre = NULL;
    gimple post_stmt = NULL;
  
!   push_gimplify_context (gimple_in_ssa_p (cfun));
  
    switch (gimple_code (stmt))
      {
Index: gimple-fold.c
===================================================================
*** gimple-fold.c	(revision 205035)
--- gimple-fold.c	(working copy)
*************** gimplify_and_update_call_from_tree (gimp
*** 608,614 ****
    gimple stmt, new_stmt;
    gimple_stmt_iterator i;
    gimple_seq stmts = NULL;
-   struct gimplify_ctx gctx;
    gimple laststore;
    tree reaching_vuse;
  
--- 608,613 ----
*************** gimplify_and_update_call_from_tree (gimp
*** 616,623 ****
  
    gcc_assert (is_gimple_call (stmt));
  
!   push_gimplify_context (&gctx);
!   gctx.into_ssa = gimple_in_ssa_p (cfun);
  
    lhs = gimple_call_lhs (stmt);
    if (lhs == NULL_TREE)
--- 615,621 ----
  
    gcc_assert (is_gimple_call (stmt));
  
!   push_gimplify_context (gimple_in_ssa_p (cfun));
  
    lhs = gimple_call_lhs (stmt);
    if (lhs == NULL_TREE)
Index: omp-low.c
===================================================================
*** omp-low.c	(revision 205035)
--- omp-low.c	(working copy)
*************** lower_omp_sections (gimple_stmt_iterator
*** 8351,8361 ****
    gimple_stmt_iterator tgsi;
    gimple stmt, new_stmt, bind, t;
    gimple_seq ilist, dlist, olist, new_body;
-   struct gimplify_ctx gctx;
  
    stmt = gsi_stmt (*gsi_p);
  
!   push_gimplify_context (&gctx);
  
    dlist = NULL;
    ilist = NULL;
--- 8351,8360 ----
    gimple_stmt_iterator tgsi;
    gimple stmt, new_stmt, bind, t;
    gimple_seq ilist, dlist, olist, new_body;
  
    stmt = gsi_stmt (*gsi_p);
  
!   push_gimplify_context ();
  
    dlist = NULL;
    ilist = NULL;
*************** lower_omp_single (gimple_stmt_iterator *
*** 8561,8569 ****
    tree block;
    gimple t, bind, single_stmt = gsi_stmt (*gsi_p);
    gimple_seq bind_body, bind_body_tail = NULL, dlist;
-   struct gimplify_ctx gctx;
  
!   push_gimplify_context (&gctx);
  
    block = make_node (BLOCK);
    bind = gimple_build_bind (NULL, NULL, block);
--- 8560,8567 ----
    tree block;
    gimple t, bind, single_stmt = gsi_stmt (*gsi_p);
    gimple_seq bind_body, bind_body_tail = NULL, dlist;
  
!   push_gimplify_context ();
  
    block = make_node (BLOCK);
    bind = gimple_build_bind (NULL, NULL, block);
*************** lower_omp_master (gimple_stmt_iterator *
*** 8621,8629 ****
    gimple stmt = gsi_stmt (*gsi_p), bind;
    location_t loc = gimple_location (stmt);
    gimple_seq tseq;
-   struct gimplify_ctx gctx;
  
!   push_gimplify_context (&gctx);
  
    block = make_node (BLOCK);
    bind = gimple_build_bind (NULL, NULL, block);
--- 8619,8626 ----
    gimple stmt = gsi_stmt (*gsi_p), bind;
    location_t loc = gimple_location (stmt);
    gimple_seq tseq;
  
!   push_gimplify_context ();
  
    block = make_node (BLOCK);
    bind = gimple_build_bind (NULL, NULL, block);
*************** lower_omp_ordered (gimple_stmt_iterator
*** 8688,8696 ****
  {
    tree block;
    gimple stmt = gsi_stmt (*gsi_p), bind, x;
-   struct gimplify_ctx gctx;
  
!   push_gimplify_context (&gctx);
  
    block = make_node (BLOCK);
    bind = gimple_build_bind (NULL, NULL, block);
--- 8685,8692 ----
  {
    tree block;
    gimple stmt = gsi_stmt (*gsi_p), bind, x;
  
!   push_gimplify_context ();
  
    block = make_node (BLOCK);
    bind = gimple_build_bind (NULL, NULL, block);
*************** lower_omp_critical (gimple_stmt_iterator
*** 8734,8740 ****
    gimple stmt = gsi_stmt (*gsi_p), bind;
    location_t loc = gimple_location (stmt);
    gimple_seq tbody;
-   struct gimplify_ctx gctx;
  
    name = gimple_omp_critical_name (stmt);
    if (name)
--- 8730,8735 ----
*************** lower_omp_critical (gimple_stmt_iterator
*** 8787,8793 ****
        unlock = build_call_expr_loc (loc, unlock, 0);
      }
  
!   push_gimplify_context (&gctx);
  
    block = make_node (BLOCK);
    bind = gimple_build_bind (NULL, NULL, block);
--- 8782,8788 ----
        unlock = build_call_expr_loc (loc, unlock, 0);
      }
  
!   push_gimplify_context ();
  
    block = make_node (BLOCK);
    bind = gimple_build_bind (NULL, NULL, block);
*************** lower_omp_for (gimple_stmt_iterator *gsi
*** 8877,8885 ****
    gimple stmt = gsi_stmt (*gsi_p), new_stmt;
    gimple_seq omp_for_body, body, dlist;
    size_t i;
-   struct gimplify_ctx gctx;
  
!   push_gimplify_context (&gctx);
  
    lower_omp (gimple_omp_for_pre_body_ptr (stmt), ctx);
  
--- 8872,8879 ----
    gimple stmt = gsi_stmt (*gsi_p), new_stmt;
    gimple_seq omp_for_body, body, dlist;
    size_t i;
  
!   push_gimplify_context ();
  
    lower_omp (gimple_omp_for_pre_body_ptr (stmt), ctx);
  
*************** create_task_copyfn (gimple task_stmt, om
*** 9094,9100 ****
    bool record_needs_remap = false, srecord_needs_remap = false;
    splay_tree_node n;
    struct omp_taskcopy_context tcctx;
-   struct gimplify_ctx gctx;
    location_t loc = gimple_location (task_stmt);
  
    child_fn = gimple_omp_task_copy_fn (task_stmt);
--- 9088,9093 ----
*************** create_task_copyfn (gimple task_stmt, om
*** 9107,9113 ****
      DECL_CONTEXT (t) = child_fn;
  
    /* Populate the function.  */
!   push_gimplify_context (&gctx);
    push_cfun (child_cfun);
  
    bind = build3 (BIND_EXPR, void_type_node, NULL, NULL, NULL);
--- 9100,9106 ----
      DECL_CONTEXT (t) = child_fn;
  
    /* Populate the function.  */
!   push_gimplify_context ();
    push_cfun (child_cfun);
  
    bind = build3 (BIND_EXPR, void_type_node, NULL, NULL, NULL);
*************** lower_omp_taskreg (gimple_stmt_iterator
*** 9387,9393 ****
    gimple stmt = gsi_stmt (*gsi_p);
    gimple par_bind, bind, dep_bind = NULL;
    gimple_seq par_body, olist, ilist, par_olist, par_rlist, par_ilist, new_body;
-   struct gimplify_ctx gctx, dep_gctx;
    location_t loc = gimple_location (stmt);
  
    clauses = gimple_omp_taskreg_clauses (stmt);
--- 9380,9385 ----
*************** lower_omp_taskreg (gimple_stmt_iterator
*** 9412,9418 ****
    if (gimple_code (stmt) == GIMPLE_OMP_TASK
        && find_omp_clause (clauses, OMP_CLAUSE_DEPEND))
      {
!       push_gimplify_context (&dep_gctx);
        dep_bind = gimple_build_bind (NULL, NULL, make_node (BLOCK));
        lower_depend_clauses (stmt, &dep_ilist, &dep_olist);
      }
--- 9404,9410 ----
    if (gimple_code (stmt) == GIMPLE_OMP_TASK
        && find_omp_clause (clauses, OMP_CLAUSE_DEPEND))
      {
!       push_gimplify_context ();
        dep_bind = gimple_build_bind (NULL, NULL, make_node (BLOCK));
        lower_depend_clauses (stmt, &dep_ilist, &dep_olist);
      }
*************** lower_omp_taskreg (gimple_stmt_iterator
*** 9420,9426 ****
    if (ctx->srecord_type)
      create_task_copyfn (stmt, ctx);
  
!   push_gimplify_context (&gctx);
  
    par_olist = NULL;
    par_ilist = NULL;
--- 9412,9418 ----
    if (ctx->srecord_type)
      create_task_copyfn (stmt, ctx);
  
!   push_gimplify_context ();
  
    par_olist = NULL;
    par_ilist = NULL;
*************** lower_omp_target (gimple_stmt_iterator *
*** 9510,9516 ****
    gimple stmt = gsi_stmt (*gsi_p);
    gimple tgt_bind = NULL, bind;
    gimple_seq tgt_body = NULL, olist, ilist, new_body;
-   struct gimplify_ctx gctx;
    location_t loc = gimple_location (stmt);
    int kind = gimple_omp_target_kind (stmt);
    unsigned int map_cnt = 0;
--- 9502,9507 ----
*************** lower_omp_target (gimple_stmt_iterator *
*** 9525,9531 ****
      tgt_body = gimple_omp_body (stmt);
    child_fn = ctx->cb.dst_fn;
  
!   push_gimplify_context (&gctx);
  
    for (c = clauses; c ; c = OMP_CLAUSE_CHAIN (c))
      switch (OMP_CLAUSE_CODE (c))
--- 9516,9522 ----
      tgt_body = gimple_omp_body (stmt);
    child_fn = ctx->cb.dst_fn;
  
!   push_gimplify_context ();
  
    for (c = clauses; c ; c = OMP_CLAUSE_CHAIN (c))
      switch (OMP_CLAUSE_CODE (c))
*************** static void
*** 9811,9818 ****
  lower_omp_teams (gimple_stmt_iterator *gsi_p, omp_context *ctx)
  {
    gimple teams_stmt = gsi_stmt (*gsi_p);
!   struct gimplify_ctx gctx;
!   push_gimplify_context (&gctx);
  
    tree block = make_node (BLOCK);
    gimple bind = gimple_build_bind (NULL, NULL, block);
--- 9802,9808 ----
  lower_omp_teams (gimple_stmt_iterator *gsi_p, omp_context *ctx)
  {
    gimple teams_stmt = gsi_stmt (*gsi_p);
!   push_gimplify_context ();
  
    tree block = make_node (BLOCK);
    gimple bind = gimple_build_bind (NULL, NULL, block);
*************** execute_lower_omp (void)
*** 10105,10114 ****
  
    if (all_contexts->root)
      {
-       struct gimplify_ctx gctx;
- 
        if (task_shared_vars)
! 	push_gimplify_context (&gctx);
        lower_omp (&body, NULL);
        if (task_shared_vars)
  	pop_gimplify_context (NULL);
--- 10095,10102 ----
  
    if (all_contexts->root)
      {
        if (task_shared_vars)
! 	push_gimplify_context ();
        lower_omp (&body, NULL);
        if (task_shared_vars)
  	pop_gimplify_context (NULL);
Index: tree-inline.c
===================================================================
*** tree-inline.c	(revision 205035)
--- tree-inline.c	(working copy)
*************** optimize_inline_calls (tree fn)
*** 4518,4524 ****
    copy_body_data id;
    basic_block bb;
    int last = n_basic_blocks_for_fn (cfun);
-   struct gimplify_ctx gctx;
    bool inlined_p = false;
  
    /* Clear out ID.  */
--- 4518,4523 ----
*************** optimize_inline_calls (tree fn)
*** 4539,4545 ****
    id.transform_lang_insert_block = NULL;
    id.statements_to_fold = pointer_set_create ();
  
!   push_gimplify_context (&gctx);
  
    /* We make no attempts to keep dominance info up-to-date.  */
    free_dominance_info (CDI_DOMINATORS);
--- 4538,4544 ----
    id.transform_lang_insert_block = NULL;
    id.statements_to_fold = pointer_set_create ();
  
!   push_gimplify_context ();
  
    /* We make no attempts to keep dominance info up-to-date.  */
    free_dominance_info (CDI_DOMINATORS);