Re: [PATCH] Pretty-print GOACC_REDUCTION arguments

[Tom de Vries <Tom_deVries@mentor.com>]

[-- Attachment #1: Type: text/plain, Size: 2738 bytes --]

On 09/27/2017 03:46 PM, Thomas Schwinge wrote:
> Hi!
> 
> On Mon, 25 Sep 2017 16:57:52 +0200, Tom de Vries <Tom_deVries@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.

> I would print the actual "GOMP_DIM_GANG" instead of "gang" etc.,
> to make it easier to see where these values are coming from.
> 

Done.

>> OK for trunk, if testing is ok?


>> +	  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.)
> 

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.

Bootstrapped and reg-tested on x86_64.

OK for trunk?

Thanks,
- Tom

[-- Attachment #2: 0001-Pretty-print-GOACC_REDUCTION-arguments.patch --]
[-- Type: text/x-patch, Size: 2075 bytes --]

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  <tom@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))