Revised patch included - addresses part of the issues:
* gimplify.cc: Fix placement of GOMP_alloc by really checking for
   DECL_EXPR (experimented with it before but settled
   for a different pattern)
* c/ Add it to has_jump_unsafe_decl similar to VLA
   + added msg to the switch/goto error handling.
   + new c-c++-common/gomp/allocate-11.c testcase for it

But not all discussion points are solved, yet. Namely,
how to diagnose:

   omp_allocator_handle_t uninit;
   int var, var2;
   uninit = omp_low_lat_mem_alloc;
   omp_allocator_handle_t late_declared = omp_low_lat_mem_alloc;
#pragma omp allocate(var) allocator(uninit)
#pragma omp allocate(var) allocator(late_declared)

(Currently, it is only diagnosed with -Wuninitialized (or -Wextra).)

Otherwise, I think all is covered, but I might have missed something.

Regarding:

On 29.08.23 19:14, Jakub Jelinek wrote:

> What about
>    int n = 5;
>    omp_allocator_handle_t my_allocator = omp_low_lat_mem_alloc;
>    #pragma omp allocate(n) allocator(my_allocator)
> ?  What we do in that case?  Is that invalid because my_allocator
> isn't in scope when n is defined?

IMHO yes - as we can always construct cases with loops in the
execution-order dependence.

The current implementation handles it as VLA,
i.e. warning (VLA vs. allocator):

foo.c:5:3: warning: ‘m’ is used uninitialized [-Wuninitialized]
     5 |   int A[m];
       |   ^~~

foo.c:7:7: warning: ‘my_allocator’ is used uninitialized [-Wuninitialized]

     7 |   int n = 5;

As this is more surprising than VLA, it is probably worth an error
and a more explicit error. The question is how to best detect that
the allocator is either declared after the to-be-omp-allocated variable
declaration or that it is declared before but the assignment happens
only after the declaration (either before 'omp allocate' or not).


I wonder how to best check for this. During parsing, we can check for
TREE_USE and whether an initializer exists, for DECL_EXPR and for
c_check_in_current_scope, but this is not sufficient. We could use
DECL_SOURCE_LOCATION () with '<' comparisons, but I am not sure how it
plays with #line and #include - nor will it catch:
   int var;
   allocator = ...
   #pragma omp allocate(var) allocator(allocator)
where 'allocator' is TREE_USED and declared before 'var' but still wrong.
The 'is used initialized' is done deep in the middle end, but I don't think
we want to add some flag_openmp always-run special case there.

[Admittedly, we do not need to catch all issues and -Wuninitialized, enabled
by -Wextra, will also find this usage. Still, we should try to diagnose the
most common issues.]


> Well, in this case the warning is there just because the patch chose to put
> it at the start of the BIND_EXPR rather than right before DECL_EXPR.

Granted; the way the code before the DECL_EXPR was found wasn't ideal; I
now changed it back to what I previously had - where for C++, I need to
handle also a cleanup_point_expr around DECL_EXPR.

> For switches, there is the case of the switch jumping across declaration
> of an automatic var which is not initialized/constructed (I think in that
> case there is normally no warning/error and happens a lot in the wild
> including GCC sources) but perhaps one could treat those cases with
> #pragma omp allocate as if they are actually constructed (though, I'd still
> raise it at OpenMP F2F),
Can you open an OpenMP spec issue?
> and another case with switch which doesn't even do
> that.
> Consider
>    switch (i)
>      {
>      case 42:
>        bar ();
>        break;
>      case 51:
>        int j = 5;
>        use (&j);
>        break;
>      }
> This is valid for both C and C++, one doesn't jump across any initialization
> in there.  Yet if the j allocation is done at the start of the BIND_EXPR, it
> will jump across that initialization.

With the previously posted patch (and also the current one), that yields:

         <D.2896>:
         D.2900 = __builtin_GOMP_alloc (0, 4, 0B);
         *D.2900 = 5;

which looks fine to me.

Tobias
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955