Re: [Patch] OpenMP (C only): omp allocate - extend parsing support, improve diagnostic (was: [Patch] OpenMP (C only): omp allocate - handle stack vars, improve diagnostic)

[Jakub Jelinek <jakub@redhat.com>]

Hi!

One question to David below, CCed.

On Mon, Sep 11, 2023 at 01:44:07PM +0200, Tobias Burnus wrote:
> --- a/gcc/c/c-decl.cc
> +++ b/gcc/c/c-decl.cc
> @@ -681,6 +681,11 @@ decl_jump_unsafe (tree decl)
>    if (VAR_P (decl) && C_DECL_COMPOUND_LITERAL_P (decl))
>      return false;
>  
> +  if (flag_openmp
> +      && VAR_P (decl)
> +      && lookup_attribute ("omp allocate", DECL_ATTRIBUTES (decl)))
> +    return true;
> +
>    /* Always warn about crossing variably modified types.  */
>    if ((VAR_P (decl) || TREE_CODE (decl) == TYPE_DECL)
>        && c_type_variably_modified_p (TREE_TYPE (decl)))
> @@ -724,6 +729,12 @@ c_print_identifier (FILE *file, tree node, int indent)
>    c_binding_oracle = save;
>  }
>  

I think we want a function comment here.

> +void
> +c_mark_decl_jump_unsafe_in_current_scope ()
> +{
> +  current_scope->has_jump_unsafe_decl = 1;
> +}
> +
> +	  if (DECL_SOURCE_LOCATION (allocator) > DECL_SOURCE_LOCATION (var))
> +	    {
> +	      error_at (OMP_CLAUSE_LOCATION (nl),
> +			"allocator variable %qD must be declared before %qD",
> +			allocator, var);
> +	      inform (DECL_SOURCE_LOCATION (allocator), "declared here");
> +	      inform (DECL_SOURCE_LOCATION (var), "declared here");

I think this will be confusing to users when the inform is the same in both
cases.  I'd use "allocator declared here" in the first case.

And, am really not sure if one can just simply compare location_t like that.
Isn't there some function which determines what source location is before
another one?  David?

> +		 if (EXPR_LOCATION (*l) < DECL_SOURCE_LOCATION (var))
> +		   break;

Likewise.

> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/gomp/allocate-12.c
> @@ -0,0 +1,46 @@
> +/* TODO: enable for C++ once implemented. */
> +/* { dg-do compile { target c } } */
> +
> +typedef enum omp_allocator_handle_t

I think all the c-c++-common tests declaring
omp_allocator_handle_t should have
#if __cplusplus >= 201103L
: __UINTPTR_TYPE__
#endif
here (even if they are just { target c } for now, we'd certainly
forget to adjust them).


	Jakub