omp-expand.cc: Fix wrong code with non-rectangular loop nest [PR111017]
Before commit r12-5295-g47de0b56ee455e, all gimple_build_cond in
expand_omp_for_* were inserted with
gsi_insert_before (gsi_p, cond_stmt, GSI_SAME_STMT);
except the one dealing with the multiplicative factor that was
gsi_insert_after (gsi, cond_stmt, GSI_CONTINUE_LINKING);
That commit for PR103208 fixed the issue of some missing regimplify of
operands of GIMPLE_CONDs by moving the condition handling to the new function
expand_omp_build_cond. While that function has an 'bool after = false'
argument to switch between the two variants.
However, all callers ommited this argument. This commit reinstates the
prior behavior by passing 'true' for the factor != 0 condition, fixing
the included testcase.
PR middle-end/111017
gcc/
* omp-expand.cc (expand_omp_for_init_vars): Pass after=true
to expand_omp_build_cond for 'factor != 0' condition, resulting
in pre-r12-5295-g47de0b56ee455e code for the gimple insert.
libgomp/
* testsuite/libgomp.c-c++-common/non-rect-loop-1.c: New test.
---
gcc/omp-expand.cc | 3 +-
.../libgomp.c-c++-common/non-rect-loop-1.c | 72 ++++++++++++++++++++++
2 files changed, 74 insertions(+), 1 deletion(-)
@@ -2562,7 +2562,8 @@ expand_omp_for_init_vars (struct omp_for_data *fd, gimple_stmt_iterator *gsi,
tree factor = fd->factor;
gcond *cond_stmt
= expand_omp_build_cond (gsi, NE_EXPR, factor,
- build_zero_cst (TREE_TYPE (factor)));
+ build_zero_cst (TREE_TYPE (factor)),
+ true);
edge e = split_block (gsi_bb (*gsi), cond_stmt);
basic_block bb0 = e->src;
e->flags = EDGE_TRUE_VALUE;
new file mode 100644
@@ -0,0 +1,72 @@
+/* PR middle-end/111017 */
+
+#include <omp.h>
+
+#define DIM 32
+#define N (DIM*DIM)
+
+int
+main ()
+{
+ int a[N], b[N], c[N];
+ int dim = DIM;
+
+ for (int i = 0; i < N; i++)
+ {
+ a[i] = 3*i;
+ b[i] = 7*i;
+ c[i] = 42;
+ }
+
+ #pragma omp parallel for collapse(2)
+ for (int i = 0; i < DIM; i++)
+ for (int j = (i*DIM); j < (i*DIM + DIM); j++)
+ c[j] = a[j] + b[j];
+
+ for (int i = 0; i < DIM; i++)
+ for (int j = (i*DIM); j < (i*DIM + DIM); j++)
+ if (c[j] != a[j] + b[j] || c[j] != 3*j +7*j)
+ __builtin_abort ();
+ for (int i = 0; i < N; i++)
+ c[i] = 42;
+
+ #pragma omp parallel for collapse(2)
+ for (int i = 0; i < dim; i++)
+ for (int j = (i*dim); j < (i*dim + dim); j++)
+ c[j] = a[j] + b[j];
+
+ for (int i = 0; i < DIM; i++)
+ for (int j = (i*DIM); j < (i*DIM + DIM); j++)
+ if (c[j] != a[j] + b[j] || c[j] != 3*j +7*j)
+ __builtin_abort ();
+ for (int i = 0; i < N; i++)
+ c[i] = 42;
+
+ for (int dev = 0; dev <= omp_get_num_devices(); dev++)
+ {
+ #pragma omp target teams loop device(dev) map(to:a,b) map(from:c)
+ for (int i = 0; i < DIM; i++)
+ for (int j = (i*DIM); j < (i*DIM + DIM); j++)
+ c[j] = a[j] + b[j];
+
+ for (int i = 0; i < DIM; i++)
+ for (int j = (i*DIM); j < (i*DIM + DIM); j++)
+ if (c[j] != a[j] + b[j] || c[j] != 3*j +7*j)
+ __builtin_abort ();
+ for (int i = 0; i < N; i++)
+ c[i] = 42;
+
+ #pragma omp target teams loop device(dev) map(to:a,b) map(from:c)
+ for (int i = 0; i < dim; i++)
+ for (int j = (i*dim); j < (i*dim + dim); j++)
+ c[j] = a[j] + b[j];
+
+ for (int i = 0; i < DIM; i++)
+ for (int j = (i*DIM); j < (i*DIM + DIM); j++)
+ if (c[j] != a[j] + b[j] || c[j] != 3*j +7*j)
+ __builtin_abort ();
+ for (int i = 0; i < N; i++)
+ c[i] = 42;
+ }
+ return 0;
+}
This patch fixes a bug with an OpenMP non-rectangular loop nest where the factor is 0. With the old code before r12-5295-g47de0b56ee455e, the testcase of the PR (or included in the attached patch) worked fine. omp-expand.c contained back then: https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=gcc/omp-expand.c;hb=eacdfaf7ca07367ede1a0c50aa997953958dabae#l2560 2560 gcond *cond_stmt 2561 = gimple_build_cond (NE_EXPR, factor, 2562 build_zero_cst (TREE_TYPE (factor)), 2563 NULL_TREE, NULL_TREE); 2564 gsi_insert_after (gsi, cond_stmt, GSI_CONTINUE_LINKING); In commit https://gcc.gnu.org/r12-5295-g47de0b56ee455e a new function was introduced: +/* Prepend or append LHS CODE RHS condition before or after *GSI_P. */ + +static gcond * +expand_omp_build_cond (gimple_stmt_iterator *gsi_p, enum tree_code code, + tree lhs, tree rhs, bool after = false) +{ + gcond *cond_stmt = gimple_build_cond (code, lhs, rhs, NULL_TREE, NULL_TREE); + if (after) + gsi_insert_after (gsi_p, cond_stmt, GSI_CONTINUE_LINKING); + else + gsi_insert_before (gsi_p, cond_stmt, GSI_SAME_STMT); While it supports both before/GSI_SAME_STMT and after/GSI_CONTINUE_LINKING, the patch missed to add an '/* after= */ true for the 'factor != 0' condition above. (For all others, after=false was fine.) This patch reinstates the prior after/GSI_CONTINUE_LINKING by adding 'true' to the call and, thus, fixes the in between segfaulting testcase of the PR, https://gcc.gnu.org/PR111017 Comments, questions, concerns? If not, I intent to commit the attached patch to mainline on Monday and after the usual grace time to GCC 13 and then to GCC 12. 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