Re: [patch] OpenMP: Add -Wopenmp and use it
On Fri, Nov 24, 2023 at 02:51:28PM +0100, Tobias Burnus wrote: > Following the general trend to add a "[-W...]" to the warning messages > for both better grouping of the warnings and - more importantly - for > providing > a means to silence such a warning (or to -Werror= them explicitly), this patch > replaces several '0' by OPT_Wopenmp. > > Comments or remarks before I commit it? LGTM, thanks for working on it. Jakub
[patch] OpenMP: Add -Wopenmp and use it
Following the general trend to add a "[-W...]" to the warning messages
for both better grouping of the warnings and - more importantly - for providing
a means to silence such a warning (or to -Werror= them explicitly), this patch
replaces several '0' by OPT_Wopenmp.
Comments or remarks before I commit it?
Tobias
PS: This does not cover all OpenMP warnings: Besides those '0' that I have
missed,
there are also some warnings which use a different -W... - I have not checked
whether their current -W... value or -Wopenmp makes more sense for them.
-
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
OpenMP: Add -Wopenmp and use it
The new warning has two purposes: First, it makes clearer to the
user that it is about OpenMP and, secondly and more importantly,
it permits to use -Wno-openmp.
The newly added -Wopenmp is enabled by default and replaces the
'0' (always warning) in several OpenMP-related warning calls.
For code shared with OpenACC, it only uses OPT_Wopenmp for
'flag_openmp | flag_openmp_simd'.
gcc/c-family/ChangeLog:
* c.opt (Wopenmp): Add, enable by default.
gcc/c/ChangeLog:
* c-parser.cc (c_parser_omp_clause_num_threads,
c_parser_omp_clause_num_tasks, c_parser_omp_clause_grainsize,
c_parser_omp_clause_priority, c_parser_omp_clause_schedule,
c_parser_omp_clause_num_teams, c_parser_omp_clause_thread_limit,
c_parser_omp_clause_dist_schedule, c_parser_omp_scan_loop_body,
c_parser_omp_assumption_clauses): Add OPT_Wopenmp to warning_at.
gcc/cp/ChangeLog:
* parser.cc (cp_parser_omp_clause_dist_schedule,
cp_parser_omp_scan_loop_body, cp_parser_omp_assumption_clauses):
Add OPT_Wopenmp to warning_at.
* semantics.cc (finish_omp_clauses): Likewise.
gcc/ChangeLog:
* doc/invoke.texi (-Wopenmp): Add.
* gimplify.cc (gimplify_omp_for): Add OPT_Wopenmp to warning_at.
* omp-expand.cc (expand_omp_ordered_sink): Likewise.
* omp-general.cc (omp_check_context_selector): Likewise.
* omp-low.cc (scan_omp_for, check_omp_nesting_restrictions,
lower_omp_ordered_clauses): Likewise.
* omp-simd-clone.cc (simd_clone_clauses_extract): Likewise.
gcc/fortran/ChangeLog:
* lang.opt (Wopenmp): Add, enabled by dafault and documented in C.
* openmp.cc (gfc_match_omp_declare_target, resolve_positive_int_expr,
resolve_nonnegative_int_expr, resolve_omp_clauses,
gfc_resolve_omp_do_blocks): Use OPT_Wopenmp with gfc_warning{,_now}.
gcc/c-family/c.opt| 4
gcc/c/c-parser.cc | 38 +++---
gcc/cp/parser.cc | 14 --
gcc/cp/semantics.cc | 18 ++
gcc/doc/invoke.texi | 7 ++-
gcc/fortran/lang.opt | 4
gcc/fortran/openmp.cc | 32 +++-
gcc/gimplify.cc | 4 ++--
gcc/omp-expand.cc | 11 ++-
gcc/omp-general.cc| 4 ++--
gcc/omp-low.cc| 29 +++--
gcc/omp-simd-clone.cc | 8
12 files changed, 103 insertions(+), 70 deletions(-)
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 3848f378de1..c3d45a6ed96 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1175,6 +1175,10 @@ Wopenacc-parallelism
C C++ Var(warn_openacc_parallelism) Warning
Warn about potentially suboptimal choices related to OpenACC parallelism.
+Wopenmp
+C ObjC C++ ObjC++ Warning Var(warn_openmp) Init(1)
+Warn about suspicious OpenMP code
+
Wopenmp-simd
C C++ Var(warn_openmp_simd) Warning LangEnabledBy(C C++,Wall)
Warn if a simd directive is overridden by the vectorizer cost model.
diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc
index 371dd29557b..f493d764627 100644
--- a/gcc/c/c-parser.cc
+++ b/gcc/c/c-parser.cc
@@ -16071,7 +16071,7 @@ c_parser_omp_clause_num_threads (c_parser *parser, tree list)
protected_set_expr_location (c, expr_loc);
if (c == boolean_true_node)
{
- warning_at (expr_loc, 0,
+ warning_at (expr_loc, OPT_Wopenmp,
"% value must be positive");
t = integer_one_node;
}
@@ -16132,7 +16132,8 @@ c_parser_omp_clause_num_tasks (c_parser *parser, tree list)
SET_EXPR_LOCATION (c, expr_loc);
if (c == boolean_true_node)
{
- warning_at (expr_loc, 0, "% value must be positive");
+ warning_at (expr_loc, OPT_Wopenmp,
+ "% value must be positive");
t = integer_one_node;
}
@@ -16193,7 +16194,8 @@ c_parser_omp_clause_grainsize (c_parser *parser, tree list)
SET_EXPR_LOCATION (c, expr_loc);
if (c == boolean_true_node)
{
- warning_at (expr_loc, 0, "% value must be positive");
+ warning_at (expr_loc, OPT_Wopenmp,
+ "% value must be positive");
t = integer_one_node;
}
@@ -16241,7 +16243,8 @@ c_parser_omp_clause_priority (c_parser *parser, tree list)
SET_EXPR_LOCATION (c, expr_loc);
if (c == boolean_true_node)
{
- warning_at (expr_loc, 0,
[Patch,v3] OpenMP: Accept argument to depobj's destroy clause
As discussed on IRC, we now go for a warning and useOEP_LEXICOGRAPHIC - at
least until the spec issue has been solve.
[The OpenMP spec Issue 3739 tracks the open questions/issues mentioned
in this thread.] Updated patch attached. 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
OpenMP: Accept argument to depobj's destroy clause
Since OpenMP 5.2, the destroy clause takes an depend argument as argument;
for the depobj directive, it the new argument is optional but, if present,
it must be identical to the directive's argument.
gcc/c/ChangeLog:
* c-parser.cc (c_parser_omp_depobj): Accept optionally an argument
to the destroy clause.
gcc/cp/ChangeLog:
* parser.cc (cp_parser_omp_depobj): Accept optionally an argument
to the destroy clause.
gcc/fortran/ChangeLog:
* openmp.cc (gfc_match_omp_depobj): Accept optionally an argument
to the destroy clause.
libgomp/ChangeLog:
* libgomp.texi (5.2 Impl. Status): An argument to the destroy clause
is now supported.
gcc/testsuite/ChangeLog:
* c-c++-common/gomp/depobj-3.c: New test.
* gfortran.dg/gomp/depobj-3.f90: New test.
gcc/c/c-parser.cc | 24 ++-
gcc/cp/parser.cc| 25 ++-
gcc/fortran/openmp.cc | 12 +++-
gcc/testsuite/c-c++-common/gomp/depobj-3.c | 47 +
gcc/testsuite/gfortran.dg/gomp/depobj-3.f90 | 18 +++
libgomp/libgomp.texi| 2 +-
6 files changed, 124 insertions(+), 4 deletions(-)
diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc
index 371dd29557b..989c0503f37 100644
--- a/gcc/c/c-parser.cc
+++ b/gcc/c/c-parser.cc
@@ -21605,6 +21605,9 @@ c_parser_omp_critical (location_t loc, c_parser *parser, bool *if_p)
destroy
update (dependence-type)
+ OpenMP 5.2 additionally:
+ destroy ( depobj )
+
dependence-type:
in
out
@@ -21663,7 +21666,26 @@ c_parser_omp_depobj (c_parser *parser)
clause = error_mark_node;
}
else if (!strcmp ("destroy", p))
- kind = OMP_CLAUSE_DEPEND_LAST;
+ {
+ matching_parens c_parens;
+ kind = OMP_CLAUSE_DEPEND_LAST;
+ if (c_parser_next_token_is (parser, CPP_OPEN_PAREN)
+ && c_parens.require_open (parser))
+ {
+ tree destobj = c_parser_expr_no_commas (parser, NULL).value;
+ if (!lvalue_p (destobj))
+ error_at (EXPR_LOC_OR_LOC (destobj, c_loc),
+ "% expression is not lvalue expression");
+ else if (depobj != error_mark_node
+ && !operand_equal_p (destobj, depobj,
+ OEP_MATCH_SIDE_EFFECTS
+ | OEP_LEXICOGRAPHIC))
+ warning_at (EXPR_LOC_OR_LOC (destobj, c_loc), 0,
+ "the % expression %qE should be the same "
+ "as the % argument %qE", destobj, depobj);
+ c_parens.skip_until_found_close (parser);
+ }
+ }
else if (!strcmp ("update", p))
{
matching_parens c_parens;
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index f6d088bc73f..e4e2feac486 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -43173,6 +43173,9 @@ cp_parser_omp_critical (cp_parser *parser, cp_token *pragma_tok, bool *if_p)
destroy
update (dependence-type)
+ OpenMP 5.2 additionally:
+ destroy ( depobj )
+
dependence-type:
in
out
@@ -43219,7 +43222,27 @@ cp_parser_omp_depobj (cp_parser *parser, cp_token *pragma_tok)
clause = error_mark_node;
}
else if (!strcmp ("destroy", p))
- kind = OMP_CLAUSE_DEPEND_LAST;
+ {
+ kind = OMP_CLAUSE_DEPEND_LAST;
+ matching_parens c_parens;
+ if (cp_lexer_next_token_is (parser->lexer, CPP_OPEN_PAREN)
+ && c_parens.require_open (parser))
+ {
+ tree destobj = cp_parser_assignment_expression (parser);
+ if (depobj != error_mark_node
+ && destobj != error_mark_node
+ && !operand_equal_p (destobj, depobj, OEP_MATCH_SIDE_EFFECTS
+ | OEP_LEXICOGRAPHIC))
+ warning_at (EXPR_LOC_OR_LOC (destobj, c_loc), 0,
+ "the % expression %qE should be the same "
+ "as the % argument %qE", destobj, depobj);
+ if (!c_parens.require_close (parser))
+ cp_parser_skip_to_closing_parenthesis (parser,
+ /*recovering=*/true,
+ /*or_comma=*/false,
+ /*consume_paren=*/true);
+ }
+ }
else if (!strcmp ("update", p))
{
matching_parens c_parens;
diff --git a/gcc/fortran/openmp.cc b/gcc/fortran/openmp.cc
index 2e2e23d567b..b9ac61103af 100644
--- a/gcc/fortran/openmp.cc
+++ b/gcc/fortran/openmp.cc
@@ -4731,10 +4731,20 @@ gfc_match_omp_depobj (void)
goto error;
}
}
- else if (gfc_match ("destroy") == MATCH_YES)
+ else if (gfc_match ("destroy ") == MATCH_YES)
{
+ gfc_expr *destroyobj = NULL;
c = gfc_get_omp_clauses ();
c->destroy = true;
+
+
