| Message ID | aab81eb4-1368-425b-0f2f-9d18b62545f3@acm.org |
|---|---|
| State | New |
| Headers | show |
| Series | c++: local-scope OMP UDR reductions have no template head | expand |
On Wed, Sep 16, 2020 at 02:11:32PM -0400, Nathan Sidwell wrote: > Jakub, > are you ok with the bool return from cp_check_omp_declare_reduction? That > seemed like the least invasive change. > > This corrects the earlier problems with removing the template header > from local omp reductions. And it uncovered a latent bug. When we > tsubst such a decl, we immediately tsubst its body. > cp_check_omp_declare_reduction gets a success return value to gate > that instantiation. > > udr-2.C got a further error, as the omp checking machinery doesn't > appear to turn the reduction into an error mark when failing. I > didn't dig into that further. udr-3.C appears to have been invalid > and accidentally worked. The attached patch doesn't match the ChangeLog... > gcc/cp/ > * cp-tree.h (cp_check_omp_declare_reduction): Return bool. > * semantics.c (cp_check_omp_declare_reduction): Return true on for > success. > * pt.c (push_template_decl_real): OMP reductions do not get a > template header. > (tsubst_function_decl): Remove special casing for local decl omp > reductions. > (tsubst_expr): Call instantiate_body for a local omp reduction. > (instantiate_body): Add nested_p parm, and deal with such > instantiations. > (instantiate_decl): Reject FUNCTION_SCOPE entities, adjust > instantiate_body call. > gcc/testsuite/ > * g++.dg/gomp/udr-2.C: Add additional expected error. > libgomp/ > * testsuite/libgomp.c++/udr-3.C: Add missing ctor. > > -- > Nathan Sidwell > diff --git i/gcc/c-family/c-opts.c w/gcc/c-family/c-opts.c > index 23934416f64..24e21ce0d0c 100644 > --- i/gcc/c-family/c-opts.c > +++ w/gcc/c-family/c-opts.c > @@ -1129,7 +1129,10 @@ c_common_post_options (const char **pfilename) > input_location = UNKNOWN_LOCATION; > > *pfilename = this_input_filename > - = cpp_read_main_file (parse_in, in_fnames[0], !cpp_opts->preprocessed); > + = cpp_read_main_file (parse_in, in_fnames[0], > + /* We'll inject preamble pieces if this is > + not preprocessed. */ > + !cpp_opts->preprocessed); > /* Don't do any compilation or preprocessing if there is no input file. */ > if (this_input_filename == NULL) > { > @@ -1453,6 +1456,7 @@ c_finish_options (void) > = linemap_check_ordinary (linemap_add (line_table, LC_RENAME, 0, > _("<built-in>"), 0)); > cb_file_change (parse_in, bltin_map); > + linemap_line_start (line_table, 0, 1); > > /* Make sure all of the builtins about to be declared have > BUILTINS_LOCATION has their location_t. */ > @@ -1476,6 +1480,7 @@ c_finish_options (void) > = linemap_check_ordinary (linemap_add (line_table, LC_RENAME, 0, > _("<command-line>"), 0)); > cb_file_change (parse_in, cmd_map); > + linemap_line_start (line_table, 0, 1); > > /* All command line defines must have the same location. */ > cpp_force_token_locations (parse_in, cmd_map->start_location); Jakub
On 9/16/20 2:31 PM, Jakub Jelinek wrote: > On Wed, Sep 16, 2020 at 02:11:32PM -0400, Nathan Sidwell wrote: >> Jakub, >> are you ok with the bool return from cp_check_omp_declare_reduction? That >> seemed like the least invasive change. >> >> This corrects the earlier problems with removing the template header >> from local omp reductions. And it uncovered a latent bug. When we >> tsubst such a decl, we immediately tsubst its body. >> cp_check_omp_declare_reduction gets a success return value to gate >> that instantiation. >> >> udr-2.C got a further error, as the omp checking machinery doesn't >> appear to turn the reduction into an error mark when failing. I >> didn't dig into that further. udr-3.C appears to have been invalid >> and accidentally worked. > > The attached patch doesn't match the ChangeLog... here's the right one, problem with ordering by time or name :( nathan
On Wed, Sep 16, 2020 at 02:38:22PM -0400, Nathan Sidwell wrote: > On 9/16/20 2:31 PM, Jakub Jelinek wrote: > > On Wed, Sep 16, 2020 at 02:11:32PM -0400, Nathan Sidwell wrote: > > > Jakub, > > > are you ok with the bool return from cp_check_omp_declare_reduction? That > > > seemed like the least invasive change. > > > > > > This corrects the earlier problems with removing the template header > > > from local omp reductions. And it uncovered a latent bug. When we > > > tsubst such a decl, we immediately tsubst its body. > > > cp_check_omp_declare_reduction gets a success return value to gate > > > that instantiation. > > > > > > udr-2.C got a further error, as the omp checking machinery doesn't > > > appear to turn the reduction into an error mark when failing. I > > > didn't dig into that further. udr-3.C appears to have been invalid > > > and accidentally worked. > > > > The attached patch doesn't match the ChangeLog... > > here's the right one, problem with ordering by time or name :( LGTM, if both make check-c++-all RUNTESTFLAGS='gomp.exp goacc.exp' in gcc/ and make check RUNTESTFLAGS=c++.exp in libgomp/ build dirs shows any regressions, no objections. Jakub
diff --git i/gcc/c-family/c-opts.c w/gcc/c-family/c-opts.c index 23934416f64..24e21ce0d0c 100644 --- i/gcc/c-family/c-opts.c +++ w/gcc/c-family/c-opts.c @@ -1129,7 +1129,10 @@ c_common_post_options (const char **pfilename) input_location = UNKNOWN_LOCATION; *pfilename = this_input_filename - = cpp_read_main_file (parse_in, in_fnames[0], !cpp_opts->preprocessed); + = cpp_read_main_file (parse_in, in_fnames[0], + /* We'll inject preamble pieces if this is + not preprocessed. */ + !cpp_opts->preprocessed); /* Don't do any compilation or preprocessing if there is no input file. */ if (this_input_filename == NULL) { @@ -1453,6 +1456,7 @@ c_finish_options (void) = linemap_check_ordinary (linemap_add (line_table, LC_RENAME, 0, _("<built-in>"), 0)); cb_file_change (parse_in, bltin_map); + linemap_line_start (line_table, 0, 1); /* Make sure all of the builtins about to be declared have BUILTINS_LOCATION has their location_t. */ @@ -1476,6 +1480,7 @@ c_finish_options (void) = linemap_check_ordinary (linemap_add (line_table, LC_RENAME, 0, _("<command-line>"), 0)); cb_file_change (parse_in, cmd_map); + linemap_line_start (line_table, 0, 1); /* All command line defines must have the same location. */ cpp_force_token_locations (parse_in, cmd_map->start_location);
Jakub, are you ok with the bool return from cp_check_omp_declare_reduction? That seemed like the least invasive change. This corrects the earlier problems with removing the template header from local omp reductions. And it uncovered a latent bug. When we tsubst such a decl, we immediately tsubst its body. cp_check_omp_declare_reduction gets a success return value to gate that instantiation. udr-2.C got a further error, as the omp checking machinery doesn't appear to turn the reduction into an error mark when failing. I didn't dig into that further. udr-3.C appears to have been invalid and accidentally worked. gcc/cp/ * cp-tree.h (cp_check_omp_declare_reduction): Return bool. * semantics.c (cp_check_omp_declare_reduction): Return true on for success. * pt.c (push_template_decl_real): OMP reductions do not get a template header. (tsubst_function_decl): Remove special casing for local decl omp reductions. (tsubst_expr): Call instantiate_body for a local omp reduction. (instantiate_body): Add nested_p parm, and deal with such instantiations. (instantiate_decl): Reject FUNCTION_SCOPE entities, adjust instantiate_body call. gcc/testsuite/ * g++.dg/gomp/udr-2.C: Add additional expected error. libgomp/ * testsuite/libgomp.c++/udr-3.C: Add missing ctor.