Re: [PATCH 04/11] AArch64: Test OpenMP lastprivate clause for various constructs.
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.
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.
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