Add new macro gl_WARN_ADD_MULTIPLE

Add new macro gl_WARN_ADD_MULTIPLE

[Alex Gramiak]

[-- Attachment #1: Type: text/plain, Size: 2905 bytes --]

GNU Emacs spends about ~6s out of ~28s in its configuration process to
determine supported C compiler warnings. The attached patch introduces a
new macro, gl_WARN_ADD_MULTIPLE, that brings this down to about half a
second. This macro first checks if all the supplied warnings are
supported, and then only checks them individually (via gl_WARN_ADD) if
that check fails.

I have also attached sample files configure-prev.ac and configure-new.ac
that were extracted from Emacs's configure.ac for testing. I created a
new directory, copied m4/warnings.m4 and m4/manywarnings.m4 to a new m4
subdirectory, and ran:

  autoconf -f <sample file>

then benchmarked using:

  perf stat -r 20 ./configure

Results of configure-prev.ac:

 Performance counter stats for './configure' (20 runs):

          6,285.79 msec task-clock:u              
                 0      context-switches:u        
                 0      cpu-migrations:u          
           918,039      page-faults:u             
    11,101,658,357      cycles:u                  
    12,777,542,067      stalled-cycles-frontend:u 
    11,234,412,307      stalled-cycles-backend:u  
    14,261,222,341      instructions:u            
                                                  
     2,906,440,712      branches:u                
       100,574,333      branch-misses:u           

           6.34363 +- 0.00614 seconds time elapsed  ( +-  0.10% )


Results of configure-new.ac:

 Performance counter stats for './configure' (20 runs):

            530.29 msec task-clock:u              
                 0      context-switches:u        
                 0      cpu-migrations:u          
            72,010      page-faults:u             
       901,496,456      cycles:u                  
     1,066,313,480      stalled-cycles-frontend:u 
       949,724,068      stalled-cycles-backend:u  
     1,240,144,586      instructions:u            
                                                  
       254,307,360      branches:u                
         7,217,416      branch-misses:u           

           0.53320 +- 0.00210 seconds time elapsed  ( +-  0.39% )

Results of configure-new.ac with a forced fallback (I added a
nonexistent warning to ws):

 Performance counter stats for './configure' (20 runs):

          5,992.42 msec task-clock:u              
                 0      context-switches:u        
                 0      cpu-migrations:u          
           935,599      page-faults:u             
    11,197,607,809      cycles:u                  
    12,982,878,617      stalled-cycles-frontend:u 
    11,427,814,210      stalled-cycles-backend:u  
    14,333,736,238      instructions:u            
                                                  
     2,918,880,583      branches:u                
       100,570,882      branch-misses:u           

            6.0497 +- 0.0468 seconds time elapsed  ( +-  0.77% )


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: gl_WARN_ADD_MULTIPLE --]
[-- Type: text/x-patch, Size: 2974 bytes --]

From ec89f6d04a6b5ac67ec787d28943e4fa4188e5f4 Mon Sep 17 00:00:00 2001
From: Alexander Gramiak <agrambot@gmail.com>
Date: Wed, 1 May 2019 16:31:23 -0600
Subject: [PATCH] warnings: Add new macro gl_WARN_ADD_MULTIPLE

* m4/warnings.m4 (gl_WARN_ADD_MULTIPLE): New macro.
---
 ChangeLog      |  5 +++++
 m4/warnings.m4 | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+)

diff --git a/ChangeLog b/ChangeLog
index cf4116bf3..324112055 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2019-05-01  Alexander Gramiak <agrambot@gmail.com>
+
+	warnings: Add new macro gl_WARN_ADD_MULTIPLE
+	* m4/warnings.m4 (gl_WARN_ADD_MULTIPLE): New macro.
+
 2019-04-30  Paul Eggert  <eggert@cs.ucla.edu>
 
 	Sync lib/mktime-internal.h from glibc
diff --git a/m4/warnings.m4 b/m4/warnings.m4
index 235cac617..92939233d 100644
--- a/m4/warnings.m4
+++ b/m4/warnings.m4
@@ -49,6 +49,52 @@ AS_VAR_POPDEF([gl_Flags])dnl
 AS_VAR_POPDEF([gl_Warn])dnl
 ])
 
+# gl_WARN_ADD_MULTIPLE(OPTIONS, [VARIABLE = WARN_CFLAGS/WARN_CXXFLAGS],
+#                       [PROGRAM = AC_LANG_PROGRAM()])
+# -----------------------------------------------------------------
+# Check if the compiler supports OPTIONS when compiling PROGRAM
+# First check if all OPTIONS are supported together, and if not,
+# check each option individually.
+#
+# The effects of this macro depend on the current language (_AC_LANG).
+AC_DEFUN([gl_WARN_ADD_MULTIPLE],
+[
+dnl FIXME: gl_Warn must be used unquoted until we can assume Autoconf
+dnl 2.64 or newer.
+AS_VAR_PUSHDEF([gl_Warn], [gl_cv_warn_[]_AC_LANG_ABBREV[]_$1])dnl
+AS_VAR_PUSHDEF([gl_Flags], [_AC_LANG_PREFIX[]FLAGS])dnl
+gl_positives=
+for gl_warn_item in $1; do
+  case $gl_warn_item in
+    -Wno-*) gl_positives="$gl_positives -W`expr "X$gl_warn_item" : 'X-Wno-\(.*\)'`" ;;
+    *) gl_positives="$gl_positives $gl_warn_item"
+  esac
+done
+m4_pushdef([gl_Positives], [$gl_positives])dnl
+AC_CACHE_CHECK([whether _AC_LANG compiler handles $1], m4_defn([gl_Warn]), [
+  gl_save_compiler_FLAGS="$gl_Flags"
+  gl_AS_VAR_APPEND(m4_defn([gl_Flags]),
+    [" $gl_unknown_warnings_are_errors ]m4_defn([gl_Positives])["])
+  AC_LINK_IFELSE([m4_default([$3], [AC_LANG_PROGRAM([])])],
+                 [AS_VAR_SET(gl_Warn, [yes])],
+                 [AS_VAR_SET(gl_Warn, [no])])
+  gl_Flags="$gl_save_compiler_FLAGS"
+])
+AS_VAR_IF(gl_Warn,
+          [yes],
+          [gl_AS_VAR_APPEND(m4_if([$2], [], [[WARN_]_AC_LANG_PREFIX[FLAGS]], [[$2]]),
+            [" $1"])],
+          [for gl_warn_item in $1; do
+             gl_WARN_ADD([$gl_warn_item])
+           done])
+m4_popdef([gl_Positives])dnl
+AS_VAR_POPDEF([gl_Flags])dnl
+AS_VAR_POPDEF([gl_Warn])dnl
+m4_ifval([$2],
+         [AS_LITERAL_IF([$2], [AC_SUBST([$2])])],
+         [AC_SUBST([WARN_]_AC_LANG_PREFIX[FLAGS])])dnl
+])
+
 # gl_UNKNOWN_WARNINGS_ARE_ERRORS
 # ------------------------------
 # Clang doesn't complain about unknown warning options unless one also
-- 
2.21.0


[-- Attachment #3: gl_WARN_ADD --]
[-- Type: text/plain, Size: 2621 bytes --]

AC_INIT(GNU Emacs, 27.0.50, bug-gnu-emacs@gnu.org, , https://www.gnu.org/software/emacs/)
# This, $nw, is the list of warnings we disable.
nw=

nw="$nw -Wcast-align -Wcast-align=strict" # Emacs is tricky with pointers.
nw="$nw -Wduplicated-branches"    # Too many false alarms
nw="$nw -Wformat-overflow=2"      # False alarms due to GCC bug 80776
nw="$nw -Wsystem-headers"         # Don't let system headers trigger warnings
nw="$nw -Woverlength-strings"     # Not a problem these days
nw="$nw -Wformat-nonliteral"      # we do this a lot
nw="$nw -Wvla"                    # Emacs uses <vla.h>.
nw="$nw -Wunused-const-variable=2" # lisp.h declares const objects.
nw="$nw -Winline"                 # OK to ignore 'inline'
nw="$nw -Wstrict-overflow"        # OK to optimize assuming that
                                  # signed overflow has undefined behavior
nw="$nw -Wsync-nand"              # irrelevant here, and provokes ObjC warning
nw="$nw -Wunsafe-loop-optimizations" # OK to suppress unsafe optimizations
nw="$nw -Wbad-function-cast"      # These casts are no worse than others.
nw="$nw -Wabi"                    # Not useful, perceived as noise

# Emacs doesn't care about shadowing; see
# <https://lists.gnu.org/r/emacs-diffs/2011-11/msg00265.html>.
nw="$nw -Wshadow"

# Emacs's use of alloca inhibits protecting the stack.
nw="$nw -Wstack-protector"

# Emacs's use of __attribute__ ((cold)) causes false alarms with this option.
nw="$nw -Wsuggest-attribute=cold"

# Emacs's use of partly-const functions such as Fgnutls_available_p
# make this option problematic.
nw="$nw -Wsuggest-attribute=const"

# Emacs's use of partly-pure functions such as CHECK_TYPE make this
# option problematic.
nw="$nw -Wsuggest-attribute=pure"

# This part is merely for shortening the command line,
# since -Wall implies -Wswitch.
nw="$nw -Wswitch"

# This part is merely for shortening the command line,
# since -Wno-FOO needs to be added below regardless.
nw="$nw -Wmissing-field-initializers"
nw="$nw -Woverride-init"
nw="$nw -Wtype-limits"
nw="$nw -Wunused-parameter"

gl_MANYWARN_ALL_GCC([ws])
gl_MANYWARN_COMPLEMENT([ws], [$ws], [$nw])
for w in $ws; do
  gl_WARN_ADD([$w])
done

gl_WARN_ADD([-Wredundant-decls])     # Prefer this, as we don't use Bison.
gl_WARN_ADD([-Wno-missing-field-initializers]) # We need this one
gl_WARN_ADD([-Wno-override-init])    # More trouble than it is worth
gl_WARN_ADD([-Wno-sign-compare])     # Too many warnings for now
gl_WARN_ADD([-Wno-type-limits])      # Too many warnings for now
gl_WARN_ADD([-Wno-unused-parameter]) # Too many warnings for now
gl_WARN_ADD([-Wno-format-nonliteral])

[-- Attachment #4: gl_WARN_ADD_MULTIPLE --]
[-- Type: text/plain, Size: 2597 bytes --]

AC_INIT(GNU Emacs, 27.0.50, bug-gnu-emacs@gnu.org, , https://www.gnu.org/software/emacs/)
# This, $nw, is the list of warnings we disable.
nw=

nw="$nw -Wcast-align -Wcast-align=strict" # Emacs is tricky with pointers.
nw="$nw -Wduplicated-branches"    # Too many false alarms
nw="$nw -Wformat-overflow=2"      # False alarms due to GCC bug 80776
nw="$nw -Wsystem-headers"         # Don't let system headers trigger warnings
nw="$nw -Woverlength-strings"     # Not a problem these days
nw="$nw -Wformat-nonliteral"      # we do this a lot
nw="$nw -Wvla"                    # Emacs uses <vla.h>.
nw="$nw -Wunused-const-variable=2" # lisp.h declares const objects.
nw="$nw -Winline"                 # OK to ignore 'inline'
nw="$nw -Wstrict-overflow"        # OK to optimize assuming that
                                  # signed overflow has undefined behavior
nw="$nw -Wsync-nand"              # irrelevant here, and provokes ObjC warning
nw="$nw -Wunsafe-loop-optimizations" # OK to suppress unsafe optimizations
nw="$nw -Wbad-function-cast"      # These casts are no worse than others.
nw="$nw -Wabi"                    # Not useful, perceived as noise

# Emacs doesn't care about shadowing; see
# <https://lists.gnu.org/r/emacs-diffs/2011-11/msg00265.html>.
nw="$nw -Wshadow"

# Emacs's use of alloca inhibits protecting the stack.
nw="$nw -Wstack-protector"

# Emacs's use of __attribute__ ((cold)) causes false alarms with this option.
nw="$nw -Wsuggest-attribute=cold"

# Emacs's use of partly-const functions such as Fgnutls_available_p
# make this option problematic.
nw="$nw -Wsuggest-attribute=const"

# Emacs's use of partly-pure functions such as CHECK_TYPE make this
# option problematic.
nw="$nw -Wsuggest-attribute=pure"

# This part is merely for shortening the command line,
# since -Wall implies -Wswitch.
nw="$nw -Wswitch"

# This part is merely for shortening the command line,
# since -Wno-FOO needs to be added below regardless.
nw="$nw -Wmissing-field-initializers"
nw="$nw -Woverride-init"
nw="$nw -Wtype-limits"
nw="$nw -Wunused-parameter"

gl_MANYWARN_ALL_GCC([ws])
gl_MANYWARN_COMPLEMENT([ws], [$ws], [$nw])

ws="$ws -Wredundant-decls"     # Prefer this, as we don't use Bison.
ws="$ws -Wno-missing-field-initializers" # We need this one
ws="$ws -Wno-override-init"    # More trouble than it is worth
ws="$ws -Wno-sign-compare"     # Too many warnings for now
ws="$ws -Wno-type-limits"      # Too many warnings for now
ws="$ws -Wno-unused-parameter" # Too many warnings for now
ws="$ws -Wno-format-nonliteral"

dnl ws="$ws -Wbogus-warning"

gl_WARN_ADD_MULTIPLE([$ws])

Re: Add new macro gl_WARN_ADD_MULTIPLE

[Bruno Haible]

Hi Alex,

> GNU Emacs spends about ~6s out of ~28s in its configuration process to
> determine supported C compiler warnings.

It's good that you tackle this problem. Thanks!

> This macro first checks if all the supplied warnings are
> supported, and then only checks them individually (via gl_WARN_ADD) if
> that check fails.

So, the speed improvement exists only for the newest compilers; people
who use a compiler that was released 1 or 2 years before the Emacs release
will see no improvement.

How about a modified algorithm?
  1) In a first pass, use the GCC's --help=warnings output to determine which
     warnings are likely supported. Unfortunately, for clang, I don't know
     of such a help option.
  2) When the first check (with all the options) fails, split that list into
     2 (or 3, or 4) chunks of nearly equal size, and perform the check
     on each sub-list. And so on, recurse.
     This way, you would still get some performance benefit if the compiler
     supports 80% of the requested warnings but not all of them.

Finally, since there is no semantic difference between gl_WARN_ADD and
gl_WARN_ADD_MULTIPLE, except that the latter allows multiple options, how
about extending the gl_WARN_ADD macro (to allow multiple options) instead
of defining a different macro (gl_WARN_ADD_MULTIPLE)?

Bruno

Re: Add new macro gl_WARN_ADD_MULTIPLE

[Tim Rühsen]

[-- Attachment #1.1: Type: text/plain, Size: 1950 bytes --]

On 5/2/19 10:16 AM, Bruno Haible wrote:
> Hi Alex,
> 
>> GNU Emacs spends about ~6s out of ~28s in its configuration process to
>> determine supported C compiler warnings.
> 
> It's good that you tackle this problem. Thanks!
> 
>> This macro first checks if all the supplied warnings are
>> supported, and then only checks them individually (via gl_WARN_ADD) if
>> that check fails.
> 
> So, the speed improvement exists only for the newest compilers; people
> who use a compiler that was released 1 or 2 years before the Emacs release
> will see no improvement.
> 
> How about a modified algorithm?
>   1) In a first pass, use the GCC's --help=warnings output to determine which
>      warnings are likely supported. Unfortunately, for clang, I don't know
>      of such a help option.
>   2) When the first check (with all the options) fails, split that list into
>      2 (or 3, or 4) chunks of nearly equal size, and perform the check
>      on each sub-list. And so on, recurse.
>      This way, you would still get some performance benefit if the compiler
>      supports 80% of the requested warnings but not all of them.
> 
> Finally, since there is no semantic difference between gl_WARN_ADD and
> gl_WARN_ADD_MULTIPLE, except that the latter allows multiple options, how
> about extending the gl_WARN_ADD macro (to allow multiple options) instead
> of defining a different macro (gl_WARN_ADD_MULTIPLE)?

Mentioned 2 years ago (but being still too busy to make a gnulib module
from it); Wget2's implementation using --help=warnings is here and FSF
copyrighted:

https://gitlab.com/gnuwget/wget2/blob/master/m4/wget_manywarnings.m4

And how to use / fine-tune it can be seen in L106 of

https://gitlab.com/gnuwget/wget2/blob/master/configure.ac

There is gcc and clang support. The configure.ac creates two sets of
compiler flags, for wget2/libwget sources and for gnulib sources.

Regards, Tim


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Add new macro gl_WARN_ADD_MULTIPLE

[Bruno Haible]

Hi Tim,

> Mentioned 2 years ago (but being still too busy to make a gnulib module
> from it); Wget2's implementation using --help=warnings is here and FSF
> copyrighted:
> 
> https://gitlab.com/gnuwget/wget2/blob/master/m4/wget_manywarnings.m4
> 
> And how to use / fine-tune it can be seen in L106 of
> 
> https://gitlab.com/gnuwget/wget2/blob/master/configure.ac

Does this implementation of manywarnings support only the case where
you want all possible warnings except an explicitly specified set?
Or also the case where you want only an explicitly specified set of
warnings?
Although I think that the first approach is the better one in the long
run, some projects may want to use the second approach, and the gnulib
module so far supports both approaches.

> There is gcc and clang support.

This way of detecting clang is not portable:

    case $CC in
      *gcc*) CCNAME="gcc";;
      *clang*) CCNAME="clang";;
    esac

On FreeBSD, for example, $CC = "cc" is clang but you cannot know it
by looking at the command name. You need to run the preprocessor and
see how it processes
  #if defined __clang__

Bruno

Re: Add new macro gl_WARN_ADD_MULTIPLE

[Tim Rühsen]

[-- Attachment #1.1: Type: text/plain, Size: 2423 bytes --]

Hi Bruno,

On 02.05.19 13:28, Bruno Haible wrote:
> Hi Tim,
> 
>> Mentioned 2 years ago (but being still too busy to make a gnulib module
>> from it); Wget2's implementation using --help=warnings is here and FSF
>> copyrighted:
>>
>> https://gitlab.com/gnuwget/wget2/blob/master/m4/wget_manywarnings.m4
>>
>> And how to use / fine-tune it can be seen in L106 of
>>
>> https://gitlab.com/gnuwget/wget2/blob/master/configure.ac
> 
> Does this implementation of manywarnings support only the case where
> you want all possible warnings except an explicitly specified set?
> Or also the case where you want only an explicitly specified set of
> warnings?

It supports the first approach only, in detail
- all warnings except a given subset
- you can add warnings, e.g. you might need -Wno-... for warnings that
are implicitly enabled. Especially with clang where '--help=warnings -Q'
isn't available. Here we use -Weverything and you have to add options to
silence some of the overly expressive warnings.

This approach benefits from new options in newer versions of gcc/clang
without maintaining lists of options in the .m4 file.

The original idea was to avoid compiler calls to reduce configure time.

It works only with gcc 4.3+.

> Or also the case where you want only an explicitly specified set of
> warnings?

If you need that, why not just set CFLAGS ? Or maybe I don't understand
exactly...


> Although I think that the first approach is the better one in the long
> run, some projects may want to use the second approach, and the gnulib
> module so far supports both approaches.
> 
>> There is gcc and clang support.
> 
> This way of detecting clang is not portable:
> 
>     case $CC in
>       *gcc*) CCNAME="gcc";;
>       *clang*) CCNAME="clang";;
>     esac
> 
> On FreeBSD, for example, $CC = "cc" is clang but you cannot know it
> by looking at the command name. You need to run the preprocessor and
> see how it processes
>   #if defined __clang__

Thanks, good to know. Our development env is a pretty up-to-date
GNU/Linux with a recent gcc and clang - that is were we use
--enable-manywarnings. Of course this macro file has to be fine-tuned
for other development environments. So anyone wanting to use the feature
on FreeBSD, Windows+MSVC or wherever is free to contribute. (But I don't
expect much from that direction.)

Regards, Tim


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Add new macro gl_WARN_ADD_MULTIPLE

[Bruno Haible]

Hi Tim,

> > Does this implementation of manywarnings support only the case where
> > you want all possible warnings except an explicitly specified set?
> > Or also the case where you want only an explicitly specified set of
> > warnings?
> 
> It supports the first approach only, in detail
> - all warnings except a given subset
> - you can add warnings, e.g. you might need -Wno-... for warnings that
> are implicitly enabled. Especially with clang where '--help=warnings -Q'
> isn't available. Here we use -Weverything and you have to add options to
> silence some of the overly expressive warnings.

OK, but a similar thing can be done for the opposite case (just with no
-Weverything). So we could have two macros
  gl_MANYWARN_GCC_ALL_EXCEPT([warnings to disable])
  gl_MANYWARN_GCC_ENABLE([warnings to enable])

> It works only with gcc 4.3+.

We can keep the existing, slower code around and use it for GCC versions
< 4.2.

> > Or also the case where you want only an explicitly specified set of
> > warnings?
> 
> If you need that, why not just set CFLAGS ? Or maybe I don't understand
> exactly...

Because GCC freaks out when you pass it warning options that it does not
understand. The whole point of this testing is to pass to GCC only those
options that will not make it freak out.

Bruno

Re: Add new macro gl_WARN_ADD_MULTIPLE

[Tim Rühsen]

[-- Attachment #1.1: Type: text/plain, Size: 2103 bytes --]

Hi Bruno,

On 02.05.19 21:12, Bruno Haible wrote:
>>> Does this implementation of manywarnings support only the case where
>>> you want all possible warnings except an explicitly specified set?
>>> Or also the case where you want only an explicitly specified set of
>>> warnings?
>>
>> It supports the first approach only, in detail
>> - all warnings except a given subset
>> - you can add warnings, e.g. you might need -Wno-... for warnings that
>> are implicitly enabled. Especially with clang where '--help=warnings -Q'
>> isn't available. Here we use -Weverything and you have to add options to
>> silence some of the overly expressive warnings.
> 
> OK, but a similar thing can be done for the opposite case (just with no
> -Weverything). So we could have two macros
>   gl_MANYWARN_GCC_ALL_EXCEPT([warnings to disable])

This is already there, you fill $WARN_CFLAGS with all C warn flags with
  wget_MANYWARNINGS(WARN_CFLAGS, C)
Now you can remove your unwanted warn flags via
  wget_WORD_REMOVE([WARN_CFLAGS], [$WARN_CFLAGS], [list of unwanted
warnings])

>   gl_MANYWARN_GCC_ENABLE([warnings to enable])

see below...

> 
>> It works only with gcc 4.3+.
> 
> We can keep the existing, slower code around and use it for GCC versions
> < 4.2.
> 
>>> Or also the case where you want only an explicitly specified set of
>>> warnings?
>>
>> If you need that, why not just set CFLAGS ? Or maybe I don't understand
>> exactly...
> 
> Because GCC freaks out when you pass it warning options that it does not
> understand. The whole point of this testing is to pass to GCC only those
> options that will not make it freak out.

I understand, but have a use case for this. It means I have to keep this
list up-to-date with each new compiler version (checking what is new and
if i want to add it to the list). That is maintenance burden I try to
avoid with my approach, were I automatically make use of new warning
flags. E.g. after the next 'apt upgrade' I might pull in gcc 9 and the
next build just might come up with new warnings...

Regards, Tim


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]