Hi Tom! On Tue, 3 Oct 2017 10:56:59 +0200, Tom de Vries <tom_devr...@mentor.com> wrote: > On 09/27/2017 03:46 PM, Thomas Schwinge wrote: > > On Mon, 25 Sep 2017 16:57:52 +0200, Tom de Vries <tom_devr...@mentor.com> > > wrote: > >> currently for a GOACC_REDUCTION internal fn call we print: > >> ... > >> sum_5 = GOACC_REDUCTION (SETUP, _3, 0, 0, 67, 0); > >> ... > >> > >> This patch adds a comment for some arguments explaining the meaning of > >> the argument: > >> ... > >> sum_5 = GOACC_REDUCTION (SETUP, _3, 0, 0 /*gang*/, 67 /*+*/, 0); > >> ... > > > > ACK to the general idea. > > > > However, I note that for the "CODE" we currently *only* print the textual > > variant ("SETUP") (just like we're also doing for a few other "IFN_*"), > > whereas for "LEVEL" and "OP" you now print both. Should these really be > > different? I think I actually do prefer the style you're using (print > > both). > > I chose the c-like syntax to make it easier to copy gimple to test-case > ( based on comment > https://gcc.gnu.org/ml/gcc-patches/2015-09/msg02180.html ). But > reflecting on it a bit more, I realized that it's an internal function > and as such won't work anyway in test-cases, so I'm not sure how > relevant it is in this case.
The way you're doing it still makes sense to me: we might (at some later point...) read such dumps back in, using the GIMPLE front end or similar. (Clearly nothing to worry about right now.) > >> + case IFN_GOACC_REDUCTION: > >> + switch (i) > >> + { > >> + case 3: > >> + switch (tree_to_uhwi (gimple_call_arg (gs, i))) > > > > Something ;-) is wrong. Running this on > > libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-1.c, I run into: > > > > during GIMPLE pass: omplower > > dump file: reduction-1.c.006t.omplower > > > > In file included from > > source-gcc/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-1.c:9:0: > > source-gcc/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-1.c: > > In function 'test_reductions': > > > > source-gcc/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction.h:20:7: > > internal compiler error: in tree_to_uhwi, at tree.c:6606 > > abort (); \ > > > This seems to be the "LEVEL" being "-1" -- probably meaning "not yet > > decided"? (Haven't looked that up.) As far as I can tell, initially all these "LEVEL" arguments are set to "-1". See the "place" argument in the "lower_oacc_reductions" call in "lower_oacc_head_tail". So, I understand this to mean "not yet decided", from here on, until some real level of parallelism gets set. > In execute_oacc_device_lower we find: > ... > case IFN_GOACC_REDUCTION: > /* Mark the function for SSA renaming. */ > mark_virtual_operands_for_renaming (cfun); > > /* If the level is -1, this ended up being an unused > > axis. Handle as a default. */ > if (integer_minus_onep (gimple_call_arg (call, 3))) > default_goacc_reduction (call); > else > targetm.goacc.reduction (call); > ... > > I'm printing it now as GOMP_DIM_NONE. But after this processing, there are no "IFN_GOACC_REDUCTION" anymore, so we should rather print something like "unknown" instead of "GOMP_DIM_NONE" (which doesn't exist anyway). With that changed, we're good as far as I'm concerned, thanks! But I can't formally approve, of course. Reviewed-by: Thomas Schwinge <tho...@codesourcery.com> (See <http://mid.mail-archive.com/87tvzuk29t.fsf@euler.schwinge.homeip.net>.) Grüße Thomas > Bootstrapped and reg-tested on x86_64. > > OK for trunk? > > Thanks, > - Tom > Pretty-print GOACC_REDUCTION arguments > > Prints > sum_5 = GOACC_REDUCTION (SETUP, _3, 0, 0 /*GOMP_DIM_GANG*/, 67 /*+*/, 0); > instead of > sum_5 = GOACC_REDUCTION (SETUP, _3, 0, 0, 67, 0); > > 2017-09-25 Tom de Vries <t...@codesourcery.com> > > * gimple-pretty-print.c (dump_gimple_call_args): Pretty-print > GOACC_REDUCTION arguments. > > --- > gcc/gimple-pretty-print.c | 42 ++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 42 insertions(+) > > diff --git a/gcc/gimple-pretty-print.c b/gcc/gimple-pretty-print.c > index ed8e51c..33ca45d 100644 > --- a/gcc/gimple-pretty-print.c > +++ b/gcc/gimple-pretty-print.c > @@ -41,6 +41,7 @@ along with GCC; see the file COPYING3. If not see > #include "stringpool.h" > #include "attribs.h" > #include "asan.h" > +#include "gomp-constants.h" > > #define INDENT(SPACE) > \ > do { int i; for (i = 0; i < SPACE; i++) pp_space (buffer); } while (0) > @@ -765,6 +766,47 @@ dump_gimple_call_args (pretty_printer *buffer, gcall > *gs, dump_flags_t flags) > if (i) > pp_string (buffer, ", "); > dump_generic_node (buffer, gimple_call_arg (gs, i), 0, flags, false); > + > + if (gimple_call_internal_p (gs)) > + switch (gimple_call_internal_fn (gs)) > + { > + case IFN_GOACC_REDUCTION: > + switch (i) > + { > + case 3: > + switch (tree_to_shwi (gimple_call_arg (gs, i))) > + { > + case GOMP_DIM_GANG: > + pp_string (buffer, " /*GOMP_DIM_GANG*/"); > + break; > + case GOMP_DIM_WORKER: > + pp_string (buffer, " /*GOMP_DIM_WORKER*/"); > + break; > + case GOMP_DIM_VECTOR: > + pp_string (buffer, " /*GOMP_DIM_VECTOR*/"); > + break; > + case -1: > + pp_string (buffer, " /*GOMP_DIM_NONE*/"); > + break; > + default: > + gcc_unreachable (); > + } > + break; > + case 4: > + { > + enum tree_code rcode > + = (enum tree_code)tree_to_shwi (gimple_call_arg (gs, i)); > + pp_string (buffer, " /*"); > + pp_string (buffer, op_symbol_code (rcode)); > + pp_string (buffer, "*/"); > + } > + break; > + default: > + break; > + } > + default: > + break; > + } > } > > if (gimple_call_va_arg_pack_p (gs))