On Wed, Nov 04, 2015 at 08:58:32PM -0800, Cesar Philippidis wrote:
> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> index e3f55a7..4424596 100644
> --- a/gcc/cp/pt.c
> +++ b/gcc/cp/pt.c
> @@ -14395,6 +14395,15 @@ tsubst_omp_clauses (tree clauses, bool declare_simd,
> bool allow_fields,
> case OMP_CLAUSE_PRIORITY:
> case OMP_CLAUSE_ORDERED:
> case OMP_CLAUSE_HINT:
> + case OMP_CLAUSE_NUM_GANGS:
> + case OMP_CLAUSE_NUM_WORKERS:
> + case OMP_CLAUSE_VECTOR_LENGTH:
> + case OMP_CLAUSE_GANG:
GANG has two arguments, so you want to handle it differently, you need
to tsubst both arguments.
> + case OMP_CLAUSE_WORKER:
> + case OMP_CLAUSE_VECTOR:
> + case OMP_CLAUSE_ASYNC:
> + case OMP_CLAUSE_WAIT:
> + case OMP_CLAUSE_TILE:
I don't think tile will work well this way, if the only argument is a
TREE_VEC, then I think you hit:
case TREE_VEC:
/* A vector of template arguments. */
gcc_assert (!type);
return tsubst_template_args (t, args, complain, in_decl);
which does something very much different from making a copy of the TREE_VEC
and calling tsubst_expr on each argument.
Thus, either you need to handle it manually here, or think about different
representation of OMP_CLAUSE_TILE? It seems you allow at most one tile
clause, so perhaps you could split the single source tile clause into one
tile clause per expression in there (the only issue is that the FEs
emit the clauses in last to first order, so you'd need to nreverse the
tile clause list before adding it to the list of all clauses).
Otherwise it looks ok, except:
> diff --git a/gcc/testsuite/g++.dg/goacc/template-reduction.C
> b/gcc/testsuite/g++.dg/goacc/template-reduction.C
> new file mode 100644
> index 0000000..668eeb3
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/goacc/template-reduction.C
> +++ b/gcc/testsuite/g++.dg/goacc/template.C
the testsuite coverage is orders of magnitude smaller than it should be.
Just look at the amount of OpenMP template tests (both compile time and
runtime):
grep template libgomp/testsuite/libgomp.c++/*[^4] | wc -l; grep -l template
libgomp/testsuite/libgomp.c++/*[^4] | wc -l; grep template
gcc/testsuite/g++.dg/gomp/* | wc -l; grep -l template
gcc/testsuite/g++.dg/gomp/* | wc -l
629 # templates
45 # tests with templates
151 # templates
58 # tests with templates
and even that is really not sufficient. From my experience, special care is
needed in template tests to test both non-type dependent and type-dependent
cases (e.g. some diagnostics is emitted already when parsing the templates
even when they won't be instantiated at all, other diagnostic is done during
instantiation), or for e.g. references there are interesting cases where
a reference to template parameter typename is used or where a reference to
some time is tsubsted into a template parameter typename.
E.g. you don't have any test coverage for the vector (num: ...)
or gang (static: *, num: type_dep)
or gang (static: type_dep1, type_dep2)
(which would show you the above issue with the gang clause), or sufficient
coverage for tile, etc.
Of course that coverage can be added incrementally.
Jakub
