OpenMP: Fix var replacement with 'simd' and linear-step vars [PR106548]

Message ID 52902f75-33c1-49c3-8e21-e6c6d1808a15@codesourcery.com
State New, archived
Headers
Series OpenMP: Fix var replacement with 'simd' and linear-step vars [PR106548] |

Commit Message

Tobias Burnus Aug. 16, 2022, 3:28 p.m. UTC
  The testcase is just a copy of linear-1 with 'omp ... for' replaced by 'omp ... for simd',
matching what the PR report referred to.

The problem occurs for 'omp ... for simd linear( i : step)' when 'step' is a variable
when a omp_fn... is generated - as in this case, the original variable is used (in the
reduced example of the PR, the PARM_DECL of 'f') instead of the replacement.

OK for mainline? Thoughts on backporting (and for which versions)?

Tobias
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
  

Comments

Jakub Jelinek Aug. 17, 2022, 12:38 p.m. UTC | #1
On Tue, Aug 16, 2022 at 05:28:40PM +0200, Tobias Burnus wrote:
> The testcase is just a copy of linear-1 with 'omp ... for' replaced by 'omp ... for simd',
> matching what the PR report referred to.
> 
> The problem occurs for 'omp ... for simd linear( i : step)' when 'step' is a variable
> when a omp_fn... is generated - as in this case, the original variable is used (in the
> reduced example of the PR, the PARM_DECL of 'f') instead of the replacement.
> 
> OK for mainline? Thoughts on backporting (and for which versions)?
> 
> Tobias
> -----------------
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

> OpenMP: Fix var replacement with 'simd' and linear-step vars [PR106548]
> 
> gcc/ChangeLog:
> 
> 	PR middle-end/106548
> 	* omp-low.cc (lower_rec_input_clauses): Use build_outer_var_ref
> 	for 'simd' linear-step values that are variable.
> 
> libgomp/ChangeLog:
> 
> 	PR middle-end/106548
> 	* testsuite/libgomp.c/linear-2.c: New test.
> 
> diff --git a/gcc/omp-low.cc b/gcc/omp-low.cc
> index 3c4b8593c8b..d6d6ff372a1 100644
> --- a/gcc/omp-low.cc
> +++ b/gcc/omp-low.cc
> @@ -6188,6 +6188,10 @@ lower_rec_input_clauses (tree clauses, gimple_seq *ilist, gimple_seq *dlist,
>  		      && gimple_omp_for_combined_into_p (ctx->stmt))
>  		    {
>  		      tree t = OMP_CLAUSE_LINEAR_STEP (c);
> +		      if (VAR_P (t)
> +			  || TREE_CODE (t) == PARM_DECL
> +			  || TREE_CODE (t) == RESULT_DECL)
> +			t = build_outer_var_ref (t, ctx);

I think this should be just
		      if (DECL_P (t))
			t = build_outer_var_ref (t, ctx);

Ok with that change.
With backports I'd wait a week or two (12.2 is frozen anyway), can be
applied to any open release branches you are willing to backport it to.

> --- /dev/null
> +++ b/libgomp/testsuite/libgomp.c/linear-2.c
> @@ -0,0 +1,251 @@
> +/* PR middle-end/106548.  */

Usually simd related runtime tests in libgomp use something like
/* { dg-do run } */
/* { dg-additional-options "-msse2" { target sse2_runtime } } */
/* { dg-additional-options "-mavx" { target avx_runtime } } */
to test some actual vectorization if possible.

	Jakub
  

Patch

OpenMP: Fix var replacement with 'simd' and linear-step vars [PR106548]

gcc/ChangeLog:

	PR middle-end/106548
	* omp-low.cc (lower_rec_input_clauses): Use build_outer_var_ref
	for 'simd' linear-step values that are variable.

libgomp/ChangeLog:

	PR middle-end/106548
	* testsuite/libgomp.c/linear-2.c: New test.

diff --git a/gcc/omp-low.cc b/gcc/omp-low.cc
index 3c4b8593c8b..d6d6ff372a1 100644
--- a/gcc/omp-low.cc
+++ b/gcc/omp-low.cc
@@ -6188,6 +6188,10 @@  lower_rec_input_clauses (tree clauses, gimple_seq *ilist, gimple_seq *dlist,
 		      && gimple_omp_for_combined_into_p (ctx->stmt))
 		    {
 		      tree t = OMP_CLAUSE_LINEAR_STEP (c);
+		      if (VAR_P (t)
+			  || TREE_CODE (t) == PARM_DECL
+			  || TREE_CODE (t) == RESULT_DECL)
+			t = build_outer_var_ref (t, ctx);
 		      tree stept = TREE_TYPE (t);
 		      tree ct = omp_find_clause (clauses,
 						 OMP_CLAUSE__LOOPTEMP_);
diff --git a/libgomp/testsuite/libgomp.c/linear-2.c b/libgomp/testsuite/libgomp.c/linear-2.c
new file mode 100644
index 00000000000..fee6fbc276d
--- /dev/null
+++ b/libgomp/testsuite/libgomp.c/linear-2.c
@@ -0,0 +1,251 @@ 
+/* PR middle-end/106548.  */
+int a[256];
+
+__attribute__((noinline, noclone)) int
+f1 (int i)
+{
+  #pragma omp parallel for simd linear (i: 4)
+  for (int j = 16; j < 64; j++)
+    {
+      a[i] = j;
+      i += 4;
+    }
+  return i;
+}
+
+__attribute__((noinline, noclone)) short int
+f2 (short int i, char k)
+{
+  #pragma omp parallel for simd linear (i: k + 1)
+  for (long j = 16; j < 64; j++)
+    {
+      a[i] = j;
+      i += 4;
+    }
+  return i;
+}
+
+__attribute__((noinline, noclone)) long long int
+f3 (long long int i, long long int k)
+{
+  #pragma omp parallel for simd linear (i: k)
+  for (short j = 16; j < 64; j++)
+    {
+      a[i] = j;
+      i += 4;
+    }
+  return i;
+}
+
+__attribute__((noinline, noclone)) int
+f4 (int i)
+{
+  #pragma omp parallel for simd linear (i: 4) schedule(static, 3)
+  for (int j = 16; j < 64; j++)
+    {
+      a[i] = j;
+      i += 4;
+    }
+  return i;
+}
+
+__attribute__((noinline, noclone)) short int
+f5 (short int i, char k)
+{
+  #pragma omp parallel for simd linear (i: k + 1) schedule(static, 5)
+  for (long j = 16; j < 64; j++)
+    {
+      a[i] = j;
+      i += 4;
+    }
+  return i;
+}
+
+__attribute__((noinline, noclone)) long long int
+f6 (long long int i, long long int k)
+{
+  #pragma omp parallel for simd linear (i: k) schedule(static, 7)
+  for (short j = 16; j < 64; j++)
+    {
+      a[i] = j;
+      i += 4;
+    }
+  return i;
+}
+
+__attribute__((noinline, noclone)) int
+f7 (int i)
+{
+  #pragma omp parallel for simd linear (i: 4) schedule(dynamic, 3)
+  for (int j = 16; j < 64; j++)
+    {
+      a[i] = j;
+      i += 4;
+    }
+  return i;
+}
+
+__attribute__((noinline, noclone)) short int
+f8 (short int i, char k)
+{
+  #pragma omp parallel for simd linear (i: k + 1) schedule(dynamic, 5)
+  for (long j = 16; j < 64; j++)
+    {
+      a[i] = j;
+      i += 4;
+    }
+  return i;
+}
+
+__attribute__((noinline, noclone)) long long int
+f9 (long long int i, long long int k)
+{
+  #pragma omp parallel for simd linear (i: k) schedule(dynamic, 7)
+  for (short j = 16; j < 64; j++)
+    {
+      a[i] = j;
+      i += 4;
+    }
+  return i;
+}
+
+__attribute__((noinline, noclone)) int
+f10 (int i, long step)
+{
+  #pragma omp parallel for simd linear (i: 4)
+  for (int j = 16; j < 112; j += step)
+    {
+      a[i] = j / 2 + 8;
+      i += 4;
+    }
+  return i;
+}
+
+__attribute__((noinline, noclone)) short int
+f11 (short int i, char k, char step)
+{
+  #pragma omp parallel for simd linear (i: k + 1)
+  for (long j = 16; j < 112; j += step)
+    {
+      a[i] = j / 2 + 8;
+      i += 4;
+    }
+  return i;
+}
+
+__attribute__((noinline, noclone)) long long int
+f12 (long long int i, long long int k, int step)
+{
+  #pragma omp parallel for simd linear (i: k)
+  for (short j = 16; j < 112; j += step)
+    {
+      a[i] = j / 2 + 8;
+      i += 4;
+    }
+  return i;
+}
+
+__attribute__((noinline, noclone)) int
+f13 (int i, long long int step)
+{
+  #pragma omp parallel for simd linear (i: 4) schedule(static, 3)
+  for (int j = 16; j < 112; j += step)
+    {
+      a[i] = j / 2 + 8;
+      i += 4;
+    }
+  return i;
+}
+
+__attribute__((noinline, noclone)) short int
+f14 (short int i, char k, int step)
+{
+  #pragma omp parallel for simd linear (i: k + 1) schedule(static, 5)
+  for (long j = 16; j < 112; j += step)
+    {
+      a[i] = j / 2 + 8;
+      i += 4;
+    }
+  return i;
+}
+
+__attribute__((noinline, noclone)) long long int
+f15 (long long int i, long long int k, long int step)
+{
+  #pragma omp parallel for simd linear (i: k) schedule(static, 7)
+  for (short j = 16; j < 112; j += step)
+    {
+      a[i] = j / 2 + 8;
+      i += 4;
+    }
+  return i;
+}
+
+__attribute__((noinline, noclone)) int
+f16 (int i, long long int step)
+{
+  #pragma omp parallel for simd linear (i: 4) schedule(dynamic, 3)
+  for (int j = 16; j < 112; j += step)
+    {
+      a[i] = j / 2 + 8;
+      i += 4;
+    }
+  return i;
+}
+
+__attribute__((noinline, noclone)) short int
+f17 (short int i, char k, int step)
+{
+  #pragma omp parallel for simd linear (i: k + 1) schedule(dynamic, 5)
+  for (long j = 16; j < 112; j += step)
+    {
+      a[i] = j / 2 + 8;
+      i += 4;
+    }
+  return i;
+}
+
+__attribute__((noinline, noclone)) long long int
+f18 (long long int i, long long int k, long int step)
+{
+  #pragma omp parallel for simd linear (i: k) schedule(dynamic, 7)
+  for (short j = 16; j < 112; j += step)
+    {
+      a[i] = j / 2 + 8;
+      i += 4;
+    }
+  return i;
+}
+
+int
+main ()
+{
+#define TEST(x) \
+  if (x != 8 + 48 * 4)				\
+    __builtin_abort ();				\
+  for (int i = 0; i < 256; i++)			\
+    if (a[i] != (((i & 3) == 0 && i >= 8	\
+		  && i < 8 + 48 * 4)		\
+		 ? ((i - 8) / 4) + 16 : 0))	\
+      __builtin_abort ();			\
+  __builtin_memset (a, 0, sizeof (a))
+  TEST (f1 (8));
+  TEST (f2 (8, 3));
+  TEST (f3 (8LL, 4LL));
+  TEST (f4 (8));
+  TEST (f5 (8, 3));
+  TEST (f6 (8LL, 4LL));
+  TEST (f7 (8));
+  TEST (f8 (8, 3));
+  TEST (f9 (8LL, 4LL));
+  TEST (f10 (8, 2));
+  TEST (f11 (8, 3, 2));
+  TEST (f12 (8LL, 4LL, 2));
+  TEST (f13 (8, 2));
+  TEST (f14 (8, 3, 2));
+  TEST (f15 (8LL, 4LL, 2));
+  TEST (f16 (8, 2));
+  TEST (f17 (8, 3, 2));
+  TEST (f18 (8LL, 4LL, 2));
+  return 0;
+}