Message ID | 20150709212330.GG10247@tucnak.redhat.com |
---|---|
State | New |
Headers | show |
On Thu, 9 Jul 2015, Jakub Jelinek wrote: > Hi! > > As discussed on IRC, we have a problem because fold_stmt can call > force_gimple_operand_1, which in turn does > push_gimplify_context/pop_gimplify_context (NULL), even when inside > of some gimplify_ctxp and thus pushes the decls into function context > rather than the current gimplify_ctxp. > > So far I've bootstrapped/regtested this version, which supposedly could be > useful for the 5 branch. Other options are to avoid > push_gimplify_context/pop_gimplify_context in force_gimple_operand_1, if > gimplify_ctxp is already non-NULL when it is called (the problem with that > solution is that gimplify_ctxp is now private in gimplify.c and > force_gimple_operand_1 lives somewhere else, so we'd need some accessor > function for that or something). Or avoid force_gimple_operand* from > fold_stmt. I think this is ok for now also on trunk. Yes, I think we should simply disallow pushing gimplifier contexts onto others (not sure where that triggers...). Thus definitely move away from using force_gimple_operand in the middle-end (which is my plan with match-and-simplify anyway). Thanks, Richard. > 2015-07-09 Jakub Jelinek <jakub@redhat.com> > > PR middle-end/66820 > * gimplify.c (maybe_fold_stmt): Don't fold in ORT_PARALLEL > or ORT_TASK contexts. > * omp-low.c (lower_omp): Call fold_stmt even if taskreg_nesting_level > is non-zero. > > * gcc.dg/gomp/pr66820.c: New test. > > --- gcc/gimplify.c.jj 2015-07-08 19:50:04.000000000 +0200 > +++ gcc/gimplify.c 2015-07-09 16:36:58.336609740 +0200 > @@ -2245,16 +2245,17 @@ gimplify_arg (tree *arg_p, gimple_seq *p > return gimplify_expr (arg_p, pre_p, NULL, test, fb); > } > > -/* Don't fold inside offloading regions: it can break code by adding decl > - references that weren't in the source. We'll do it during omplower pass > - instead. */ > +/* Don't fold inside offloading or taskreg regions: it can break code by > + adding decl references that weren't in the source. We'll do it during > + omplower pass instead. */ > > static bool > maybe_fold_stmt (gimple_stmt_iterator *gsi) > { > struct gimplify_omp_ctx *ctx; > for (ctx = gimplify_omp_ctxp; ctx; ctx = ctx->outer_context) > - if (ctx->region_type == ORT_TARGET) > + if (ctx->region_type == ORT_TARGET > + || (ctx->region_type & (ORT_PARALLEL | ORT_TASK)) != 0) > return false; > return fold_stmt (gsi); > } > --- gcc/omp-low.c.jj 2015-07-08 19:49:39.000000000 +0200 > +++ gcc/omp-low.c 2015-07-09 16:37:43.169942838 +0200 > @@ -11890,8 +11890,8 @@ lower_omp (gimple_seq *body, omp_context > for (gsi = gsi_start (*body); !gsi_end_p (gsi); gsi_next (&gsi)) > lower_omp_1 (&gsi, ctx); > /* During gimplification, we haven't folded statments inside offloading > - regions (gimplify.c:maybe_fold_stmt); do that now. */ > - if (target_nesting_level) > + or taskreg regions (gimplify.c:maybe_fold_stmt); do that now. */ > + if (target_nesting_level || taskreg_nesting_level) > for (gsi = gsi_start (*body); !gsi_end_p (gsi); gsi_next (&gsi)) > fold_stmt (&gsi); > input_location = saved_location; > --- gcc/testsuite/gcc.dg/gomp/pr66820.c.jj 2015-04-23 09:27:31.812958582 +0200 > +++ gcc/testsuite/gcc.dg/gomp/pr66820.c 2015-07-09 17:16:34.762225577 +0200 > @@ -0,0 +1,18 @@ > +/* PR middle-end/66820 */ > +/* { dg-do compile } */ > +/* { dg-options "-fopenmp" } */ > + > +void bar (char *); > + > +void > +foo (char **x) > +{ > +#pragma omp parallel for > + for (int i = 0; i < 16; i++) > + { > + char y[50]; > + __builtin_strcpy (y, x[i]); > + __builtin_strcat (y, "foo"); > + bar (y); > + } > +} > > Jakub > >
--- gcc/gimplify.c.jj 2015-07-08 19:50:04.000000000 +0200 +++ gcc/gimplify.c 2015-07-09 16:36:58.336609740 +0200 @@ -2245,16 +2245,17 @@ gimplify_arg (tree *arg_p, gimple_seq *p return gimplify_expr (arg_p, pre_p, NULL, test, fb); } -/* Don't fold inside offloading regions: it can break code by adding decl - references that weren't in the source. We'll do it during omplower pass - instead. */ +/* Don't fold inside offloading or taskreg regions: it can break code by + adding decl references that weren't in the source. We'll do it during + omplower pass instead. */ static bool maybe_fold_stmt (gimple_stmt_iterator *gsi) { struct gimplify_omp_ctx *ctx; for (ctx = gimplify_omp_ctxp; ctx; ctx = ctx->outer_context) - if (ctx->region_type == ORT_TARGET) + if (ctx->region_type == ORT_TARGET + || (ctx->region_type & (ORT_PARALLEL | ORT_TASK)) != 0) return false; return fold_stmt (gsi); } --- gcc/omp-low.c.jj 2015-07-08 19:49:39.000000000 +0200 +++ gcc/omp-low.c 2015-07-09 16:37:43.169942838 +0200 @@ -11890,8 +11890,8 @@ lower_omp (gimple_seq *body, omp_context for (gsi = gsi_start (*body); !gsi_end_p (gsi); gsi_next (&gsi)) lower_omp_1 (&gsi, ctx); /* During gimplification, we haven't folded statments inside offloading - regions (gimplify.c:maybe_fold_stmt); do that now. */ - if (target_nesting_level) + or taskreg regions (gimplify.c:maybe_fold_stmt); do that now. */ + if (target_nesting_level || taskreg_nesting_level) for (gsi = gsi_start (*body); !gsi_end_p (gsi); gsi_next (&gsi)) fold_stmt (&gsi); input_location = saved_location; --- gcc/testsuite/gcc.dg/gomp/pr66820.c.jj 2015-04-23 09:27:31.812958582 +0200 +++ gcc/testsuite/gcc.dg/gomp/pr66820.c 2015-07-09 17:16:34.762225577 +0200 @@ -0,0 +1,18 @@ +/* PR middle-end/66820 */ +/* { dg-do compile } */ +/* { dg-options "-fopenmp" } */ + +void bar (char *); + +void +foo (char **x) +{ +#pragma omp parallel for + for (int i = 0; i < 16; i++) + { + char y[50]; + __builtin_strcpy (y, x[i]); + __builtin_strcat (y, "foo"); + bar (y); + } +}