Hi Cesar! On Tue, 27 Oct 2015 11:36:10 -0700, Cesar Philippidis wrote: > This patch contains the following: > > * C front end changes from trunk: > https://gcc.gnu.org/ml/gcc-patches/2015-10/msg02528.html > > * C++ front end changes from trunk: > https://gcc.gnu.org/ml/gcc-patches/2015-10/msg02540.html > > * Proposed fortran cleanups and enhanced error reporting changes: > https://gcc.gnu.org/ml/gcc-patches/2015-10/msg02288.html I suppose the latter is a prerequisite for other Fortran front end changes you've also committed? Otherwise, why not get that patch into trunk first? That sould save me from having to deal with potentially more merge conflicts later on... > In addition, I've also added a couple of more test cases and updated the > way that combined directives are handled in fortran. Because the > device_type clauses form a chain of gfc_omp_clauses, I couldn't reuse > gfc_split_omp_clauses for combined parallel and kernels loops. So that's > why I introduced a new gfc_filter_oacc_combined_clauses function. Thanks, but... > I'll apply this patch to gomp-4_0-branch shortly. I know that I should > have broken this patch down into smaller patches Yes. > but it was already > arranged as one big patch in my source tree. You're using Git, so that's not a good excuse. :-P > --- a/gcc/fortran/trans-openmp.c > +++ b/gcc/fortran/trans-openmp.c > @@ -3634,12 +3634,65 @@ gfc_trans_omp_do (gfc_code *code, gfc_exec_op op, stmtblock_t *pblock, > return gfc_finish_block (&block); > } > > -/* parallel loop and kernels loop. */ > +/* Helper function to filter combined oacc constructs. ORIG_CLAUSES > + contains the unfiltered list of clauses. LOOP_CLAUSES corresponds to > + the filter list of loop clauses corresponding to the enclosed list. > + This function is called recursively to handle device_type clauses. */ > + > +static void > +gfc_filter_oacc_combined_clauses (gfc_omp_clauses **orig_clauses, > + gfc_omp_clauses **loop_clauses) > +{ > + if (*orig_clauses == NULL) > + { > + *loop_clauses = NULL; > + return; > + } > + > + *loop_clauses = gfc_get_omp_clauses (); > + > + memset (*loop_clauses, 0, sizeof (gfc_omp_clauses)); This has already been created zero-initialized. > + (*loop_clauses)->gang = (*orig_clauses)->gang; > + (*orig_clauses)->gang = false; > + (*loop_clauses)->gang_expr = (*orig_clauses)->gang_expr; > + (*orig_clauses)->gang_expr = NULL; > + (*loop_clauses)->gang_static = (*orig_clauses)->gang_static; > + (*orig_clauses)->gang_static = false; > + (*loop_clauses)->vector = (*orig_clauses)->vector; > + (*orig_clauses)->vector = false; > + (*loop_clauses)->vector_expr = (*orig_clauses)->vector_expr; > + (*orig_clauses)->vector_expr = NULL; > + (*loop_clauses)->worker = (*orig_clauses)->worker; > + (*orig_clauses)->worker = false; > + (*loop_clauses)->worker_expr = (*orig_clauses)->worker_expr; > + (*orig_clauses)->worker_expr = NULL; > + (*loop_clauses)->seq = (*orig_clauses)->seq; > + (*orig_clauses)->seq = false; > + (*loop_clauses)->independent = (*orig_clauses)->independent; > + (*orig_clauses)->independent = false; > + (*loop_clauses)->par_auto = (*orig_clauses)->par_auto; > + (*orig_clauses)->par_auto = false; > + (*loop_clauses)->acc_collapse = (*orig_clauses)->acc_collapse; > + (*orig_clauses)->acc_collapse = false; > + (*loop_clauses)->collapse = (*orig_clauses)->collapse; > + /* Don't reset (*orig_clauses)->collapse. */ Why? (Extend source code comment?) The original code (cited just below) did this differently. > + (*loop_clauses)->tile = (*orig_clauses)->tile; > + (*orig_clauses)->tile = false; > + (*loop_clauses)->tile_list = (*orig_clauses)->tile_list; > + (*orig_clauses)->tile_list = NULL; > + (*loop_clauses)->device_types = (*orig_clauses)->device_types; > + > + gfc_filter_oacc_combined_clauses (&(*orig_clauses)->dtype_clauses, > + &(*loop_clauses)->dtype_clauses); > +} > + > +/* Combined OpenACC parallel loop and kernels loop. */ > static tree > gfc_trans_oacc_combined_directive (gfc_code *code) > { > stmtblock_t block, inner, *pblock = NULL; > - gfc_omp_clauses construct_clauses, loop_clauses; > + gfc_omp_clauses *loop_clauses; > tree stmt, oacc_clauses = NULL_TREE; > enum tree_code construct_code; > bool scan_nodesc_arrays = false; > @@ -3661,39 +3714,18 @@ gfc_trans_oacc_combined_directive (gfc_code *code) > > gfc_start_block (&block); > > - memset (&loop_clauses, 0, sizeof (loop_clauses)); > - if (code->ext.omp_clauses != NULL) > - { > - memcpy (&construct_clauses, code->ext.omp_clauses, > - sizeof (construct_clauses)); > - loop_clauses.collapse = construct_clauses.collapse; > - loop_clauses.gang = construct_clauses.gang; > - loop_clauses.gang_expr = construct_clauses.gang_expr; > - loop_clauses.gang_static = construct_clauses.gang_static; > - loop_clauses.vector = construct_clauses.vector; > - loop_clauses.vector_expr = construct_clauses.vector_expr; > - loop_clauses.worker = construct_clauses.worker; > - loop_clauses.worker_expr = construct_clauses.worker_expr; > - loop_clauses.seq = construct_clauses.seq; > - loop_clauses.independent = construct_clauses.independent; > - construct_clauses.collapse = 0; > - construct_clauses.gang = false; > - construct_clauses.vector = false; > - construct_clauses.worker = false; > - construct_clauses.seq = false; > - construct_clauses.independent = false; > - oacc_clauses = gfc_trans_omp_clauses (&block, &construct_clauses, > - code->loc); > - } > + gfc_filter_oacc_combined_clauses (&code->ext.omp_clauses, &loop_clauses); > + oacc_clauses = gfc_trans_omp_clauses (&block, code->ext.omp_clauses, > + code->loc); > > array_set = gfc_init_nodesc_arrays (&inner, &oacc_clauses, code, > scan_nodesc_arrays); > With... > --- /dev/null > +++ b/gcc/testsuite/gfortran.dg/goacc/combined-directives.f90 ... newly added, and... > --- a/libgomp/testsuite/libgomp.oacc-fortran/combdir-1.f90 > +++ /dev/null ... renamed to... > --- /dev/null > +++ b/libgomp/testsuite/libgomp.oacc-fortran/combined-directive-1.f90 ... this, plus the unaltered libgomp/testsuite/libgomp.oacc-c-c++-common/combdir-1.c, we now got three variants: "combined-directives", "combined-directive", and "combdir". Please settle on one of them. > --- a/gcc/testsuite/gfortran.dg/gomp/intentin1.f90 > +++ b/gcc/testsuite/gfortran.dg/gomp/intentin1.f90 > @@ -11,6 +11,6 @@ subroutine foo (x) > !$omp simd linear (x) ! { dg-error "INTENT.IN. POINTER" } > do i = 1, 10 > end do > -!$omp single ! { dg-error "INTENT.IN. POINTER" } > -!$omp end single copyprivate (x) > +!$omp single > +!$omp end single copyprivate (x) ! { dg-error "INTENT.IN. POINTER" } > end Please revert this unrelated change. Additionally, in r229482 I checked in the following to restore bootstrap builds: [...]/source-gcc/gcc/cp/parser.c:29649:1: error: 'tree_node* require_positive_expr(tree, location_t, const char*)' defined but not used [-Werror=unused-function] require_positive_expr (tree t, location_t loc, const char *str) ^ [...]/source-gcc/gcc/fortran/openmp.c: In function 'match gfc_match_oacc_declare()': [...]/source-gcc/gcc/fortran/openmp.c:1449:30: error: unused variable 'oc' [-Werror=unused-variable] gfc_oacc_declare *new_oc, *oc; ^ [...]/source-gcc/gcc/fortran/openmp.c: In function 'void gfc_resolve_oacc_declare(gfc_namespace*)': [...]/source-gcc/gcc/fortran/openmp.c:4918:7: error: unused variable 'list' [-Werror=unused-variable] int list; ^ commit 1e53d795ea68bcc7e5d5a7fa21e58c506e557cb4 Author: tschwinge Date: Wed Oct 28 10:57:45 2015 +0000 Address -Werror=unused-variable and -Werror=unused-function diagnostics gcc/cp/ * parser.c (require_positive_expr): Remove function. gcc/fortran/ * openmp.c (gfc_match_oacc_declare): Remove oc local variable. (gfc_resolve_oacc_declare): Remove list local variable. git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/branches/gomp-4_0-branch@229482 138bc75d-0d04-0410-961f-82ee72b054a4 --- gcc/cp/ChangeLog.gomp | 4 ++++ gcc/cp/parser.c | 17 ----------------- gcc/fortran/ChangeLog.gomp | 5 +++++ gcc/fortran/openmp.c | 3 +-- 4 files changed, 10 insertions(+), 19 deletions(-) diff --git gcc/cp/ChangeLog.gomp gcc/cp/ChangeLog.gomp index c0b96ba..b96bfce 100644 --- gcc/cp/ChangeLog.gomp +++ gcc/cp/ChangeLog.gomp @@ -1,3 +1,7 @@ +2015-10-28 Thomas Schwinge + + * parser.c (require_positive_expr): Remove function. + 2015-10-27 Cesar Philippidis * parser.c (cp_parser_oacc_shape_clause): Backport from trunk. diff --git gcc/cp/parser.c gcc/cp/parser.c index d113cfb..2c6060b 100644 --- gcc/cp/parser.c +++ gcc/cp/parser.c @@ -29642,23 +29642,6 @@ cp_parser_oacc_data_clause_deviceptr (cp_parser *parser, tree list) return list; } -/* Attempt to statically determine when the number T isn't positive. - Warn if we determined this and return positive one as the new - expression. */ -static tree -require_positive_expr (tree t, location_t loc, const char *str) -{ - tree c = fold_build2_loc (loc, LE_EXPR, boolean_type_node, t, - build_int_cst (TREE_TYPE (t), 0)); - if (c == boolean_true_node) - { - warning_at (loc, 0, - "%<%s%> value must be positive", str); - t = integer_one_node; - } - return t; -} - /* OpenACC: num_gangs ( expression ) num_workers ( expression ) diff --git gcc/fortran/ChangeLog.gomp gcc/fortran/ChangeLog.gomp index d126367..9e1b95b 100644 --- gcc/fortran/ChangeLog.gomp +++ gcc/fortran/ChangeLog.gomp @@ -1,3 +1,8 @@ +2015-10-28 Thomas Schwinge + + * openmp.c (gfc_match_oacc_declare): Remove oc local variable. + (gfc_resolve_oacc_declare): Remove list local variable. + 2015-10-27 Cesar Philippidis * gfortran.h (gfc_omp_namelist): Add locus where member. diff --git gcc/fortran/openmp.c gcc/fortran/openmp.c index 9621eaf..afcce9a 100644 --- gcc/fortran/openmp.c +++ gcc/fortran/openmp.c @@ -1446,7 +1446,7 @@ gfc_match_oacc_declare (void) gfc_omp_clauses *c; gfc_omp_namelist *n; gfc_namespace *ns = gfc_current_ns; - gfc_oacc_declare *new_oc, *oc; + gfc_oacc_declare *new_oc; bool module_var = false; if (gfc_match_omp_clauses (&c, OACC_DECLARE_CLAUSES, 0, false, false, true) @@ -4915,7 +4915,6 @@ resolve_oacc_declare_map (gfc_oacc_declare *declare, int list) void gfc_resolve_oacc_declare (gfc_namespace *ns) { - int list; gfc_omp_namelist *n; locus loc; gfc_oacc_declare *oc; Grüße Thomas