* [committed] openmp: Fix up handling of non-rectangular simd loops with pointer type iterators [PR106449]
@ 2022-07-29 8:03 Jakub Jelinek
2022-07-29 9:48 ` [Patch] Add libgomp.c-c++-common/pr106449-2.c (was: [committed] openmp: Fix up handling of non-rectangular simd loops with pointer type iterators [PR106449]) Tobias Burnus
0 siblings, 1 reply; 3+ messages in thread
From: Jakub Jelinek @ 2022-07-29 8:03 UTC (permalink / raw)
To: gcc-patches; +Cc: Tobias Burnus
Hi!
There were 2 issues visible on this new testcase, one that we didn't have
special POINTER_TYPE_P handling in a few spots of expand_omp_simd - for
pointers we need to use POINTER_PLUS_EXPR and need to have the non-pointer
part in sizetype, for non-rectangular loop on the other side we can rely on
multiplication factor 1, pointers can't be multiplied, without those changes
we'd ICE. The other issue was that we put n2 expression directly into a
comparison in a condition and regimplified that, for the &a[512] case that
and with gimplification being destructed that unfortunately meant modification
of original fd->loops[?].n2. Fixed by unsharing the expression. This was
causing a runtime failure on the testcase.
Bootstrapped/regtested on x86_64-linux and i686-linux, committed to trunk.
2022-07-29 Jakub Jelinek <jakub@redhat.com>
PR middle-end/106449
* omp-expand.cc (expand_omp_simd): Fix up handling of pointer
iterators in non-rectangular simd loops. Unshare fd->loops[i].n2
or n2 before regimplifying it inside of a condition.
* testsuite/libgomp.c-c++-common/pr106449.c: New test.
--- gcc/omp-expand.cc.jj 2022-06-28 13:03:30.930689700 +0200
+++ gcc/omp-expand.cc 2022-07-28 10:47:03.138939297 +0200
@@ -6714,7 +6696,7 @@ expand_omp_simd (struct omp_region *regi
if (fd->loops[i].m2)
t = n2v = create_tmp_var (itype);
else
- t = fold_convert (itype, fd->loops[i].n2);
+ t = fold_convert (itype, unshare_expr (fd->loops[i].n2));
t = force_gimple_operand_gsi (&gsi, t, true, NULL_TREE,
false, GSI_CONTINUE_LINKING);
tree v = fd->loops[i].v;
@@ -6728,7 +6710,7 @@ expand_omp_simd (struct omp_region *regi
if (fd->collapse > 1 && !broken_loop)
t = n2var;
else
- t = fold_convert (type, n2);
+ t = fold_convert (type, unshare_expr (n2));
t = force_gimple_operand_gsi (&gsi, t, true, NULL_TREE,
false, GSI_CONTINUE_LINKING);
tree v = fd->loop.v;
@@ -6840,7 +6819,7 @@ expand_omp_simd (struct omp_region *regi
if (fd->loops[i].m2)
t = nextn2v = create_tmp_var (itype);
else
- t = fold_convert (itype, fd->loops[i].n2);
+ t = fold_convert (itype, unshare_expr (fd->loops[i].n2));
t = force_gimple_operand_gsi (&gsi, t, true, NULL_TREE,
false, GSI_CONTINUE_LINKING);
tree v = fd->loops[i].v;
@@ -6870,17 +6849,25 @@ expand_omp_simd (struct omp_region *regi
ne->probability = e->probability.invert ();
gsi = gsi_after_labels (init_bb);
- t = fold_convert (TREE_TYPE (fd->loops[i + 1].v),
- fd->loops[i + 1].n1);
if (fd->loops[i + 1].m1)
{
- tree t2 = fold_convert (TREE_TYPE (t),
+ tree t2 = fold_convert (TREE_TYPE (fd->loops[i + 1].v),
fd->loops[i + 1
- fd->loops[i + 1].outer].v);
- tree t3 = fold_convert (TREE_TYPE (t), fd->loops[i + 1].m1);
- t2 = fold_build2 (MULT_EXPR, TREE_TYPE (t), t2, t3);
- t = fold_build2 (PLUS_EXPR, TREE_TYPE (t), t, t2);
+ if (POINTER_TYPE_P (TREE_TYPE (t2)))
+ t = fold_build_pointer_plus (t2, fd->loops[i + 1].n1);
+ else
+ {
+ t = fold_convert (TREE_TYPE (fd->loops[i + 1].v),
+ fd->loops[i + 1].n1);
+ tree t3 = fold_convert (TREE_TYPE (t), fd->loops[i + 1].m1);
+ t2 = fold_build2 (MULT_EXPR, TREE_TYPE (t), t2, t3);
+ t = fold_build2 (PLUS_EXPR, TREE_TYPE (t), t, t2);
+ }
}
+ else
+ t = fold_convert (TREE_TYPE (fd->loops[i + 1].v),
+ fd->loops[i + 1].n1);
expand_omp_build_assign (&gsi, fd->loops[i + 1].v, t);
if (fd->loops[i + 1].m2)
{
@@ -6889,14 +6876,19 @@ expand_omp_simd (struct omp_region *regi
gcc_assert (n2v == NULL_TREE);
n2v = create_tmp_var (TREE_TYPE (fd->loops[i + 1].v));
}
- t = fold_convert (TREE_TYPE (fd->loops[i + 1].v),
- fd->loops[i + 1].n2);
- tree t2 = fold_convert (TREE_TYPE (t),
+ tree t2 = fold_convert (TREE_TYPE (fd->loops[i + 1].v),
fd->loops[i + 1
- fd->loops[i + 1].outer].v);
- tree t3 = fold_convert (TREE_TYPE (t), fd->loops[i + 1].m2);
- t2 = fold_build2 (MULT_EXPR, TREE_TYPE (t), t2, t3);
- t = fold_build2 (PLUS_EXPR, TREE_TYPE (t), t, t2);
+ if (POINTER_TYPE_P (TREE_TYPE (t2)))
+ t = fold_build_pointer_plus (t2, fd->loops[i + 1].n2);
+ else
+ {
+ t = fold_convert (TREE_TYPE (fd->loops[i + 1].v),
+ fd->loops[i + 1].n2);
+ tree t3 = fold_convert (TREE_TYPE (t), fd->loops[i + 1].m2);
+ t2 = fold_build2 (MULT_EXPR, TREE_TYPE (t), t2, t3);
+ t = fold_build2 (PLUS_EXPR, TREE_TYPE (t), t, t2);
+ }
expand_omp_build_assign (&gsi, n2v, t);
}
if (i + 2 == fd->collapse && n2var)
@@ -6912,17 +6904,25 @@ expand_omp_simd (struct omp_region *regi
tree t2 = fold_build2 (MINUS_EXPR, type, n2, fd->loop.v);
if (fd->loops[i + 1].m1 || fd->loops[i + 1].m2)
{
+ tree itype = TREE_TYPE (fd->loops[i].v);
+ if (POINTER_TYPE_P (itype))
+ itype = signed_type_for (itype);
t = build_int_cst (itype, (fd->loops[i + 1].cond_code
== LT_EXPR ? -1 : 1));
t = fold_build2 (PLUS_EXPR, itype,
fold_convert (itype,
fd->loops[i + 1].step), t);
- if (fd->loops[i + 1].m2)
- t = fold_build2 (PLUS_EXPR, itype, t, n2v);
- else
+ if (fd->loops[i + 1].m2 == NULL_TREE)
t = fold_build2 (PLUS_EXPR, itype, t,
fold_convert (itype,
fd->loops[i + 1].n2));
+ else if (POINTER_TYPE_P (TREE_TYPE (n2v)))
+ {
+ t = fold_build_pointer_plus (n2v, t);
+ t = fold_convert (itype, t);
+ }
+ else
+ t = fold_build2 (PLUS_EXPR, itype, t, n2v);
t = fold_build2 (MINUS_EXPR, itype, t,
fold_convert (itype, fd->loops[i + 1].v));
tree step = fold_convert (itype, fd->loops[i + 1].step);
--- libgomp/testsuite/libgomp.c-c++-common/pr106449.c.jj 2022-07-28 11:35:50.276558893 +0200
+++ libgomp/testsuite/libgomp.c-c++-common/pr106449.c 2022-07-28 12:03:27.495268905 +0200
@@ -0,0 +1,62 @@
+/* PR middle-end/106449 */
+/* { dg-do run } */
+
+void
+foo (void)
+{
+ int a[1024], *b[65536];
+ int *p, *q, **r = &b[0], i;
+ #pragma omp simd collapse(2) linear(r : 2)
+ for (p = &a[0]; p < &a[512]; p++)
+ for (q = p + 64; q < p + 128; q++)
+ {
+ *r++ = p;
+ *r++ = q;
+ }
+ for (i = 0; i < 32768; i++)
+ if (b[2 * i] != &a[i / 64] || b[2 * i + 1] != &a[(i / 64) + 64 + (i % 64)])
+ __builtin_abort ();
+}
+
+void
+bar (int n, int m)
+{
+ int a[1024], *b[65536];
+ int *p, *q, **r = &b[0], i;
+ #pragma omp parallel for simd collapse(2) linear(r : 2)
+ for (p = &a[0]; p < &a[512]; p++)
+ for (q = p + n; q < p + m; q++)
+ {
+ *r++ = p;
+ *r++ = q;
+ }
+ for (i = 0; i < 32768; i++)
+ if (b[2 * i] != &a[i / 64] || b[2 * i + 1] != &a[(i / 64) + 64 + (i % 64)])
+ __builtin_abort ();
+}
+
+void
+baz (int n, int m)
+{
+ int a[1024], *b[8192];
+ int *p, *q, **r = &b[0], i;
+ #pragma omp parallel for simd collapse(2) linear(r : 2)
+ for (p = &a[0]; p < &a[512]; p += 4)
+ for (q = p + n; q < p + m; q += 2)
+ {
+ *r++ = p;
+ *r++ = q;
+ }
+ for (i = 0; i < 4096; i++)
+ if (b[2 * i] != &a[(i / 32) * 4] || b[2 * i + 1] != &a[(i / 32) * 4 + 64 + (i % 32) * 2])
+ __builtin_abort ();
+}
+
+int
+main ()
+{
+ foo ();
+ bar (64, 128);
+ baz (64, 128);
+ return 0;
+}
Jakub
^ permalink raw reply [flat|nested] 3+ messages in thread
* [Patch] Add libgomp.c-c++-common/pr106449-2.c (was: [committed] openmp: Fix up handling of non-rectangular simd loops with pointer type iterators [PR106449])
2022-07-29 8:03 [committed] openmp: Fix up handling of non-rectangular simd loops with pointer type iterators [PR106449] Jakub Jelinek
@ 2022-07-29 9:48 ` Tobias Burnus
2022-07-29 10:34 ` Jakub Jelinek
0 siblings, 1 reply; 3+ messages in thread
From: Tobias Burnus @ 2022-07-29 9:48 UTC (permalink / raw)
To: Jakub Jelinek, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1360 bytes --]
On 29.07.22 10:03, Jakub Jelinek wrote:
> There were 2 issues visible on this new testcase, one that we didn't have
> special POINTER_TYPE_P handling in a few spots of expand_omp_simd ...
> The other issue was that we put n2 expression directly into a
> comparison in a condition and regimplified that, for the &a[512] case that
> and with gimplification being destructed that unfortunately meant modification
> of original fd->loops[?].n2. Fixed by unsharing the expression.
I created a testcase for the non-simd case – and due to messing up, it failed;
hence, I filled PR middle-end/106467. After fixing the testcase, it passes.
(→ closed PR as invalid).
However, given that the testcase now exists, I think it makes sense to add it :-)
Changes compared to the simd testcase: replaced '(parallel for) simd' by 'for',
removed 'linear', used now 'b' and 'c' instead of storing both ptrs in 'b'.
Side remark: Before GCC 12, GCC complained about "q = p + n" with
"error: initializer expression refers to iteration variable ‘p’".
OK for mainline?
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
[-- Attachment #2: ptr-for-test.diff --]
[-- Type: text/x-patch, Size: 2053 bytes --]
Add libgomp.c-c++-common/pr106449-2.c
This run-time test test pointer-based iteration with collapse,
similar to the '(parallel) simd' test for PR106449 but for 'for'.
libgomp/ChangeLog:
* testsuite/libgomp.c-c++-common/pr106449-2.c: New test.
.../testsuite/libgomp.c-c++-common/pr106449-2.c | 64 ++++++++++++++++++++++
1 file changed, 64 insertions(+)
diff --git a/libgomp/testsuite/libgomp.c-c++-common/pr106449-2.c b/libgomp/testsuite/libgomp.c-c++-common/pr106449-2.c
new file mode 100644
index 00000000000..7fef7461bcf
--- /dev/null
+++ b/libgomp/testsuite/libgomp.c-c++-common/pr106449-2.c
@@ -0,0 +1,64 @@
+/* { dg-do run } */
+
+/* Based on pr106449.c - but using 'for' instead of 'simd'.
+ Cf. PR middle-end/106449 (for pr106449.c) and PR middle-end/106467. */
+
+void
+foo (void)
+{
+ int a[1024], *b[65536], *c[65536];
+ int *p, *q, **r = &b[0], **r2 = &c[0], i;
+ #pragma omp for collapse(2)
+ for (p = &a[0]; p < &a[512]; p++)
+ for (q = p + 64; q < p + 128; q++)
+ {
+ *r++ = p;
+ *r2++ = q;
+ }
+ for (i = 0; i < 32768; i++)
+ if (b[i] != &a[i / 64] || c[i] != &a[(i / 64) + 64 + (i % 64)])
+ __builtin_abort ();
+}
+
+void
+bar (int n, int m)
+{
+ int a[1024], *b[32768], *c[32768];
+ int *p, *q, **r = &b[0], **r2 = &c[0], i;
+ #pragma omp for collapse(2)
+ for (p = &a[0]; p < &a[512]; p++)
+ for (q = p + n; q < p + m; q++)
+ {
+ *r++ = p;
+ *r2++ = q;
+ }
+ for (i = 0; i < 32768; i++)
+ if (b[i] != &a[i / 64] || c[i] != &a[(i / 64) + 64 + (i % 64)])
+ __builtin_abort ();
+}
+
+void
+baz (int n, int m)
+{
+ int a[1024], *b[8192], *c[8192];
+ int *p, *q, **r = &b[0], **r2 = &c[0], i;
+ #pragma omp for collapse(2)
+ for (p = &a[0]; p < &a[512]; p += 4)
+ for (q = p + n; q < p + m; q += 2)
+ {
+ *r++ = p;
+ *r2++ = q;
+ }
+ for (i = 0; i < 4096; i++)
+ if (b[i] != &a[(i / 32) * 4] || c[i] != &a[(i / 32) * 4 + 64 + (i % 32) * 2])
+ __builtin_abort ();
+}
+
+int
+main ()
+{
+ foo ();
+ bar (64, 128);
+ baz (64, 128);
+ return 0;
+}
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Patch] Add libgomp.c-c++-common/pr106449-2.c (was: [committed] openmp: Fix up handling of non-rectangular simd loops with pointer type iterators [PR106449])
2022-07-29 9:48 ` [Patch] Add libgomp.c-c++-common/pr106449-2.c (was: [committed] openmp: Fix up handling of non-rectangular simd loops with pointer type iterators [PR106449]) Tobias Burnus
@ 2022-07-29 10:34 ` Jakub Jelinek
0 siblings, 0 replies; 3+ messages in thread
From: Jakub Jelinek @ 2022-07-29 10:34 UTC (permalink / raw)
To: Tobias Burnus; +Cc: gcc-patches
On Fri, Jul 29, 2022 at 11:48:08AM +0200, Tobias Burnus wrote:
> On 29.07.22 10:03, Jakub Jelinek wrote:
> > There were 2 issues visible on this new testcase, one that we didn't have
> > special POINTER_TYPE_P handling in a few spots of expand_omp_simd ...
> > The other issue was that we put n2 expression directly into a
> > comparison in a condition and regimplified that, for the &a[512] case that
> > and with gimplification being destructed that unfortunately meant modification
> > of original fd->loops[?].n2. Fixed by unsharing the expression.
>
> I created a testcase for the non-simd case – and due to messing up, it failed;
> hence, I filled PR middle-end/106467. After fixing the testcase, it passes.
> (→ closed PR as invalid).
>
> However, given that the testcase now exists, I think it makes sense to add it :-)
>
> Changes compared to the simd testcase: replaced '(parallel for) simd' by 'for',
> removed 'linear', used now 'b' and 'c' instead of storing both ptrs in 'b'.
>
> Side remark: Before GCC 12, GCC complained about "q = p + n" with
> "error: initializer expression refers to iteration variable ‘p’".
>
> OK for mainline?
Ok, thanks.
Jakub
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-07-29 10:34 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-29 8:03 [committed] openmp: Fix up handling of non-rectangular simd loops with pointer type iterators [PR106449] Jakub Jelinek
2022-07-29 9:48 ` [Patch] Add libgomp.c-c++-common/pr106449-2.c (was: [committed] openmp: Fix up handling of non-rectangular simd loops with pointer type iterators [PR106449]) Tobias Burnus
2022-07-29 10:34 ` Jakub Jelinek
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).