Re: [PATCH 04/11] AArch64: Test OpenMP lastprivate clause for various constructs.

2024-09-13 Thread Tejas Belagod

On 8/6/24 4:59 PM, Jakub Jelinek wrote:

On Mon, May 27, 2024 at 10:36:19AM +0530, Tejas Belagod wrote:

This patch tests various OpenMP lastprivate clause with SVE object types in
various construct contexts.

gcc/testsuite/ChangeLog:

* gcc.target/aarch64/sve/omp/lastprivate.c: New test.


This is a dg-do compile test, so it could be in gcc.target/aarch64/sve/gomp/

Generally I'd suggest not to mix erroneous constructs with correct ones,
so that the correct ones can be tested up to assembly generation (or runtime
if needed), which might not be the case when there are errors in the same
testcase.
Furethermore, I think it would be useful to actually test the behavior
of lastprviate even for the constructs where you have error now, so
in lastprivate-1.c include say #pragma omp parallel sections lastprivate (va)
rather than just #pragma omp sections lastprivate (va) or the latter
if it is nested inside of #pragma omp parallel (and va is shared there).



Thanks for the reviews.


+/* This worksharing construct binds to an implicit outer parallel region in
+whose scope va is declared and therefore is default private.  This causes
+the lastprivate clause list item va to be diagnosed as private in the outer
+context.  Similarly for constructs for and distribute.  */
+#pragma omp sections lastprivate (va) /* { dg-error {lastprivate variable 'va' 
is private in outer context} } */
+{
+  #pragma omp section
+  vb = svld1_s32 (svptrue_b32 (), b);
+  #pragma omp section
+  vc = svld1_s32 (svptrue_b32 (), c);
+  #pragma omp section
+  va = svadd_s32_z (svptrue_b32 (), vb, vc);
+}


Note, unless you know this is included in implicit parallel region with a
single thread, this isn't really valid.  The sections directive is user
explicitly saying that there are no dependencies between the 3 sections
here, which is clearly not the case, the first two compute variables which
the third one uses.


So without being enclosed in a 'parallel', the compiler complains that 
va is private in outer context. Why is the outer region an implicit 
parallel? I tried looking for it in the spec, but failed to locate much 
about implicit parallel enclosures.


Sorry, yes I misread the OMP spec where it says there's an implicit 
barrier at the end of sections construct as section directive block! 
Thanks for the correction.





+  int a[N], b[N], c[N];
+  svint32_t va, vb, vc;
+  int i;
+
+#pragma omp parallel for
+  for (i = 0; i < N; i++)
+{
+  b[i] = i;
+  c[i] = i + 1;
+}
+
+#pragma omp for lastprivate (va) /* { dg-error {lastprivate variable 'va' is 
private in outer context} } */
+  for (i = 0; i < 1; i++)
+{
+  vb = svld1_s32 (svptrue_b32 (), b);
+  vc = svld1_s32 (svptrue_b32 (), c);
+  va = svadd_s32_z (svptrue_b32 (), vb, vc);
+}


vb and vc are shared, so if this was inside of a parallel, it wouldn't be
valid again, there would be a data race (assuming more than one iteration,
a single iteration is kind of weird).

+
+  return va;
+}
+
+svint32_t __attribute__ ((noinline))
+omp_lastprivate_simd ()
+{
+
+  int a[N], b[N], c[N];
+  svint32_t va, vb, vc;
+  int i;
+
+#pragma omp parallel for
+  for (i = 0; i < N; i++)
+{
+  b[i] = i;
+  c[i] = i + 1;
+}
+
+#pragma omp simd lastprivate (va)
+  for (i = 0; i < 1; i++)
+{
+  vb = svld1_s32 (svptrue_b32 (), b);
+  vc = svld1_s32 (svptrue_b32 (), c);
+  va = svadd_s32_z (svptrue_b32 (), vb, vc);
+}


Similarly, these really aren't good examples IMHO on what user should
actually write.



I'm rewriting lastprivate tests to be more realistic - its better to 
make it an execution test.


Thanks,
Tejas.


Jakub





Re: [PATCH 04/11] AArch64: Test OpenMP lastprivate clause for various constructs.

2024-08-06 Thread Jakub Jelinek
On Mon, May 27, 2024 at 10:36:19AM +0530, Tejas Belagod wrote:
> This patch tests various OpenMP lastprivate clause with SVE object types in
> various construct contexts.
> 
> gcc/testsuite/ChangeLog:
> 
>   * gcc.target/aarch64/sve/omp/lastprivate.c: New test.

This is a dg-do compile test, so it could be in gcc.target/aarch64/sve/gomp/

Generally I'd suggest not to mix erroneous constructs with correct ones,
so that the correct ones can be tested up to assembly generation (or runtime
if needed), which might not be the case when there are errors in the same
testcase.
Furethermore, I think it would be useful to actually test the behavior
of lastprviate even for the constructs where you have error now, so
in lastprivate-1.c include say #pragma omp parallel sections lastprivate (va)
rather than just #pragma omp sections lastprivate (va) or the latter
if it is nested inside of #pragma omp parallel (and va is shared there).

> +/* This worksharing construct binds to an implicit outer parallel region in
> +whose scope va is declared and therefore is default private.  This causes
> +the lastprivate clause list item va to be diagnosed as private in the 
> outer
> +context.  Similarly for constructs for and distribute.  */
> +#pragma omp sections lastprivate (va) /* { dg-error {lastprivate variable 
> 'va' is private in outer context} } */
> +{
> +  #pragma omp section
> +  vb = svld1_s32 (svptrue_b32 (), b);
> +  #pragma omp section
> +  vc = svld1_s32 (svptrue_b32 (), c);
> +  #pragma omp section
> +  va = svadd_s32_z (svptrue_b32 (), vb, vc);
> +}

Note, unless you know this is included in implicit parallel region with a
single thread, this isn't really valid.  The sections directive is user
explicitly saying that there are no dependencies between the 3 sections
here, which is clearly not the case, the first two compute variables which
the third one uses.

> +  int a[N], b[N], c[N];
> +  svint32_t va, vb, vc;
> +  int i;
> +
> +#pragma omp parallel for
> +  for (i = 0; i < N; i++)
> +{
> +  b[i] = i;
> +  c[i] = i + 1;
> +}
> +
> +#pragma omp for lastprivate (va) /* { dg-error {lastprivate variable 'va' is 
> private in outer context} } */
> +  for (i = 0; i < 1; i++)
> +{
> +  vb = svld1_s32 (svptrue_b32 (), b);
> +  vc = svld1_s32 (svptrue_b32 (), c);
> +  va = svadd_s32_z (svptrue_b32 (), vb, vc);
> +}

vb and vc are shared, so if this was inside of a parallel, it wouldn't be
valid again, there would be a data race (assuming more than one iteration,
a single iteration is kind of weird).
> +
> +  return va;
> +}
> +
> +svint32_t __attribute__ ((noinline))
> +omp_lastprivate_simd ()
> +{
> +
> +  int a[N], b[N], c[N];
> +  svint32_t va, vb, vc;
> +  int i;
> +
> +#pragma omp parallel for
> +  for (i = 0; i < N; i++)
> +{
> +  b[i] = i;
> +  c[i] = i + 1;
> +}
> +
> +#pragma omp simd lastprivate (va)
> +  for (i = 0; i < 1; i++)
> +{
> +  vb = svld1_s32 (svptrue_b32 (), b);
> +  vc = svld1_s32 (svptrue_b32 (), c);
> +  va = svadd_s32_z (svptrue_b32 (), vb, vc);
> +}

Similarly, these really aren't good examples IMHO on what user should
actually write.

Jakub



[PATCH 04/11] AArch64: Test OpenMP lastprivate clause for various constructs.

2024-05-26 Thread Tejas Belagod
This patch tests various OpenMP lastprivate clause with SVE object types in
various construct contexts.

gcc/testsuite/ChangeLog:

* gcc.target/aarch64/sve/omp/lastprivate.c: New test.
---
 .../gcc.target/aarch64/sve/omp/lastprivate.c  | 121 ++
 1 file changed, 121 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/omp/lastprivate.c

diff --git a/gcc/testsuite/gcc.target/aarch64/sve/omp/lastprivate.c 
b/gcc/testsuite/gcc.target/aarch64/sve/omp/lastprivate.c
new file mode 100644
index 000..e4ecc58a9c4
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/sve/omp/lastprivate.c
@@ -0,0 +1,121 @@
+/* { dg-do compile } */
+/* { dg-options "-msve-vector-bits=256 -std=gnu99 -fopenmp -O2 
-fdump-tree-ompexp" } */
+
+#include 
+
+#define N 8
+
+#ifndef CONSTRUCT
+#define CONSTRUCT
+#endif
+
+svint32_t __attribute__ ((noinline))
+omp_lastprivate_sections ()
+{
+
+  int a[N], b[N], c[N];
+  svint32_t va, vb, vc;
+  int i;
+
+#pragma omp parallel for
+  for (i = 0; i < N; i++)
+{
+  b[i] = i;
+  c[i] = i + 1;
+}
+
+/* This worksharing construct binds to an implicit outer parallel region in
+whose scope va is declared and therefore is default private.  This causes
+the lastprivate clause list item va to be diagnosed as private in the outer
+context.  Similarly for constructs for and distribute.  */
+#pragma omp sections lastprivate (va) /* { dg-error {lastprivate variable 'va' 
is private in outer context} } */
+{
+  #pragma omp section
+  vb = svld1_s32 (svptrue_b32 (), b);
+  #pragma omp section
+  vc = svld1_s32 (svptrue_b32 (), c);
+  #pragma omp section
+  va = svadd_s32_z (svptrue_b32 (), vb, vc);
+}
+
+  return va;
+}
+
+
+svint32_t __attribute__ ((noinline))
+omp_lastprivate_for ()
+{
+
+  int a[N], b[N], c[N];
+  svint32_t va, vb, vc;
+  int i;
+
+#pragma omp parallel for
+  for (i = 0; i < N; i++)
+{
+  b[i] = i;
+  c[i] = i + 1;
+}
+
+#pragma omp for lastprivate (va) /* { dg-error {lastprivate variable 'va' is 
private in outer context} } */
+  for (i = 0; i < 1; i++)
+{
+  vb = svld1_s32 (svptrue_b32 (), b);
+  vc = svld1_s32 (svptrue_b32 (), c);
+  va = svadd_s32_z (svptrue_b32 (), vb, vc);
+}
+
+  return va;
+}
+
+svint32_t __attribute__ ((noinline))
+omp_lastprivate_simd ()
+{
+
+  int a[N], b[N], c[N];
+  svint32_t va, vb, vc;
+  int i;
+
+#pragma omp parallel for
+  for (i = 0; i < N; i++)
+{
+  b[i] = i;
+  c[i] = i + 1;
+}
+
+#pragma omp simd lastprivate (va)
+  for (i = 0; i < 1; i++)
+{
+  vb = svld1_s32 (svptrue_b32 (), b);
+  vc = svld1_s32 (svptrue_b32 (), c);
+  va = svadd_s32_z (svptrue_b32 (), vb, vc);
+}
+
+  return va;
+}
+
+svint32_t __attribute__ ((noinline))
+omp_lastprivate_distribute ()
+{
+
+  int a[N], b[N], c[N];
+  svint32_t va, vb, vc;
+  int i;
+
+#pragma omp parallel for
+  for (i = 0; i < N; i++)
+{
+  b[i] = i;
+  c[i] = i + 1;
+}
+
+#pragma omp distribute lastprivate (va) /* { dg-error {lastprivate variable 
'va' is private in outer context} } */
+  for (i = 0; i < 1; i++)
+{
+  vb = svld1_s32 (svptrue_b32 (), b);
+  vc = svld1_s32 (svptrue_b32 (), c);
+  va = svadd_s32_z (svptrue_b32 (), vb, vc);
+}
+
+  return va;
+}
-- 
2.25.1