Re: [PATCH] [og12] OpenMP: Constructors and destructors for "declare target" static aggregates

[Julian Brown <julian@codesourcery.com>]

On Sun, 2 Apr 2023 11:38:42 +0200
Thomas Schwinge <thomas@codesourcery.com> wrote:

> Hi Julian!
> 
> On 2023-03-27T18:54:30+0000, Julian Brown <julian@codesourcery.com>
> wrote:
> > This patch adds support for running constructors and destructors for
> > static (file-scope) aggregates for C++ objects which are marked with
> > "declare target" directives on OpenMP offload targets.
> >
> > At present, space is allocated on the target for such aggregates,
> > but nothing ever constructs them properly, so they end up
> > zero-initialised.  
> 
> So you've settled that on-device construction is the way to go, and
> not on-host construction before host-to-device copy?

Yes, I think there's less potential for surprises, e.g. if the
constructed object contains pointers to other "declare target" data, or
similar.

(I think it only matters otherwise if the constructor has side-effects
unrelated to the initialisation of the object -- but I don't think the
spec has anything to say about that, at least as of 5.2.)

> I now wonder if we didn't once have a similar issue with Fortran array
> constructors, and how we solved that one.  (I may be misremembering.)

Not sure about that...

> > Tested with offloading to AMD GCN. I will apply to the og12 branch
> > shortly.  
> 
> I've pushed to devel/omp/gcc-12 branch
> commit 472783f3137475b82baadac31cca31021b69aba9
> "Resolve 'error: unused parameter' in
> 'gcc/cp/decl2.cc:one_static_initialization_or_destruction'", see
> attached.

Thank you.

> But glancing at test results, I also see a good number of ICEs.
> That's with '--enable-checking=yes,extra,rtl'.
> 
>      build-gcc/gcc/testsuite/g++/g++.sum                | 924
> ++++++++++++--------- .../libgomp/testsuite/libgomp.sum
>    | 438 ++++++----
> 
> Those are mostly instances of:
> 
>     internal compiler error: tree check: expected omp_clause, have
> tree_list in c_parse_final_cleanups, at cp/decl2.cc:5291
> 
> ..., but also:
> 
>     FAIL: libgomp.c++/static-aggr-constructor-destructor-1.C
> (internal compiler error: tree check: expected omp_clause, have
> tree_list in c_parse_final_cleanups, at cp/decl2.cc:5289) FAIL:
> libgomp.c++/static-aggr-constructor-destructor-1.C (test for excess
> errors) UNRESOLVED:
> libgomp.c++/static-aggr-constructor-destructor-1.C compilation failed
> to produce executable FAIL:
> libgomp.c++/static-aggr-constructor-destructor-2.C (internal compiler
> error: tree check: expected omp_clause, have tree_list in
> c_parse_final_cleanups, at cp/decl2.cc:5289) FAIL:
> libgomp.c++/static-aggr-constructor-destructor-2.C (test for excess
> errors) UNRESOLVED:
> libgomp.c++/static-aggr-constructor-destructor-2.C compilation failed
> to produce executable
> 
> That's in new 'gcc/cp/decl2.cc' code that you've added:
> 
>     5283                while (*fvarsp)
>     5284                  {
>     5285                    tree decl = TREE_VALUE (*fvarsp);
>     5286
>     5287                    if (lookup_attribute ("omp declare
> target", 5288
> DECL_ATTRIBUTES (decl))) 5289                      fvarsp =
> &OMP_CLAUSE_CHAIN (*fvarsp); 5290                    else
>     5291                      *fvarsp = OMP_CLAUSE_CHAIN (*fvarsp);
>     5292                  }
> 
> Please have a look.

I believe these are fixed by:

https://gcc.gnu.org/pipermail/gcc-patches/2023-April/615144.html

Thanks,

Julian