| Message ID | 56531154.9080606@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
Ping. On 2015/11/23 9:15 PM, Chung-Lin Tang wrote: > The OpenACC wait directive is represented as a call to the runtime > function "GOACC_wait" instead of a tree code. I am seeing when > '#pragma acc wait' is using inside a template function, the CALL_EXPR > to GOACC_wait is being silently ignored/removed during tsubst_expr(). > > I think the correct way to organize this is that the call should be inside > an EXPR_STMT, so here's a patch to do that; basically remove the > add_stmt() call from the shared c_finish_oacc_wait() code, and add > add_stmt()/finish_expr_stmt() in the corresponding C/C++ parts. > > Tested with no regressions on trunk, okay to commit? > > Thanks, > Chung-Lin > > * c-family/c-omp.c (c_finish_oacc_wait): Remove add_stmt() call. > * c/c-parser.c (c_parser_oacc_wait): Add add_stmt() call. > * cp/parser.c (cp_parser_oacc_wait): Add finish_expr_stmt() call. >
Hi Chung-Lin! On Mon, 23 Nov 2015 21:15:00 +0800, Chung-Lin Tang <cltang@codesourcery.com> wrote: > The OpenACC wait directive is represented as a call to the runtime > function "GOACC_wait" instead of a tree code. I am seeing when > '#pragma acc wait' is using inside a template function, the CALL_EXPR > to GOACC_wait is being silently ignored/removed during tsubst_expr(). Uh. > I think the correct way to organize this is that the call should be inside > an EXPR_STMT, so here's a patch to do that; basically remove the > add_stmt() call from the shared c_finish_oacc_wait() code, and add > add_stmt()/finish_expr_stmt() in the corresponding C/C++ parts. > > Tested with no regressions on trunk, okay to commit? > --- c-family/c-omp.c (revision 230703) > +++ c-family/c-omp.c (working copy) > @@ -63,7 +63,6 @@ c_finish_oacc_wait (location_t loc, tree parms, tr > } > > stmt = build_call_expr_loc_vec (loc, stmt, args); > - add_stmt (stmt); > > vec_free (args); | | return stmt; | } I see in gcc/c/c-omp.c that several other c_finish_omp_* functions that build builtin calls instead of tree nodes, do similar things like c_finish_oacc_wait; I'd like to understand why it's -- presumably -- not a problem for these: c_finish_omp_barrier, c_finish_omp_taskwait, c_finish_omp_taskyield, c_finish_omp_flush? (Jakub?) > --- c/c-parser.c (revision 230703) > +++ c/c-parser.c (working copy) > @@ -13886,6 +13886,7 @@ c_parser_oacc_wait (location_t loc, c_parser *pars > strcpy (p_name, " wait"); > clauses = c_parser_oacc_all_clauses (parser, OACC_WAIT_CLAUSE_MASK, p_name); > stmt = c_finish_oacc_wait (loc, list, clauses); > + add_stmt (stmt); > > return stmt; > } > --- cp/parser.c (revision 230703) > +++ cp/parser.c (working copy) > @@ -34930,6 +34930,7 @@ cp_parser_oacc_wait (cp_parser *parser, cp_token * > "#pragma acc wait", pragma_tok); > > stmt = c_finish_oacc_wait (loc, list, clauses); > + stmt = finish_expr_stmt (stmt); > > return stmt; > } Grüße Thomas
Hi! On Thu, 03 Dec 2015 09:51:31 +0100, I wrote: > On Mon, 23 Nov 2015 21:15:00 +0800, Chung-Lin Tang <cltang@codesourcery.com> wrote: > > The OpenACC wait directive is represented as a call to the runtime > > function "GOACC_wait" instead of a tree code. I am seeing when > > '#pragma acc wait' is using inside a template function, the CALL_EXPR > > to GOACC_wait is being silently ignored/removed during tsubst_expr(). > > Uh. > > > I think the correct way to organize this is that the call should be inside > > an EXPR_STMT, so here's a patch to do that; basically remove the > > add_stmt() call from the shared c_finish_oacc_wait() code, and add > > add_stmt()/finish_expr_stmt() in the corresponding C/C++ parts. > > > > Tested with no regressions on trunk, okay to commit? > > > --- c-family/c-omp.c (revision 230703) > > +++ c-family/c-omp.c (working copy) > > @@ -63,7 +63,6 @@ c_finish_oacc_wait (location_t loc, tree parms, tr > > } > > > > stmt = build_call_expr_loc_vec (loc, stmt, args); > > - add_stmt (stmt); > > > > vec_free (args); > | > | return stmt; > | } > > I see in gcc/c/c-omp.c that several other c_finish_omp_* functions that > build builtin calls instead of tree nodes, do similar things like > c_finish_oacc_wait; I'd like to understand why it's -- presumably -- not > a problem for these: c_finish_omp_barrier, c_finish_omp_taskwait, > c_finish_omp_taskyield, c_finish_omp_flush? (Jakub?) Oh wait, it looks like the C++ front end is not actually using the functions defined in the C/C++-shared gcc/c-family/c-omp.c, but has its own implementations in gcc/cp/semantics.c, without "c_" prefixes? In addition to finish_expr_stmt calls, I see it's also using finish_call_expr instead of build_call_expr_loc/build_call_expr_loc_vec. So I guess we'll want to model this the same way for OpenACC support functions, and then (later) we should clean this up, to move the C-specific code from gcc/c-family/c-omp.c into the C front end? (Jakub?) > > --- c/c-parser.c (revision 230703) > > +++ c/c-parser.c (working copy) > > @@ -13886,6 +13886,7 @@ c_parser_oacc_wait (location_t loc, c_parser *pars > > strcpy (p_name, " wait"); > > clauses = c_parser_oacc_all_clauses (parser, OACC_WAIT_CLAUSE_MASK, p_name); > > stmt = c_finish_oacc_wait (loc, list, clauses); > > + add_stmt (stmt); > > > > return stmt; > > } > > --- cp/parser.c (revision 230703) > > +++ cp/parser.c (working copy) > > @@ -34930,6 +34930,7 @@ cp_parser_oacc_wait (cp_parser *parser, cp_token * > > "#pragma acc wait", pragma_tok); > > > > stmt = c_finish_oacc_wait (loc, list, clauses); > > + stmt = finish_expr_stmt (stmt); > > > > return stmt; > > } Grüße Thomas
On 2015/12/3 4:59 PM, Thomas Schwinge wrote: > Hi! > > On Thu, 03 Dec 2015 09:51:31 +0100, I wrote: >> On Mon, 23 Nov 2015 21:15:00 +0800, Chung-Lin Tang <cltang@codesourcery.com> wrote: >>> The OpenACC wait directive is represented as a call to the runtime >>> function "GOACC_wait" instead of a tree code. I am seeing when >>> '#pragma acc wait' is using inside a template function, the CALL_EXPR >>> to GOACC_wait is being silently ignored/removed during tsubst_expr(). >> >> Uh. >> >>> I think the correct way to organize this is that the call should be inside >>> an EXPR_STMT, so here's a patch to do that; basically remove the >>> add_stmt() call from the shared c_finish_oacc_wait() code, and add >>> add_stmt()/finish_expr_stmt() in the corresponding C/C++ parts. >>> >>> Tested with no regressions on trunk, okay to commit? >> >>> --- c-family/c-omp.c (revision 230703) >>> +++ c-family/c-omp.c (working copy) >>> @@ -63,7 +63,6 @@ c_finish_oacc_wait (location_t loc, tree parms, tr >>> } >>> >>> stmt = build_call_expr_loc_vec (loc, stmt, args); >>> - add_stmt (stmt); >>> >>> vec_free (args); >> | >> | return stmt; >> | } >> >> I see in gcc/c/c-omp.c that several other c_finish_omp_* functions that >> build builtin calls instead of tree nodes, do similar things like >> c_finish_oacc_wait; I'd like to understand why it's -- presumably -- not >> a problem for these: c_finish_omp_barrier, c_finish_omp_taskwait, >> c_finish_omp_taskyield, c_finish_omp_flush? (Jakub?) > > Oh wait, it looks like the C++ front end is not actually using the > functions defined in the C/C++-shared gcc/c-family/c-omp.c, but has its > own implementations in gcc/cp/semantics.c, without "c_" prefixes? In > addition to finish_expr_stmt calls, I see it's also using > finish_call_expr instead of build_call_expr_loc/build_call_expr_loc_vec. > So I guess we'll want to model this the same way for OpenACC support > functions, and then (later) we should clean this up, to move the > C-specific code from gcc/c-family/c-omp.c into the C front end? (Jakub?) I see most OpenACC/OpenMP constructs are represented by special statement codes, so they should be a different case. I so far only see the OpenACC wait directive being represented as a CALL_EXPR (maybe there are others, haven't exhaustively searched). Chung-Lin
On Thu, Dec 03, 2015 at 06:05:36PM +0800, Chung-Lin Tang wrote: > > Oh wait, it looks like the C++ front end is not actually using the > > functions defined in the C/C++-shared gcc/c-family/c-omp.c, but has its > > own implementations in gcc/cp/semantics.c, without "c_" prefixes? In > > addition to finish_expr_stmt calls, I see it's also using > > finish_call_expr instead of build_call_expr_loc/build_call_expr_loc_vec. > > So I guess we'll want to model this the same way for OpenACC support > > functions, and then (later) we should clean this up, to move the > > C-specific code from gcc/c-family/c-omp.c into the C front end? (Jakub?) > > I see most OpenACC/OpenMP constructs are represented by special statement codes, > so they should be a different case. I so far only see the OpenACC wait directive > being represented as a CALL_EXPR (maybe there are others, haven't exhaustively searched). No, Thomas is right, just look at finish_omp_{barrier,flush,taskwait,taskyield,cancel,cancellation_point}, all those are represented as CALL_EXPRs. Jakub
On 2015/12/3 6:11 PM, Jakub Jelinek wrote: > On Thu, Dec 03, 2015 at 06:05:36PM +0800, Chung-Lin Tang wrote: >>> Oh wait, it looks like the C++ front end is not actually using the >>> functions defined in the C/C++-shared gcc/c-family/c-omp.c, but has its >>> own implementations in gcc/cp/semantics.c, without "c_" prefixes? In >>> addition to finish_expr_stmt calls, I see it's also using >>> finish_call_expr instead of build_call_expr_loc/build_call_expr_loc_vec. >>> So I guess we'll want to model this the same way for OpenACC support >>> functions, and then (later) we should clean this up, to move the >>> C-specific code from gcc/c-family/c-omp.c into the C front end? (Jakub?) >> >> I see most OpenACC/OpenMP constructs are represented by special statement codes, >> so they should be a different case. I so far only see the OpenACC wait directive >> being represented as a CALL_EXPR (maybe there are others, haven't exhaustively searched). > > No, Thomas is right, just look at > finish_omp_{barrier,flush,taskwait,taskyield,cancel,cancellation_point}, > all those are represented as CALL_EXPRs. > > Jakub > Okay, I guess my impression was only for some OpenACC constructs. Overall, OpenACC wait seems one of the few cases of using c_finish_* in cp/parser.c. Whether other cases should move towards/away from that kind of style is a larger question, I was only trying to fix a libgomp.oacc-c++/template-reduction.C regression (testcase currently still in gomp4 branch) Chung-Lin
On 2015/12/3 06:32 PM, Chung-Lin Tang wrote: > On 2015/12/3 6:11 PM, Jakub Jelinek wrote: >> On Thu, Dec 03, 2015 at 06:05:36PM +0800, Chung-Lin Tang wrote: >>>> Oh wait, it looks like the C++ front end is not actually using the >>>> functions defined in the C/C++-shared gcc/c-family/c-omp.c, but has its >>>> own implementations in gcc/cp/semantics.c, without "c_" prefixes? In >>>> addition to finish_expr_stmt calls, I see it's also using >>>> finish_call_expr instead of build_call_expr_loc/build_call_expr_loc_vec. >>>> So I guess we'll want to model this the same way for OpenACC support >>>> functions, and then (later) we should clean this up, to move the >>>> C-specific code from gcc/c-family/c-omp.c into the C front end? (Jakub?) >>> >>> I see most OpenACC/OpenMP constructs are represented by special statement codes, >>> so they should be a different case. I so far only see the OpenACC wait directive >>> being represented as a CALL_EXPR (maybe there are others, haven't exhaustively searched). >> >> No, Thomas is right, just look at >> finish_omp_{barrier,flush,taskwait,taskyield,cancel,cancellation_point}, >> all those are represented as CALL_EXPRs. >> >> Jakub >> > > Okay, I guess my impression was only for some OpenACC constructs. > > Overall, OpenACC wait seems one of the few cases of using c_finish_* in cp/parser.c. > Whether other cases should move towards/away from that kind of style is a larger question, > I was only trying to fix a libgomp.oacc-c++/template-reduction.C regression (testcase currently still in gomp4 branch) > > Chung-Lin > Per our internal discussion, I will commit this patch first to the gomp4 branch, while awaiting trunk approval. Thanks, Chung-Lin
OK. Jason
Index: c-family/c-omp.c =================================================================== --- c-family/c-omp.c (revision 230703) +++ c-family/c-omp.c (working copy) @@ -63,7 +63,6 @@ c_finish_oacc_wait (location_t loc, tree parms, tr } stmt = build_call_expr_loc_vec (loc, stmt, args); - add_stmt (stmt); vec_free (args); Index: c/c-parser.c =================================================================== --- c/c-parser.c (revision 230703) +++ c/c-parser.c (working copy) @@ -13886,6 +13886,7 @@ c_parser_oacc_wait (location_t loc, c_parser *pars strcpy (p_name, " wait"); clauses = c_parser_oacc_all_clauses (parser, OACC_WAIT_CLAUSE_MASK, p_name); stmt = c_finish_oacc_wait (loc, list, clauses); + add_stmt (stmt); return stmt; } Index: cp/parser.c =================================================================== --- cp/parser.c (revision 230703) +++ cp/parser.c (working copy) @@ -34930,6 +34930,7 @@ cp_parser_oacc_wait (cp_parser *parser, cp_token * "#pragma acc wait", pragma_tok); stmt = c_finish_oacc_wait (loc, list, clauses); + stmt = finish_expr_stmt (stmt); return stmt; }