public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Handle "omp simd array" accesses in ifcvt_memrefs_wont_trap
@ 2019-05-31  9:43 Jakub Jelinek
  2019-05-31  9:44 ` Richard Biener
  0 siblings, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2019-05-31  9:43 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches

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.

--- 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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Handle "omp simd array" accesses in ifcvt_memrefs_wont_trap
  2019-05-31  9:43 [PATCH] Handle "omp simd array" accesses in ifcvt_memrefs_wont_trap Jakub Jelinek
@ 2019-05-31  9:44 ` Richard Biener
  2019-05-31 10:35   ` Jakub Jelinek
  0 siblings, 1 reply; 4+ messages in thread
From: Richard Biener @ 2019-05-31  9:44 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

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

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
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG NÌrnberg)

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Handle "omp simd array" accesses in ifcvt_memrefs_wont_trap
  2019-05-31  9:44 ` Richard Biener
@ 2019-05-31 10:35   ` Jakub Jelinek
  2019-05-31 11:32     ` Richard Biener
  0 siblings, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2019-05-31 10:35 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Handle "omp simd array" accesses in ifcvt_memrefs_wont_trap
  2019-05-31 10:35   ` Jakub Jelinek
@ 2019-05-31 11:32     ` Richard Biener
  0 siblings, 0 replies; 4+ messages in thread
From: Richard Biener @ 2019-05-31 11:32 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

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

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
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG NÌrnberg)

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-05-31 10:53 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-31  9:43 [PATCH] Handle "omp simd array" accesses in ifcvt_memrefs_wont_trap Jakub Jelinek
2019-05-31  9:44 ` Richard Biener
2019-05-31 10:35   ` Jakub Jelinek
2019-05-31 11:32     ` Richard Biener

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).