Hi!
On Mon, 25 Sep 2017 16:57:52 +0200, Tom de Vries <[email protected]> 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 would print the actual "GOMP_DIM_GANG" instead of "gang" etc.,
to make it easier to see where these values are coming from.
(I do see that for "CODE", we can easily use a suitable "DEF" macro with
the "IFN_GOACC_REDUCTION_CODES" define -- not currently possible with the
"GOMP_DIM_*" constants. That can of course be addressed later,
separately, if a Reviewer agrees to the proposed patch generally.)
> OK for trunk, if testing is ok?
> --- a/gcc/gimple-pretty-print.c
> +++ b/gcc/gimple-pretty-print.c
> @@ -765,6 +766,40 @@ 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))
> + {
Will need to add a "default" case:
[...]/source-gcc/gcc/gimple-pretty-print.c:771:9: warning: enumeration
value 'IFN_MASK_LOAD' not handled in switch [-Wswitch]
switch (gimple_call_internal_fn (gs))
^
[followed by many more]
> + 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 (); \
^~~~~~~~
source-gcc/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-1.c:55:3:
note: in expansion of macro 'check_reduction_op'
check_reduction_op (int, ^, 0, array[i], num_gangs (ng) num_workers (nw)
^~~~~~~~~~~~~~~~~~
(gdb) bt
#0 fancy_abort (file=0x1659d70 "[...]/source-gcc/gcc/tree.c", line=6606,
function=0x165e7f8 <tree_to_uhwi(tree_node const*)::__FUNCTION__>
"tree_to_uhwi") at [...]/source-gcc/gcc/diagnostic.c:1487
#1 0x0000000000edf053 in tree_to_uhwi (t=<optimized out>) at
[...]/source-gcc/gcc/tree.c:6606
#2 0x000000000096ca63 in dump_gimple_call_args (buffer=0x7fffffffc6c0,
gs=0x7ffff66ad210, flags=0) at [...]/source-gcc/gcc/gimple-pretty-print.c:777
#3 0x000000000096f7f0 in dump_gimple_call (buffer=0x7fffffffc6c0,
gs=0x7ffff66ad210, spc=20, flags=0) at
[...]/source-gcc/gcc/gimple-pretty-print.c:946
[...]
This seems to be the "LEVEL" being "-1" -- probably meaning "not yet
decided"? (Haven't looked that up.)
> + {
> + case GOMP_DIM_GANG:
> + pp_string (buffer, " /*gang*/");
> + break;
> + case GOMP_DIM_WORKER:
> + pp_string (buffer, " /*worker*/");
> + break;
> + case GOMP_DIM_VECTOR:
> + pp_string (buffer, " /*vector*/");
> + break;
> + default:
> + gcc_unreachable ();
> + }
> + break;
> + case 4:
> + {
> + enum tree_code rcode
> + = (enum tree_code)tree_to_uhwi (gimple_call_arg (gs, i));
> + pp_string (buffer, " /*");
> + pp_string (buffer, op_symbol_code (rcode));
> + pp_string (buffer, "*/");
> + }
> + break;
I take it, there is no canned function for printing the textual
representation of the tree_code "OP" ("case 4").
> + }
> + }
> }
>
> if (gimple_call_va_arg_pack_p (gs))
Grüße
Thomas
