From 4944a9f94bcda9907e0118e71137ee7e192657c2 Mon Sep 17 00:00:00 2001
From: Frederik Harwath <frederik@harwath.name>
Date: Tue, 27 Feb 2024 21:07:00 +0000
Subject: [PATCH] OpenMP: warn about iteration var modifications in loop body
OpenMP loop iteration variables may not be changed by user code in the
loop body according to the OpenMP specification. In general, the
compiler cannot enforce this, but nevertheless simple cases in which
the user modifies the iteration variable directly in the loop body
(in contrast to, e.g., modifications through a pointer) can be recognized. A
warning should be useful, for instance, to new users of OpenMP.
This commit implements a warning about forbidden iteration var modifications
during gimplification. It reuses the "loop_iter_var" vector in the
"gimplify_omp_ctx" which was previously only used for "doacross" handling to
identify the loop iteration variables during the gimplification of MODIFY_EXPRs
in omp_for bodies.
gcc/ChangeLog:
* gimplify.cc (struct gimplify_omp_ctx): Add field "in_omp_for_body" to
recognize the gimplification state during which the new warning should
be emitted. Add field "is_doacross" to distinguish the original use of
"loop_iter_var" from its new use.
(new_omp_context): Initialize new gimplify_omp_ctx fields.
(gimplify_modify_expr): Emit warning if iter var is modified.
(gimplify_omp_for): Make initialization and filling of loop_iter_var
vector unconditional and adjust new gimplify_omp_ctx fields before
gimplifying the omp_for body.
(gimplify_omp_ordered): Check for do_across field in addition to
emptiness check on loop_iter_var vector since the vector is now always
being filled.
gcc/testsuite/ChangeLog:
* c-c++-common/gomp/iter-var-modification.c: New test.
Signed-off-by: Frederik Harwath <frederik@harwath.name>
---
gcc/gimplify.cc | 54 +++++++---
.../c-c++-common/gomp/iter-var-modification.c | 100 ++++++++++++++++++
2 files changed, 138 insertions(+), 16 deletions(-)
create mode 100644 gcc/testsuite/c-c++-common/gomp/iter-var-modification.c
@@ -235,6 +235,8 @@ struct gimplify_omp_ctx
bool order_concurrent;
bool has_depend;
bool in_for_exprs;
+ bool in_omp_for_body;
+ bool is_doacross;
int defaultmap[5];
};
@@ -456,6 +458,10 @@ new_omp_context (enum omp_region_type region_type)
c->privatized_types = new hash_set<tree>;
c->location = input_location;
c->region_type = region_type;
+ c->loop_iter_var.create (0);
+ c->in_omp_for_body = false;
+ c->is_doacross = false;
+
if ((region_type & ORT_TASK) == 0)
c->default_kind = OMP_CLAUSE_DEFAULT_SHARED;
else
@@ -6312,6 +6318,18 @@ gimplify_modify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
gcc_assert (TREE_CODE (*expr_p) == MODIFY_EXPR
|| TREE_CODE (*expr_p) == INIT_EXPR);
+ if (gimplify_omp_ctxp && gimplify_omp_ctxp->in_omp_for_body)
+ {
+ size_t num_vars = gimplify_omp_ctxp->loop_iter_var.length () / 2;
+ for (size_t i = 0; i < num_vars; i++)
+ {
+ if (*to_p == gimplify_omp_ctxp->loop_iter_var[2 * i + 1])
+ warning_at (input_location, OPT_Wopenmp,
+ "forbidden modification of iteration variable %qE in "
+ "OpenMP loop", *to_p);
+ }
+ }
+
/* Trying to simplify a clobber using normal logic doesn't work,
so handle it here. */
if (TREE_CLOBBER_P (*from_p))
@@ -15334,6 +15352,8 @@ gimplify_omp_for (tree *expr_p, gimple_seq *pre_p)
== TREE_VEC_LENGTH (OMP_FOR_COND (for_stmt)));
gcc_assert (TREE_VEC_LENGTH (OMP_FOR_INIT (for_stmt))
== TREE_VEC_LENGTH (OMP_FOR_INCR (for_stmt)));
+ int len = TREE_VEC_LENGTH (OMP_FOR_INIT (for_stmt));
+ gimplify_omp_ctxp->loop_iter_var.create (len * 2);
tree c = omp_find_clause (OMP_FOR_CLAUSES (for_stmt), OMP_CLAUSE_ORDERED);
bool is_doacross = false;
@@ -15342,8 +15362,6 @@ gimplify_omp_for (tree *expr_p, gimple_seq *pre_p)
{
OMP_CLAUSE_ORDERED_DOACROSS (c) = 1;
is_doacross = true;
- int len = TREE_VEC_LENGTH (OMP_FOR_INIT (for_stmt));
- gimplify_omp_ctxp->loop_iter_var.create (len * 2);
for (tree *pc = &OMP_FOR_CLAUSES (for_stmt); *pc; )
if (OMP_CLAUSE_CODE (*pc) == OMP_CLAUSE_LINEAR)
{
@@ -15380,23 +15398,22 @@ gimplify_omp_for (tree *expr_p, gimple_seq *pre_p)
gcc_assert (DECL_P (decl));
gcc_assert (INTEGRAL_TYPE_P (TREE_TYPE (decl))
|| POINTER_TYPE_P (TREE_TYPE (decl)));
- if (is_doacross)
+
+ if (TREE_CODE (for_stmt) == OMP_FOR && OMP_FOR_ORIG_DECLS (for_stmt))
{
- if (TREE_CODE (for_stmt) == OMP_FOR && OMP_FOR_ORIG_DECLS (for_stmt))
+ tree orig_decl = TREE_VEC_ELT (OMP_FOR_ORIG_DECLS (for_stmt), i);
+ if (TREE_CODE (orig_decl) == TREE_LIST)
{
- tree orig_decl = TREE_VEC_ELT (OMP_FOR_ORIG_DECLS (for_stmt), i);
- if (TREE_CODE (orig_decl) == TREE_LIST)
- {
- orig_decl = TREE_PURPOSE (orig_decl);
- if (!orig_decl)
- orig_decl = decl;
- }
- gimplify_omp_ctxp->loop_iter_var.quick_push (orig_decl);
+ orig_decl = TREE_PURPOSE (orig_decl);
+ if (!orig_decl)
+ orig_decl = decl;
}
- else
- gimplify_omp_ctxp->loop_iter_var.quick_push (decl);
- gimplify_omp_ctxp->loop_iter_var.quick_push (decl);
+ gimplify_omp_ctxp->loop_iter_var.quick_push (orig_decl);
}
+ else
+ gimplify_omp_ctxp->loop_iter_var.quick_push (decl);
+ gimplify_omp_ctxp->loop_iter_var.quick_push (decl);
+
if (for_stmt == orig_for_stmt)
{
@@ -15818,9 +15835,13 @@ gimplify_omp_for (tree *expr_p, gimple_seq *pre_p)
TREE_SIDE_EFFECTS (OMP_FOR_BODY (orig_for_stmt)) = 1;
}
}
+ gimplify_omp_ctxp->in_omp_for_body = true;
+ gimplify_omp_ctxp->is_doacross = is_doacross;
gimple *g = gimplify_and_return_first (OMP_FOR_BODY (orig_for_stmt),
&for_body);
+ gimplify_omp_ctxp->in_omp_for_body = false;
+ gimplify_omp_ctxp->is_doacross = false;
if (TREE_CODE (orig_for_stmt) == OMP_TASKLOOP
|| (loop_p && orig_for_stmt == for_stmt))
@@ -17430,7 +17451,8 @@ gimplify_omp_ordered (tree expr, gimple_seq body)
{
for (c = OMP_ORDERED_CLAUSES (expr); c; c = OMP_CLAUSE_CHAIN (c))
if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_DOACROSS
- && gimplify_omp_ctxp->loop_iter_var.is_empty ())
+ && (!gimplify_omp_ctxp->is_doacross
+ || gimplify_omp_ctxp->loop_iter_var.is_empty ()))
{
error_at (OMP_CLAUSE_LOCATION (c),
"%<ordered%> construct with %qs clause must be "
new file mode 100644
@@ -0,0 +1,100 @@
+extern int a[1000];
+
+int main ()
+{
+#pragma omp for
+ for (int i = 0; i < 1000; i++)
+ {
+ if (i % 2 == 0)
+ i++; /* { dg-warning {forbidden modification of iteration variable .i. in OpenMP loop} } */
+ }
+
+ #pragma omp for
+ for (int i = 0; i < 1000; i++)
+ {
+ if (i % 2 == 0);
+ else
+ i++; /* { dg-warning {forbidden modification of iteration variable .i. in OpenMP loop} } */
+ }
+
+#pragma omp for
+ for (int i = 0; i < 1000; i++)
+ {
+ i = 0; /* { dg-warning {forbidden modification of iteration variable .i. in OpenMP loop} } */
+ }
+
+#pragma omp for
+ for (int i = 0; i != 1000; i++)
+ {
+ i = 0; /* { dg-warning {forbidden modification of iteration variable .i. in OpenMP loop} } */
+ }
+
+#pragma omp for
+ for (int i = 1000; i > 0; i--)
+ {
+ i = 0; /* { dg-warning {forbidden modification of iteration variable .i. in OpenMP loop} } */
+ }
+
+#pragma omp for
+ for (int *p = (int*)&a; p < a + 1000; p++)
+ {
+ p = (int*)&a; /* { dg-warning {forbidden modification of iteration variable .p. in OpenMP loop} } */
+ }
+
+#pragma omp for
+ for (int *p = (int*)&a; p < a + 1000; p++)
+ {
+ *p = 0;
+ }
+
+#pragma omp parallel for collapse(3)
+ for (int i = 0; i < 1000; i++)
+ for (int j = 0; j < 1000; j++)
+ for (int k = 0; k < 1000; k++)
+
+ {
+ j++; /* { dg-warning {forbidden modification of iteration variable .j. in OpenMP loop} } */
+ }
+
+#pragma omp target teams distribute parallel for collapse(3)
+ for (int i = 0; i < 1000; i++)
+ for (int j = 0; j < 1000; j++)
+ for (int k = 0; k < 1000; k++)
+
+ {
+ k++; /* { dg-warning {forbidden modification of iteration variable .k. in OpenMP loop} } */
+ }
+
+#pragma omp parallel for collapse(2)
+ for (int i = 0; i < 1000; i++)
+ for (int j = 0; j < 1000; j++)
+ for (int k = 0; k < 1000; k++)
+
+ {
+ j++; /* { dg-warning {forbidden modification of iteration variable .j. in OpenMP loop} } */
+ }
+
+#pragma omp target teams distribute parallel for collapse(2)
+ for (int i = 0; i < 1000; i++)
+ for (int j = 0; j < 1000; j++)
+ for (int k = 0; k < 1000; k++)
+
+ {
+ k++; /* No error since third loop is not included in collapsed loop-nest. */
+ }
+
+ /* Only modifications in the body should be reported. Do not warn about
+ assignments to i,k in the pre-body. */
+ int i = 0;
+ int k = 0;
+#pragma omp target teams distribute parallel for collapse(3)
+ for (i = 1; i < 1000; i++)
+ for (int j = 0; j < 1000; j++)
+ for (k = 1; k < 1000; k++)
+
+ {
+ j++; /* { dg-warning {forbidden modification of iteration variable .j. in OpenMP loop} } */
+ }
+
+ return 0;
+}
--
2.34.1