This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix PR64748
- From: Jakub Jelinek <jakub at redhat dot com>
- To: James Norris <jnorris at codesourcery dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Mon, 15 Feb 2016 14:41:42 +0100
- Subject: Re: [PATCH] Fix PR64748
- Authentication-results: sourceware.org; auth=none
- References: <56AFB4FE dot 1050203 at codesourcery dot com> <20160201200338 dot GK3017 at tucnak dot redhat dot com> <56B0C26B dot 7020200 at codesourcery dot com>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
On Tue, Feb 02, 2016 at 08:51:23AM -0600, James Norris wrote:
> --- a/gcc/cp/semantics.c
> +++ b/gcc/cp/semantics.c
> @@ -6683,6 +6683,14 @@ finish_omp_clauses (tree clauses, bool allow_fields, bool declare_simd)
> error ("%qD appears both in data and map clauses", t);
> remove = true;
> }
> + else if (!processing_template_decl
> + && OMP_CLAUSE_CODE (c) == OMP_CLAUSE_MAP
> + && OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_FORCE_DEVICEPTR
> + && !POINTER_TYPE_P (TREE_TYPE (t)))
> + {
> + error ("%qD is not a pointer variable", t);
> + remove = true;
> + }
Please move this a few lines up, before the first duplicate check, thus
above
else if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_MAP
&& OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_FIRSTPRIVATE_POINTER)
Also, testing it only for !processing_template_decl is undesirable, then you
can't diagnose obvious issues in non-instantiated templates. Better use:
else if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_MAP
&& OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_FORCE_DEVICEPTR
&& !type_dependent_expression_p (t)
&& !POINTER_TYPE_P (TREE_TYPE (t)))
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/goacc/deviceptr-1.C
> @@ -0,0 +1,28 @@
> +// { dg-do compile }
> +
> +template <typename P>
> +
> +void
> +func1 (P p)
> +{
> +
Please avoid the unnecessary empty lines above (both of them).
> +#pragma acc data deviceptr (p) // { dg-error "is not a pointer" }
> +{ }
> +
And here too. Perhaps use " ;" instead of "{ }"? And, more importantly,
by using a single template and instantiating it with both arguments, you are
not testing that you are not diagnosing it for the pointer case.
> +}
> +
> +void
> +func2 (void)
> +{
> + int *p;
> +
> + func1 <int*>(p);
> +}
> +
> +void
> +func3 (void)
> +{
> + int p;
> +
> + func1 <int>(p);
> +}
Also, I don't like the uses of uninitialized vars.
So better
template <typename P>
void
func1 (P p)
{
#pragma acc data deviceptr (p) // { dg-bogus "is not a pointer" }
;
}
void
func2 (int *p)
{
func1 (p);
}
template <typename P>
void
func3 (P p)
{
#pragma acc data deviceptr (p) // { dg-error "is not a pointer" }
;
}
void
func4 (int p)
{
func3 (p);
}
template <int N>
void
func5 (int *p, int q)
{
#pragma acc data deviceptr (p) // { dg-bogus "is not a pointer" }
;
#pragma acc data deviceptr (q) // { dg-error "is not a pointer" }
;
}
func5 added so to test that you diagnose even uninstantiated templates
if the vars/parameters are not type dependent.
Ok for trunk with those changes.
Jakub