On Tue, May 10, 2016 at 01:29:50PM -0700, Cesar Philippidis wrote:
> @@ -12542,7 +12543,7 @@ c_finish_omp_clauses (tree clauses, enum
> c_omp_region_type ort)
> t = OMP_CLAUSE_DECL (c);
> if (TREE_CODE (t) == TREE_LIST)
> {
> - if (handle_omp_array_sections (c, ort & C_ORT_OMP))
> + if (handle_omp_array_sections (c, ort & (C_ORT_OMP | C_ORT_ACC)))
> {
> remove = true;
> break;
There are no array sections in Cilk+, so the above is always then (for C).
Thus, the is_omp argument to handle_omp_array_sections* should be removed
and code adjusted.
> + if (ort == C_ORT_ACC)
> + error ("%qD appears more than once in data clasues", t);
Typo.
> + else
> + error ("%qD appears both in data and map clauses", t);
> remove = true;
> }
> else
> @@ -12893,7 +12908,10 @@ c_finish_omp_clauses (tree clauses, enum
> c_omp_region_type ort)
> }
> else if (bitmap_bit_p (&map_head, DECL_UID (t)))
> {
> - error ("%qD appears both in data and map clauses", t);
> + if (ort == C_ORT_ACC)
> + error ("%qD appears more than once in data clasues", t);
Likewise.
> + else
> + error ("%qD appears both in data and map clauses", t);
> remove = true;
> }
> else
> @@ -5796,12 +5796,14 @@ tree
> finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
> {
> bitmap_head generic_head, firstprivate_head, lastprivate_head;
> - bitmap_head aligned_head, map_head, map_field_head;
> + bitmap_head aligned_head, map_head, map_field_head, oacc_reduction_head;
> tree c, t, *pc;
> tree safelen = NULL_TREE;
> bool branch_seen = false;
> bool copyprivate_seen = false;
> bool ordered_seen = false;
> + bool allow_fields = (ort & C_ORT_OMP_DECLARE_SIMD) == C_ORT_OMP
> + || ort == C_ORT_ACC;
>
Formatting. You want = already on the new line, or add () around the whole
rhs and align || below (ort &.
Though, this looks wrong to me, does OpenACC all of sudden support
privatization of non-static data members in methods?
> bitmap_obstack_initialize (NULL);
> bitmap_initialize (&generic_head, &bitmap_default_obstack);
> @@ -5810,6 +5812,7 @@ finish_omp_clauses (tree clauses, enum
> c_omp_region_type ort)
> bitmap_initialize (&aligned_head, &bitmap_default_obstack);
> bitmap_initialize (&map_head, &bitmap_default_obstack);
> bitmap_initialize (&map_field_head, &bitmap_default_obstack);
> + bitmap_initialize (&oacc_reduction_head, &bitmap_default_obstack);
>
> for (pc = &clauses, c = clauses; c ; c = *pc)
> {
> @@ -5829,8 +5832,7 @@ finish_omp_clauses (tree clauses, enum
> c_omp_region_type ort)
> t = OMP_CLAUSE_DECL (c);
> if (TREE_CODE (t) == TREE_LIST)
> {
> - if (handle_omp_array_sections (c, ((ort & C_ORT_OMP_DECLARE_SIMD)
> - == C_ORT_OMP)))
> + if (handle_omp_array_sections (c, allow_fields))
IMNSHO you don't want to change this, instead adjust C++
handle_omp_array_sections* where it deals with array sections to just use
the is_omp variant; there are still other places where it deals with
non-static data members and I think you don't want to change those.
> {
> remove = true;
> break;
> @@ -6040,6 +6042,17 @@ finish_omp_clauses (tree clauses, enum
> c_omp_region_type ort)
> omp_clause_code_name[OMP_CLAUSE_CODE (c)]);
> remove = true;
> }
> + else if (ort == C_ORT_ACC
> + && OMP_CLAUSE_CODE (c) == OMP_CLAUSE_REDUCTION)
> + {
> + if (bitmap_bit_p (&oacc_reduction_head, DECL_UID (t)))
> + {
> + error ("%qD appears more than once in reduction clauses", t);
> + remove = true;
> + }
> + else
> + bitmap_set_bit (&oacc_reduction_head, DECL_UID (t));
> + }
> else if (bitmap_bit_p (&generic_head, DECL_UID (t))
> || bitmap_bit_p (&firstprivate_head, DECL_UID (t))
> || bitmap_bit_p (&lastprivate_head, DECL_UID (t)))
> @@ -6050,7 +6063,10 @@ finish_omp_clauses (tree clauses, enum
> c_omp_region_type ort)
> else if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_PRIVATE
> && bitmap_bit_p (&map_head, DECL_UID (t)))
> {
> - error ("%qD appears both in data and map clauses", t);
> + if (ort == C_ORT_ACC)
> + error ("%qD appears more than once in data clauses", t);
> + else
> + error ("%qD appears both in data and map clauses", t);
> remove = true;
> }
> else
> @@ -6076,7 +6092,7 @@ finish_omp_clauses (tree clauses, enum
> c_omp_region_type ort)
> omp_note_field_privatization (t, OMP_CLAUSE_DECL (c));
> else
> t = OMP_CLAUSE_DECL (c);
> - if (t == current_class_ptr)
> + if (ort != C_ORT_ACC && t == current_class_ptr)
> {
> error ("%<this%> allowed in OpenMP only in %<declare simd%>"
> " clauses");
> @@ -6103,7 +6119,10 @@ finish_omp_clauses (tree clauses, enum
> c_omp_region_type ort)
> }
> else if (bitmap_bit_p (&map_head, DECL_UID (t)))
> {
> - error ("%qD appears both in data and map clauses", t);
> + if (ort == C_ORT_ACC)
> + error ("%qD appears more than once in data clauses", t);
> + else
> + error ("%qD appears both in data and map clauses", t);
> remove = true;
> }
> else
> @@ -6551,8 +6570,7 @@ finish_omp_clauses (tree clauses, enum
> c_omp_region_type ort)
> }
> if (TREE_CODE (t) == TREE_LIST)
> {
> - if (handle_omp_array_sections (c, ((ort & C_ORT_OMP_DECLARE_SIMD)
> - == C_ORT_OMP)))
> + if (handle_omp_array_sections (c, allow_fields))
> remove = true;
> break;
> }
> @@ -6586,8 +6604,7 @@ finish_omp_clauses (tree clauses, enum
> c_omp_region_type ort)
> t = OMP_CLAUSE_DECL (c);
> if (TREE_CODE (t) == TREE_LIST)
> {
> - if (handle_omp_array_sections (c, ((ort & C_ORT_OMP_DECLARE_SIMD)
> - == C_ORT_OMP)))
> + if (handle_omp_array_sections (c, allow_fields))
> remove = true;
> else
> {
> @@ -6616,6 +6633,9 @@ finish_omp_clauses (tree clauses, enum
> c_omp_region_type ort)
> if (OMP_CLAUSE_CODE (c) != OMP_CLAUSE_MAP)
> error ("%qD appears more than once in motion"
> " clauses", t);
> + else if (ort == C_ORT_ACC)
> + error ("%qD appears more than once in data"
> + " clauses", t);
> else
> error ("%qD appears more than once in map"
> " clauses", t);
> @@ -6627,6 +6647,27 @@ finish_omp_clauses (tree clauses, enum
> c_omp_region_type ort)
> bitmap_set_bit (&map_field_head, DECL_UID (t));
> }
> }
> + else if (TREE_CODE (t) == TREE_LIST)
> + {
> + while (TREE_CODE (t = TREE_CHAIN (t)) == TREE_LIST)
> + ;
This looks too ugly. Please avoid the assignment inside of TREE_CODE.
Better:
do
t = TREE_CHAIN (t);
while (TREE_CODE (t) == TREE_LIST);
or while loop. Also, I'm surprised you are adding a new whole if, if it is
something you hit only for C_ORT_ACC (why), then it should be also guarded
with ort == C_ORT_ACC. TREE_LIST should mean just type dependent array
section, on which IMHO nothing should be done.
> +
> + if (DECL_P (t))
> + {
> + if (bitmap_bit_p (&map_head, DECL_UID (t)))
> + {
> + if (ort == C_ORT_ACC)
> + error ("%qD appears more than once in data "
> + "clauses", t);
> + else
> + error ("%qD appears more than once in map "
> + "clauses", t);
> + remove = true;
> + }
> + else
> + bitmap_set_bit (&map_head, DECL_UID (t));
> + }
> + }
> }
> break;
> }
> @@ -6703,7 +6744,7 @@ finish_omp_clauses (tree clauses, enum
> c_omp_region_type ort)
> omp_clause_code_name[OMP_CLAUSE_CODE (c)]);
> remove = true;
> }
> - else if (t == current_class_ptr)
> + else if (ort != C_ORT_ACC && t == current_class_ptr)
> {
> error ("%<this%> allowed in OpenMP only in %<declare simd%>"
> " clauses");
> @@ -6752,7 +6793,10 @@ finish_omp_clauses (tree clauses, enum
> c_omp_region_type ort)
> }
> else if (bitmap_bit_p (&map_head, DECL_UID (t)))
> {
> - error ("%qD appears both in data and map clauses", t);
> + if (ort == C_ORT_ACC)
> + error ("%qD appears more than once in data clauses", t);
> + else
> + error ("%qD appears both in data and map clauses", t);
> remove = true;
> }
> else
> @@ -6762,6 +6806,8 @@ finish_omp_clauses (tree clauses, enum
> c_omp_region_type ort)
> {
> if (OMP_CLAUSE_CODE (c) != OMP_CLAUSE_MAP)
> error ("%qD appears more than once in motion clauses", t);
> + if (ort == C_ORT_ACC)
> + error ("%qD appears more than once in data clauses", t);
> else
> error ("%qD appears more than once in map clauses", t);
> remove = true;
> @@ -6769,7 +6815,10 @@ finish_omp_clauses (tree clauses, enum
> c_omp_region_type ort)
> else if (bitmap_bit_p (&generic_head, DECL_UID (t))
> || bitmap_bit_p (&firstprivate_head, DECL_UID (t)))
> {
> - error ("%qD appears both in data and map clauses", t);
> + if (ort == C_ORT_ACC)
> + error ("%qD appears more than once in data clauses", t);
> + else
> + error ("%qD appears both in data and map clauses", t);
> remove = true;
> }
> else
> diff --git a/gcc/gimplify.c b/gcc/gimplify.c
> index f13980d..512b3dd 100644
> --- a/gcc/gimplify.c
> +++ b/gcc/gimplify.c
> @@ -6255,6 +6255,9 @@ omp_notice_variable (struct gimplify_omp_ctx *ctx, tree
> decl, bool in_code)
> error ("variable %qE declared in enclosing "
> "%<host_data%> region", DECL_NAME (decl));
> nflags |= GOVD_MAP;
> + if (octx->region_type == ORT_ACC_DATA
> + && (n2->value & GOVD_MAP_0LEN_ARRAY))
> + nflags |= GOVD_MAP_0LEN_ARRAY;
> goto found_outer;
> }
> }
> @@ -6830,9 +6833,14 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq
> *pre_p,
> {
> case OMP_TARGET:
> break;
> + case OACC_DATA:
> + if (TREE_CODE (TREE_TYPE (decl)) != ARRAY_TYPE)
> + break;
> case OMP_TARGET_DATA:
> case OMP_TARGET_ENTER_DATA:
> case OMP_TARGET_EXIT_DATA:
> + case OACC_ENTER_DATA:
> + case OACC_EXIT_DATA:
> case OACC_HOST_DATA:
> if (OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_FIRSTPRIVATE_POINTER
> || (OMP_CLAUSE_MAP_KIND (c)
> @@ -7286,6 +7294,9 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq
> *pre_p,
> omp_notice_variable (outer_ctx, t, true);
> }
> }
> + if (code == OACC_DATA && OMP_CLAUSE_CODE (c) == OMP_CLAUSE_MAP
> + && OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_FIRSTPRIVATE_POINTER)
If you need more than one line for an if condition, put each && subcondition
on separate line.
> + flags |= GOVD_MAP_0LEN_ARRAY;
> omp_add_variable (ctx, decl, flags);
> if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_REDUCTION
> && OMP_CLAUSE_REDUCTION_PLACEHOLDER (c))
> @@ -7544,6 +7555,9 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq
> *pre_p,
> gcc_unreachable ();
> }
>
> + if (code == OACC_DATA && OMP_CLAUSE_CODE (c) == OMP_CLAUSE_MAP
> + && OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_FIRSTPRIVATE_POINTER)
> + remove = true;
Likewise.
Jakub
