diff mbox series

Handle "omp simd array" accesses in ifcvt_memrefs_wont_trap

Message ID 20190531093323.GF19695@tucnak
State New
Headers show
Series Handle "omp simd array" accesses in ifcvt_memrefs_wont_trap | expand

Commit Message

Jakub Jelinek May 31, 2019, 9:33 a.m. UTC 
Hi!

As a preparation for lastprivate(conditional:) on #pragma omp simd, I need
if-conversion to handle "omp simd array" accesses.  These are safe, no
matter whether written or read, each simd lane accesses their own element,
after the vectorization it is all just a single vector read or store or RMW
cycle, it will never trap (it is an automatic variable), it will never be
out of bounds (the index is guaranteed to be within the bounds of the array,
as the array is sized to the vectorization factor and index is the simd lane
number within that vectorization factor).

Tested on x86_64-linux with vect.exp, ok for trunk if it passes full
bootstrap/regtest?

2019-05-31  Jakub Jelinek  <jakub@redhat.com>

	* tree-if-conv.c: Include attribs.h.
	(ifcvt_memrefs_wont_trap): Return true for ARRAY_REF access to
	"omp simd array" variable with .GOMP_SIMD_LANE as index.

	* gcc.dg/vect/vect-cond-12.c: New test.


	Jakub

Comments

Richard Biener May 31, 2019, 9:43 a.m. UTC | #1 
On Fri, 31 May 2019, Jakub Jelinek wrote:

> Hi!
> 
> As a preparation for lastprivate(conditional:) on #pragma omp simd, I need
> if-conversion to handle "omp simd array" accesses.  These are safe, no
> matter whether written or read, each simd lane accesses their own element,
> after the vectorization it is all just a single vector read or store or RMW
> cycle, it will never trap (it is an automatic variable), it will never be
> out of bounds (the index is guaranteed to be within the bounds of the array,
> as the array is sized to the vectorization factor and index is the simd lane
> number within that vectorization factor).
> 
> Tested on x86_64-linux with vect.exp, ok for trunk if it passes full
> bootstrap/regtest?

Hmm.  Is it enough to set TREE_THIS_NOTRAP on the ARRAY_REF instead?
At least it's documented the ARRAY_REF then isn't the issue.
As for conditional vs. unconditional I'm not so sure but automatic
vars are excepmted from that anyways?

Richard.

> 2019-05-31  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* tree-if-conv.c: Include attribs.h.
> 	(ifcvt_memrefs_wont_trap): Return true for ARRAY_REF access to
> 	"omp simd array" variable with .GOMP_SIMD_LANE as index.
> 
> 	* gcc.dg/vect/vect-cond-12.c: New test.
> 
> --- gcc/tree-if-conv.c.jj	2019-05-14 21:37:32.932400472 +0200
> +++ gcc/tree-if-conv.c	2019-05-31 11:13:13.558732900 +0200
> @@ -120,6 +120,7 @@ along with GCC; see the file COPYING3.
>  #include "fold-const.h"
>  #include "tree-ssa-sccvn.h"
>  #include "tree-cfgcleanup.h"
> +#include "attribs.h"
>  
>  /* Only handle PHIs with no more arguments unless we are asked to by
>     simd pragma.  */
> @@ -897,6 +898,32 @@ ifcvt_memrefs_wont_trap (gimple *stmt, v
>    if (DR_W_UNCONDITIONALLY (*master_dr))
>      return true;
>  
> +  /* OpenMP simd lane accesses to omp simd array variables are always
> +     within bounds and can be handled as if it was unconditional.  */
> +  tree base_var = get_base_address (base);
> +  if (VAR_P (base_var)
> +      && lookup_attribute ("omp simd array", DECL_ATTRIBUTES (base_var)))
> +    {
> +      tree ref = DR_REF (a);
> +      struct loop *loop = loop_containing_stmt (stmt);
> +      if (loop->simduid
> +	  && TREE_CODE (ref) == ARRAY_REF
> +	  && TREE_CODE (TREE_OPERAND (ref, 1)) == SSA_NAME)
> +	{
> +	  gimple *def = SSA_NAME_DEF_STMT (TREE_OPERAND (ref, 1));
> +	  if (is_gimple_call (def)
> +	      && gimple_call_internal_p (def)
> +	      && (gimple_call_internal_fn (def) == IFN_GOMP_SIMD_LANE))
> +	    {
> +	      tree arg = gimple_call_arg (def, 0);
> +	      gcc_assert (TREE_CODE (arg) == SSA_NAME);
> +	      arg = SSA_NAME_VAR (arg);
> +	      if (arg == loop->simduid)
> +		return true;
> +	    }
> +	}
> +    }
> +
>    /* If a is unconditionally accessed then ...
>  
>       Even a is conditional access, we can treat it as an unconditional
> --- gcc/testsuite/gcc.dg/vect/vect-cond-12.c.jj	2019-05-31 11:25:33.203577504 +0200
> +++ gcc/testsuite/gcc.dg/vect/vect-cond-12.c	2019-05-31 11:26:58.616174115 +0200
> @@ -0,0 +1,14 @@
> +/* { dg-do compile } */
> +/* { dg-additional-options "-fopenmp-simd" } */
> +/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" { target vect_condition } } } */
> +
> +int x;
> +
> +void
> +foo (int *a)
> +{
> +  #pragma omp simd lastprivate (x)
> +  for (int i = 0; i < 1024; ++i)
> +    if (a[i])
> +      x = i;
> +}
> 
> 	Jakub
>
Jakub Jelinek May 31, 2019, 9:57 a.m. UTC | #2 
On Fri, May 31, 2019 at 11:43:29AM +0200, Richard Biener wrote:
> Hmm.  Is it enough to set TREE_THIS_NOTRAP on the ARRAY_REF instead?

That works too.

> At least it's documented the ARRAY_REF then isn't the issue.
> As for conditional vs. unconditional I'm not so sure but automatic
> vars are excepmted from that anyways?

Only if they are guaranteed not to have out of bound accesses.

Anyway, I'll go with this, those ARRAY_REFs indeed should never trap.

2019-05-31  Jakub Jelinek  <jakub@redhat.com>

	* omp-low.c (lower_rec_simd_input_clauses): Set TREE_THIS_NOTRAP on
	ivar and lvar.

	* gcc.dg/vect/vect-cond-12.c: New test.

--- gcc/omp-low.c.jj	2019-05-30 23:19:14.469931759 +0200
+++ gcc/omp-low.c	2019-05-31 11:52:20.491195088 +0200
@@ -3728,6 +3728,8 @@ lower_rec_simd_input_clauses (tree new_v
 		     NULL_TREE, NULL_TREE);
       lvar = build4 (ARRAY_REF, TREE_TYPE (new_var), avar, sctx->lane,
 		     NULL_TREE, NULL_TREE);
+      TREE_THIS_NOTRAP (ivar) = 1;
+      TREE_THIS_NOTRAP (lvar) = 1;
     }
   if (DECL_P (new_var))
     {
--- gcc/testsuite/gcc.dg/vect/vect-cond-12.c.jj	2019-05-31 11:25:33.203577504 +0200
+++ gcc/testsuite/gcc.dg/vect/vect-cond-12.c	2019-05-31 11:26:58.616174115 +0200
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-fopenmp-simd" } */
+/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" { target vect_condition } } } */
+
+int x;
+
+void
+foo (int *a)
+{
+  #pragma omp simd lastprivate (x)
+  for (int i = 0; i < 1024; ++i)
+    if (a[i])
+      x = i;
+}


	Jakub
Richard Biener May 31, 2019, 10:53 a.m. UTC | #3 
On Fri, 31 May 2019, Jakub Jelinek wrote:

> On Fri, May 31, 2019 at 11:43:29AM +0200, Richard Biener wrote:
> > Hmm.  Is it enough to set TREE_THIS_NOTRAP on the ARRAY_REF instead?
> 
> That works too.
> 
> > At least it's documented the ARRAY_REF then isn't the issue.
> > As for conditional vs. unconditional I'm not so sure but automatic
> > vars are excepmted from that anyways?
> 
> Only if they are guaranteed not to have out of bound accesses.
> 
> Anyway, I'll go with this, those ARRAY_REFs indeed should never trap.

Thanks - a lot simpler indeed ;)

Richard.

> 2019-05-31  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* omp-low.c (lower_rec_simd_input_clauses): Set TREE_THIS_NOTRAP on
> 	ivar and lvar.
> 
> 	* gcc.dg/vect/vect-cond-12.c: New test.
> 
> --- gcc/omp-low.c.jj	2019-05-30 23:19:14.469931759 +0200
> +++ gcc/omp-low.c	2019-05-31 11:52:20.491195088 +0200
> @@ -3728,6 +3728,8 @@ lower_rec_simd_input_clauses (tree new_v
>  		     NULL_TREE, NULL_TREE);
>        lvar = build4 (ARRAY_REF, TREE_TYPE (new_var), avar, sctx->lane,
>  		     NULL_TREE, NULL_TREE);
> +      TREE_THIS_NOTRAP (ivar) = 1;
> +      TREE_THIS_NOTRAP (lvar) = 1;
>      }
>    if (DECL_P (new_var))
>      {
> --- gcc/testsuite/gcc.dg/vect/vect-cond-12.c.jj	2019-05-31 11:25:33.203577504 +0200
> +++ gcc/testsuite/gcc.dg/vect/vect-cond-12.c	2019-05-31 11:26:58.616174115 +0200
> @@ -0,0 +1,14 @@
> +/* { dg-do compile } */
> +/* { dg-additional-options "-fopenmp-simd" } */
> +/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" { target vect_condition } } } */
> +
> +int x;
> +
> +void
> +foo (int *a)
> +{
> +  #pragma omp simd lastprivate (x)
> +  for (int i = 0; i < 1024; ++i)
> +    if (a[i])
> +      x = i;
> +}
> 
> 
> 	Jakub
>
diff mbox series

Patch

--- gcc/tree-if-conv.c.jj	2019-05-14 21:37:32.932400472 +0200
+++ gcc/tree-if-conv.c	2019-05-31 11:13:13.558732900 +0200
@@ -120,6 +120,7 @@  along with GCC; see the file COPYING3.
 #include "fold-const.h"
 #include "tree-ssa-sccvn.h"
 #include "tree-cfgcleanup.h"
+#include "attribs.h"
 
 /* Only handle PHIs with no more arguments unless we are asked to by
    simd pragma.  */
@@ -897,6 +898,32 @@  ifcvt_memrefs_wont_trap (gimple *stmt, v
   if (DR_W_UNCONDITIONALLY (*master_dr))
     return true;
 
+  /* OpenMP simd lane accesses to omp simd array variables are always
+     within bounds and can be handled as if it was unconditional.  */
+  tree base_var = get_base_address (base);
+  if (VAR_P (base_var)
+      && lookup_attribute ("omp simd array", DECL_ATTRIBUTES (base_var)))
+    {
+      tree ref = DR_REF (a);
+      struct loop *loop = loop_containing_stmt (stmt);
+      if (loop->simduid
+	  && TREE_CODE (ref) == ARRAY_REF
+	  && TREE_CODE (TREE_OPERAND (ref, 1)) == SSA_NAME)
+	{
+	  gimple *def = SSA_NAME_DEF_STMT (TREE_OPERAND (ref, 1));
+	  if (is_gimple_call (def)
+	      && gimple_call_internal_p (def)
+	      && (gimple_call_internal_fn (def) == IFN_GOMP_SIMD_LANE))
+	    {
+	      tree arg = gimple_call_arg (def, 0);
+	      gcc_assert (TREE_CODE (arg) == SSA_NAME);
+	      arg = SSA_NAME_VAR (arg);
+	      if (arg == loop->simduid)
+		return true;
+	    }
+	}
+    }
+
   /* If a is unconditionally accessed then ...
 
      Even a is conditional access, we can treat it as an unconditional
--- gcc/testsuite/gcc.dg/vect/vect-cond-12.c.jj	2019-05-31 11:25:33.203577504 +0200
+++ gcc/testsuite/gcc.dg/vect/vect-cond-12.c	2019-05-31 11:26:58.616174115 +0200
@@ -0,0 +1,14 @@ 
+/* { dg-do compile } */
+/* { dg-additional-options "-fopenmp-simd" } */
+/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" { target vect_condition } } } */
+
+int x;
+
+void
+foo (int *a)
+{
+  #pragma omp simd lastprivate (x)
+  for (int i = 0; i < 1024; ++i)
+    if (a[i])
+      x = i;
+}