This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix PR64748


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]