On Tue, Aug 29, 2023 at 06:56:40PM +0200, Tobias Burnus wrote:
> On 29.08.23 18:28, Jakub Jelinek wrote:
> > One thing is that for C++ one needs to be careful about vars optimized
> > by NRV by the FE.
> Thanks for the warning.
> > And, just from the gimplify_bind_expr function name,
>
> (I forgot that one change in there which makes it look larger is that
> I moved the stack handling below the variable cleaning as it makes sense
> to GOMP_free before instead of after wiping the stack via
> __builtin_stack_restore.)
>
> > I'm guessing you are adding the allocations at the start of surrounding
> > BIND_EXPR, is that where
> > we generally want to allocate them?
>
> I actually started with this in a FE implementation, but that
> does not work in general. I need to put the GOMP_alloc right
> before the associated DECL_EXPR, otherwise, it will break for
> code like:
>
> omp_allocator_handle_t my_allocator = omp_low_lat_mem_alloc;
> int n = 5;
> pragma omp allocate(n) allocator(my_allocator)
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?
> For your example, I get (the full gimple dump is below):
>
> foo.c:7:11: warning: statement will never be executed [-Wswitch-unreachable]
> 7 | int j;
>
> I wonder whether we should just error out as GCC does when using a VLA
> at that place:
>
> error: switch jumps into scope of identifier with variably modified type
>
> (with a similar but modified msg wording.) In a sense, the 'allocate' is very
> similar to VLA such that we should be able to piggyback on the VLA diagnostic.
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.
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), 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.
So, to me this feels like we should treat for the crossing initialization
stuff vars mentioned in allocate directive as having a non-trivial
constructor.
If you add default: break; before } above, C++ will reject it with an error,
but C will accept it, it is up to the user to initialize the var again in C
or not use it.
And, as I said earlier, this isn't just about switch, but user gotos as
well.
Jakub
