Hi!
CCing Jason on a tsubst issue below.
On Wed, Jul 15, 2015 at 06:05:06PM -0700, Aldy Hernandez wrote:
> As I said on IRC, this looks ugly, but I can see why you wouldn't want the
> extra word on all loop variants. I've implemented it as requested.
Thanks.
> commit f55eced4ac6b045101a90914a8f27e99d26cfddf
> Author: Aldy Hernandez <al...@redhat.com>
> Date: Tue Jul 14 19:23:09 2015 -0700
>
> * gimplify.c (gimplify_omp_for): Use OMP_FOR_ORIG_DECLS.
> * tree.def (omp_for): Add new operand.
> * tree.h (OMP_FOR_ORIG_DECLS): New macro.
> c-family/
> * c-common.h (c_finish_omp_for): Add argument.
> * c-omp.c (c_finish_omp_for): Set OMP_FOR_ORIG_DECLS.
> cp/
> * cp-tree.h (finish_omp_for): Add new argument.
> * parser.c (cp_parser_omp_for_loop): Pass new argument.
> * pt.c (tsubst_omp_for_iterator): New argument orig_declv.
> Set OMP_FOR_ORIG_DECLS from orig_declv if available.
> (tsubst_expr): Pass new vector to tsubst_omp_for_iterator.
> * semantics.c (finish_omp_for): Pass original DECLs to
> c_finish_omp_for.
> Set OMP_FOR_ORIG_DECLS.
> c/
> * c-parser.c (c_parser_omp_for_loop): Pass new argument to
> c_finish_omp_for.
Can you please add the
testsuite/
additions to ChangeLog too?
> @@ -13828,6 +13828,16 @@ tsubst_omp_for_iterator (tree t, int i, tree declv,
> tree initv,
>
> init = TREE_VEC_ELT (OMP_FOR_INIT (t), i);
> gcc_assert (TREE_CODE (init) == MODIFY_EXPR);
> +
> + if (orig_declv && OMP_FOR_ORIG_DECLS (t))
> + {
> + tree o = TREE_VEC_ELT (OMP_FOR_ORIG_DECLS (t), i);
> + tree spec = retrieve_local_specialization (o);
> + if (spec)
> + o = spec;
Why doesn't o = RECUR (o); work here? Or tsubst_copy?
>From my experience tsubst_expr can result in (here undesirable)
>convert_from_reference
if the decl has type of e.g. template parameter and you instantiate with
some reference type. But, that can only happen if the decl is dependent,
see below:
> @@ -14468,6 +14479,8 @@ tsubst_expr (tree t, tree args, tsubst_flags_t
> complain, tree in_decl,
> if (OMP_FOR_INIT (t) != NULL_TREE)
> {
> declv = make_tree_vec (TREE_VEC_LENGTH (OMP_FOR_INIT (t)));
> + if (TREE_CODE (t) == OMP_FOR)
> + orig_declv = make_tree_vec (TREE_VEC_LENGTH (OMP_FOR_INIT (t)));
I'd have expected to guard this also with
if (TREE_CODE (t) == OMP_FOR && OMP_FOR_ORIG_DECLS (t) != NULL_TREE)
> @@ -7302,6 +7303,9 @@ finish_omp_for (location_t locus, enum tree_code code,
> tree declv, tree initv,
> TREE_VEC_ELT (initv, i) = init;
> }
>
> + if (!orig_declv || !TREE_VEC_LENGTH (orig_declv))
> + orig_declv = copy_node (declv);
When is TREE_VEC_LENGTH 0? Furthermore, I'd do this only after the
if (dependent_omp_for_p ())
> +
> if (dependent_omp_for_p (declv, initv, condv, incrv))
> {
> tree stmt;
> @@ -7325,6 +7329,8 @@ finish_omp_for (location_t locus, enum tree_code code,
> tree declv, tree initv,
> OMP_FOR_BODY (stmt) = body;
> OMP_FOR_PRE_BODY (stmt) = pre_body;
> OMP_FOR_CLAUSES (stmt) = clauses;
> + if (code == OMP_FOR)
> + OMP_FOR_ORIG_DECLS (stmt) = orig_declv;
And leave this out. If something is dependent, we don't really modify it
before instantiation, thus there is no need to make a copy. During
instantiation another finish_omp_for will be called, then declv, initv,
condv, incrv won't be dependent and we can orig_declv = copy_node (declv).
> diff --git a/gcc/testsuite/c-c++-common/gomp/sink-2.c
> b/gcc/testsuite/c-c++-common/gomp/sink-2.c
> new file mode 100644
> index 0000000..7a075d4
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/gomp/sink-2.c
> @@ -0,0 +1,15 @@
> +/* { dg-do compile } */
> +
> +void bar (int *);
> +
> +void
> +foo ()
> +{
> + int i,j;
> +#pragma omp parallel for ordered(1)
> + for (i=0; i < 100; ++i)
> + {
> +#pragma omp ordered depend(sink:i-1)
> + bar(&i);
Can you please add #pragma omp ordered depend(source) below the bar call?
Jakub
