diff mbox

[OpenACC] internal fn folding

Message ID 563B7A32.7010209@acm.org
State New
Headers show

Commit Message

Nathan Sidwell Nov. 5, 2015, 3:48 p.m. UTC
On 11/04/15 05:02, Bernd Schmidt wrote:
> On 11/02/2015 02:56 PM, Nathan Sidwell wrote:
>> On 10/28/15 14:40, Nathan Sidwell wrote:
>>> Richard,
>>> this patch adds folding for the new GOACC_DIM_POS and GOACC_DIM_SIZE
>>> internal
>>> functions.  IIUC gimple_fold_call is the right place to add this.
>>>
>>> The size of a compute dimension is very often a compile-time
>>> constant.  On the
>>> host, in particular it's 1, which means we can deduce the POS must be
>>> zero.
>>>
>>> ok?

This is what I committed, using the helpers I recently added. (I realized we can 
only get here for functions with the oacc attribute already set)

nathan

Comments

Thomas Schwinge Nov. 8, 2015, 2:04 p.m. UTC | #1
Hi!

On Thu, 5 Nov 2015 10:48:02 -0500, Nathan Sidwell <nathan@acm.org> wrote:
> On 11/04/15 05:02, Bernd Schmidt wrote:
> > On 11/02/2015 02:56 PM, Nathan Sidwell wrote:
> >> On 10/28/15 14:40, Nathan Sidwell wrote:
> >>> Richard,
> >>> this patch adds folding for the new GOACC_DIM_POS and GOACC_DIM_SIZE
> >>> internal
> >>> functions.  IIUC gimple_fold_call is the right place to add this.
> >>>
> >>> The size of a compute dimension is very often a compile-time
> >>> constant.  On the
> >>> host, in particular it's 1, which means we can deduce the POS must be
> >>> zero.

> This is what I committed, using the helpers I recently added. (I realized we can 
> only get here for functions with the oacc attribute already set)

> --- gimple-fold.c	(revision 229809)
> +++ gimple-fold.c	(working copy)

> +/* Transform IFN_GOACC_DIM_SIZE and IFN_GOACC_DIM_POS internal
> +   function calls to constants, where possible.  */
> +
> +static tree
> +fold_internal_goacc_dim (const gimple *call)
> +{
> +  int axis = get_oacc_ifn_dim_arg (call);
> +  int size = get_oacc_fn_dim_size (current_function_decl, axis);
> +  bool is_pos = gimple_call_internal_fn (call) == IFN_GOACC_DIM_POS;
> +  tree result = NULL_TREE;
> +
> +  /* If the size is 1, or we only want the size and it is not dynamic,
> +     we know the answer.  */
> +  if (size == 1 || (!is_pos && size))
> +    {
> +      tree type = TREE_TYPE (gimple_call_lhs (call));
> +      result = build_int_cst (type, size - is_pos);
> +    }
> +
> +  return result;
> +}

> @@ -3106,6 +3129,10 @@ gimple_fold_call (gimple_stmt_iterator *
>  	      return true;
>  	    }
>  	  break;
> +	case IFN_GOACC_DIM_SIZE:
> +	case IFN_GOACC_DIM_POS:
> +	  result = fold_internal_goacc_dim (stmt);
> +	  break;

Merging this into gomp-4_0-branch, we'd run into a lot of regressions
(for OpenACC kernels construct), for example:

    [...]/source-gcc/gcc/testsuite/c-c++-common/goacc/kernels-loop-g.c: In function 'main._omp_fn.0':
    [...]/source-gcc/gcc/testsuite/c-c++-common/goacc/kernels-loop-g.c:8:0: internal compiler error: Segmentation fault
    0xac387f crash_signal
            [...]/source-gcc/gcc/toplev.c:336
    0x9b8399 tree_int_cst_elt_check
            [...]/source-gcc/gcc/tree.h:3129
    0x9b8399 get_oacc_fn_dim_size(tree_node*, int)
            [...]/source-gcc/gcc/omp-low.c:12630
    0x86c530 fold_internal_goacc_dim
            [...]/source-gcc/gcc/gimple-fold.c:2917
    0x86c530 gimple_fold_call
            [...]/source-gcc/gcc/gimple-fold.c:3134
    0x86dfe4 fold_stmt_1
            [...]/source-gcc/gcc/gimple-fold.c:3702
    0xbf9953 execute
            [...]/source-gcc/gcc/tree-ssa-forwprop.c:2310

The dims in gcc/omp-low.c:get_oacc_fn_dim_size don't have values set, so
the "TREE_INT_CST_LOW (TREE_VALUE (dims))" fails.  I have not analyzed
what exactly is going wrong; I just figured out that it's related to the
IFN_GOACC_DIM_POS without LHS usage that Tom introduced in
gomp-4_0-branch r228735 for OpenACC kernels to "neuter gang-single code
in gang-redundant mode",
<http://news.gmane.org/find-root.php?message_id=%3C561C1336.2050401%40mentor.com%3E>.

So, in r229948 I merged Nathan's trunk r229816 into gomp-4_0-branch with
an additional hack as indicated ("++" prefix) by the following three-way
diff:

commit b421a6415fc223866bc97f8248a1fbd0a524505e
Merge: 7a6eb6b b0ccb4e
Author: tschwinge <tschwinge@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Sun Nov 8 13:49:46 2015 +0000

    svn merge -r 229814:229816 svn+ssh://gcc.gnu.org/svn/gcc/trunk
    
    
    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/branches/gomp-4_0-branch@229948 138bc75d-0d04-0410-961f-82ee72b054a4

 gcc/ChangeLog     |  6 ++++++
 gcc/gimple-fold.c | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+)

diff --cc gcc/gimple-fold.c
index c9b9593,45840af..869c6c2
--- gcc/gimple-fold.c
+++ gcc/gimple-fold.c
@@@ -2906,6 -2907,28 +2907,34 @@@ gimple_fold_builtin (gimple_stmt_iterat
    return false;
  }
  
+ /* Transform IFN_GOACC_DIM_SIZE and IFN_GOACC_DIM_POS internal
+    function calls to constants, where possible.  */
+ 
+ static tree
+ fold_internal_goacc_dim (const gimple *call)
+ {
++  /* TODO.  There is something going wrong here, for the gang_single
++     IFN_GOACC_DIM_POS without LHS, generated in gcc/omp-low.c:lower_omp_target
++     for is_oacc_kernels (see gomp-4_0-branch r228735).  */
++  if (gimple_call_lhs (call) == NULL_TREE)
++    return NULL_TREE;
++
+   int axis = get_oacc_ifn_dim_arg (call);
+   int size = get_oacc_fn_dim_size (current_function_decl, axis);
+   bool is_pos = gimple_call_internal_fn (call) == IFN_GOACC_DIM_POS;
+   tree result = NULL_TREE;
+ 
+   /* If the size is 1, or we only want the size and it is not dynamic,
+      we know the answer.  */
+   if (size == 1 || (!is_pos && size))
+     {
+       tree type = TREE_TYPE (gimple_call_lhs (call));
+       result = build_int_cst (type, size - is_pos);
+     }
+ 
+   return result;
+ }
+ 
  /* Return true if ARG0 CODE ARG1 in infinite signed precision operation
     doesn't fit into TYPE.  The test for overflow should be regardless of
     -fwrapv, and even for unsigned types.  */


Grüße
 Thomas
Tom de Vries Nov. 8, 2015, 3:43 p.m. UTC | #2
On 08/11/15 15:04, Thomas Schwinge wrote:
> Hi!
>
> On Thu, 5 Nov 2015 10:48:02 -0500, Nathan Sidwell<nathan@acm.org>  wrote:
>> >On 11/04/15 05:02, Bernd Schmidt wrote:
>>> > >On 11/02/2015 02:56 PM, Nathan Sidwell wrote:
>>>> > >>On 10/28/15 14:40, Nathan Sidwell wrote:
>>>>> > >>>Richard,
>>>>> > >>>this patch adds folding for the new GOACC_DIM_POS and GOACC_DIM_SIZE
>>>>> > >>>internal
>>>>> > >>>functions.  IIUC gimple_fold_call is the right place to add this.
>>>>> > >>>
>>>>> > >>>The size of a compute dimension is very often a compile-time
>>>>> > >>>constant.  On the
>>>>> > >>>host, in particular it's 1, which means we can deduce the POS must be
>>>>> > >>>zero.
>> >This is what I committed, using the helpers I recently added. (I realized we can
>> >only get here for functions with the oacc attribute already set)
>> >--- gimple-fold.c	(revision 229809)
>> >+++ gimple-fold.c	(working copy)
>> >+/* Transform IFN_GOACC_DIM_SIZE and IFN_GOACC_DIM_POS internal
>> >+   function calls to constants, where possible.  */
>> >+
>> >+static tree
>> >+fold_internal_goacc_dim (const gimple *call)
>> >+{
>> >+  int axis = get_oacc_ifn_dim_arg (call);
>> >+  int size = get_oacc_fn_dim_size (current_function_decl, axis);
>> >+  bool is_pos = gimple_call_internal_fn (call) == IFN_GOACC_DIM_POS;
>> >+  tree result = NULL_TREE;
>> >+
>> >+  /* If the size is 1, or we only want the size and it is not dynamic,
>> >+     we know the answer.  */
>> >+  if (size == 1 || (!is_pos && size))
>> >+    {
>> >+      tree type = TREE_TYPE (gimple_call_lhs (call));
>> >+      result = build_int_cst (type, size - is_pos);
>> >+    }
>> >+
>> >+  return result;
>> >+}
>> >@@ -3106,6 +3129,10 @@ gimple_fold_call (gimple_stmt_iterator *
>> >  	      return true;
>> >  	    }
>> >  	  break;
>> >+	case IFN_GOACC_DIM_SIZE:
>> >+	case IFN_GOACC_DIM_POS:
>> >+	  result = fold_internal_goacc_dim (stmt);
>> >+	  break;
> Merging this into gomp-4_0-branch, we'd run into a lot of regressions
> (for OpenACC kernels construct), for example:
>
>      [...]/source-gcc/gcc/testsuite/c-c++-common/goacc/kernels-loop-g.c: In function 'main._omp_fn.0':
>      [...]/source-gcc/gcc/testsuite/c-c++-common/goacc/kernels-loop-g.c:8:0: internal compiler error: Segmentation fault
>      0xac387f crash_signal
>              [...]/source-gcc/gcc/toplev.c:336
>      0x9b8399 tree_int_cst_elt_check
>              [...]/source-gcc/gcc/tree.h:3129
>      0x9b8399 get_oacc_fn_dim_size(tree_node*, int)
>              [...]/source-gcc/gcc/omp-low.c:12630
>      0x86c530 fold_internal_goacc_dim
>              [...]/source-gcc/gcc/gimple-fold.c:2917
>      0x86c530 gimple_fold_call
>              [...]/source-gcc/gcc/gimple-fold.c:3134
>      0x86dfe4 fold_stmt_1
>              [...]/source-gcc/gcc/gimple-fold.c:3702
>      0xbf9953 execute
>              [...]/source-gcc/gcc/tree-ssa-forwprop.c:2310
>
> The dims in gcc/omp-low.c:get_oacc_fn_dim_size don't have values set, so
> the "TREE_INT_CST_LOW (TREE_VALUE (dims))" fails.  I have not analyzed
> what exactly is going wrong; I just figured out that it's related to the
> IFN_GOACC_DIM_POS without LHS usage that Tom introduced in
> gomp-4_0-branch r228735 for OpenACC kernels to "neuter gang-single code
> in gang-redundant mode",
> <http://news.gmane.org/find-root.php?message_id=%3C561C1336.2050401%40mentor.com%3E>.

I've just removed that ( 
https://gcc.gnu.org/ml/gcc-patches/2015-11/msg00847.html ).

Thanks,
- Tom
diff mbox

Patch

2015-11-05  Nathan Sidwell  <nathan@codesourcery.com>

	* gimple-fold.c: Include omp-low.h.
	(fold_internal_goacc_dim): New.
	(gimple_fold_call): Call it.

Index: gimple-fold.c
===================================================================
--- gimple-fold.c	(revision 229809)
+++ gimple-fold.c	(working copy)
@@ -52,6 +52,7 @@  along with GCC; see the file COPYING3.
 #include "gimple-match.h"
 #include "gomp-constants.h"
 #include "optabs-query.h"
+#include "omp-low.h"
 
 
 /* Return true when DECL can be referenced from current unit.
@@ -2906,6 +2907,28 @@  gimple_fold_builtin (gimple_stmt_iterato
   return false;
 }
 
+/* Transform IFN_GOACC_DIM_SIZE and IFN_GOACC_DIM_POS internal
+   function calls to constants, where possible.  */
+
+static tree
+fold_internal_goacc_dim (const gimple *call)
+{
+  int axis = get_oacc_ifn_dim_arg (call);
+  int size = get_oacc_fn_dim_size (current_function_decl, axis);
+  bool is_pos = gimple_call_internal_fn (call) == IFN_GOACC_DIM_POS;
+  tree result = NULL_TREE;
+
+  /* If the size is 1, or we only want the size and it is not dynamic,
+     we know the answer.  */
+  if (size == 1 || (!is_pos && size))
+    {
+      tree type = TREE_TYPE (gimple_call_lhs (call));
+      result = build_int_cst (type, size - is_pos);
+    }
+
+  return result;
+}
+
 /* Return true if ARG0 CODE ARG1 in infinite signed precision operation
    doesn't fit into TYPE.  The test for overflow should be regardless of
    -fwrapv, and even for unsigned types.  */
@@ -3106,6 +3129,10 @@  gimple_fold_call (gimple_stmt_iterator *
 	      return true;
 	    }
 	  break;
+	case IFN_GOACC_DIM_SIZE:
+	case IFN_GOACC_DIM_POS:
+	  result = fold_internal_goacc_dim (stmt);
+	  break;
 	case IFN_UBSAN_CHECK_ADD:
 	  subcode = PLUS_EXPR;
 	  break;