Re: [patch] OpenMP: Add -Wopenmp and use it

2023-11-27 Thread Christophe Lyon
On Mon, 27 Nov 2023 at 11:33, Tobias Burnus  wrote:
>
> Hi,
>
> On 27.11.23 11:20, Christophe Lyon wrote:
>
> > I think the lack of final '.' in:
>
> Indeed - but you are lagging a bit behind:
>
> https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638128.html
>
> [committed] c-family/c.opt (-Wopenmp): Add missing tailing '.'
>
> Fri Nov 24 18:56:21 GMT 2023
>
> Committed as r14-5835-g6eb1507107dee3
>

Great thanks! Sorry for the noise, it's a bit hard and error-prone to
track which regressions have already fixed and/or are being worked on.
Our bisect started at r14-5830, just a bit too early :-)

Thanks,

Christophe


> 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


Re: [patch] OpenMP: Add -Wopenmp and use it

2023-11-27 Thread Tobias Burnus

Hi,

On 27.11.23 11:20, Christophe Lyon wrote:


I think the lack of final '.' in:


Indeed - but you are lagging a bit behind:

https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638128.html

[committed] c-family/c.opt (-Wopenmp): Add missing tailing '.'

Fri Nov 24 18:56:21 GMT 2023

Committed as r14-5835-g6eb1507107dee3

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


Re: [patch] OpenMP: Add -Wopenmp and use it

2023-11-27 Thread Jakub Jelinek
On Mon, Nov 27, 2023 at 11:20:20AM +0100, Christophe Lyon wrote:
> On Fri, 24 Nov 2023 at 15:08, Jakub Jelinek  wrote:
> > > Comments or remarks before I commit it?
> >
> > LGTM, thanks for working on it.
> >
> > Jakub
> >
> 
> I think the lack of final '.' in:
> gcc/c-family/c.opt
> + Warn about suspicious OpenMP code

Tobias has fixed that a few commits later:
r14-5835-g6eb1507107dee3e67e3a136e2917b93cdffba7c4

Sorry for missing that during patch review.

Jakub



Re: [patch] OpenMP: Add -Wopenmp and use it

2023-11-27 Thread Christophe Lyon
Hi!

On Fri, 24 Nov 2023 at 15:08, Jakub Jelinek  wrote:
>
> 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
>

I think the lack of final '.' in:
gcc/c-family/c.opt
+ Warn about suspicious OpenMP code

has caused the following regressions:
Running gcc:gcc.misc-tests/help.exp ...
FAIL: compiler driver --help=c option(s): "^ +-.*[^:.]$" absent from
output: "  -WopenmpWarn about suspicious OpenMP
code"
FAIL: compiler driver --help=c++ option(s): "^ +-.*[^:.]$" absent from
output: "  -WopenmpWarn about suspicious OpenMP
code"
FAIL: compiler driver --help=fortran option(s): "^ +-.*[^:.]$" absent
from output: "  -WopenmpWarn about suspicious
OpenMP code"
FAIL: compiler driver --help=warnings option(s): "^ +-.*[^:.]$" absent
from output: "  -WopenmpWarn about suspicious
OpenMP code"

I think you have received a notification from our CI about that?

Can you check it's as simple as that?

Thanks,

Christophe


Re: [patch] OpenMP: Add -Wopenmp and use it

2023-11-24 Thread Jakub Jelinek
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

2023-11-24 Thread Tobias Burnus

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,