[PATCH macros] Cache the results of checking compiler flag support in XORG_TESTSET_CFLAG

[Gaetan Nadon]

On 12-01-12 11:49 AM, Jon TURNEY wrote:
> Since checking all those warning flags takes an amount of time I find irritating,
> here is an attempt at caching the results of checking compiler warning flag support
>
> A couple of aspects which need definitely need review or testing:
>
> * I've tried to get things right when not the first flag in the list of alternates is
> supported, but this really needs testing with a compiler other than gcc
I used an unrecognised flag such as "-i" and moved it around in the list.
>
> * The cache variable naming policy is a bit opaque me, so the names used may need corecting
>
> * I've tried to be careful, but who knows what portability sins I have committed :-)
I did not see anything suspicious, everything is autoconf blessed.
>
> This change reduces the time to run the ./configure script produced from a configure.ac
> containing just
>
> AC_INIT([test], 1.0)
> XORG_COMPILER_FLAGS
> XORG_CWARNFLAGS
> XORG_STRICT_OPTION
> AC_OUTPUT()
>
> from ~60s to ~20s on my cygwin machine, and from ~15s to ~9s on a linux VM running on the
> same hardware
>
> Signed-off-by: Jon TURNEY <jon.turney at dronecode.org.uk>
> ---
>  xorg-macros.m4.in |   36 +++++++++++++++++++++++-------------
>  1 files changed, 23 insertions(+), 13 deletions(-)
>
> diff --git a/xorg-macros.m4.in b/xorg-macros.m4.in
> index 8197eb6..e6582cb 100644
> --- a/xorg-macros.m4.in
> +++ b/xorg-macros.m4.in
> @@ -1450,6 +1450,7 @@ AC_CHECK_DECL([__SUNPRO_C], [SUNCC="yes"], [SUNCC="no"])
>  #
>  AC_DEFUN([XORG_TESTSET_CFLAG], [
>  AC_REQUIRE([AC_PROG_CC_C99])
> +AC_REQUIRE([AC_PROG_SED])
This is not required, AC_PROG_SED is invoked by XORG_MANPAGE_SECTIONS
which is invoked by  XORG_DEFAULT_OPTIONS. There is an assumption that
all modules invoke XORG_DEFAULT_OPTIONS which is the case today. When
new macros are added., there is no attempt to test them outside this
context. No harm done to leave it in, however.
>  m4_if([$#], 0, [m4_fatal([XORG_TESTSET_CFLAG was given with an unsupported number of arguments])])
>  m4_if([$#], 1, [m4_fatal([XORG_TESTSET_CFLAG was given with an unsupported number of arguments])])
>  
> @@ -1457,11 +1458,12 @@ xorg_testset_save_CFLAGS="$CFLAGS"
>  
>  if test "x$xorg_testset_unknown_warning_option" = "x" ; then
>  	CFLAGS="$CFLAGS -Werror=unknown-warning-option"
> -	AC_MSG_CHECKING([if $CC supports -Werror=unknown-warning-option])
> -	AC_COMPILE_IFELSE([AC_LANG_SOURCE([int i;])],
> -	                  [xorg_testset_unknown_warning_option=yes],
> -	                  [xorg_testset_unknown_warning_option=no])
> -	AC_MSG_RESULT([$xorg_testset_unknown_warning_option])
> +	AC_CACHE_CHECK([if $CC supports -Werror=unknown-warning-option],
> +			xorg_cv_cc_flag_unknown_warning_option,
> +			AC_COMPILE_IFELSE([AC_LANG_SOURCE([int i;])],
> +					  [xorg_cv_cc_flag_unknown_warning_option=yes],
> +					  [xorg_cv_cc_flag_unknown_warning_option=no]))
> +	xorg_testset_unknown_warning_option=$xorg_cv_cc_flag_unknown_warning_option
>  	CFLAGS="$xorg_testset_save_CFLAGS"
>  fi
>  
> @@ -1470,11 +1472,12 @@ if test "x$xorg_testset_unused_command_line_argument" = "x" ; then
>  		CFLAGS="$CFLAGS -Werror=unknown-warning-option"
>  	fi
>  	CFLAGS="$CFLAGS -Werror=unused-command-line-argument"
> -	AC_MSG_CHECKING([if $CC supports -Werror=unused-command-line-argument])
> -	AC_COMPILE_IFELSE([AC_LANG_SOURCE([int i;])],
> -	                  [xorg_testset_unused_command_line_argument=yes],
> -	                  [xorg_testset_unused_command_line_argument=no])
> -	AC_MSG_RESULT([$xorg_testset_unused_command_line_argument])
> +	AC_CACHE_CHECK([if $CC supports -Werror=unused-command-line-argument],
> +			xorg_cv_cc_flag_unused_command_line_argument,
> +			AC_COMPILE_IFELSE([AC_LANG_SOURCE([int i;])],
> +					  [xorg_cv_cc_flag_unused_command_line_argument=yes],
> +					  [xorg_cv_cc_flag_unused_command_line_argument=no]))
> +	xorg_testset_unused_command_line_argument=$xorg_cv_cc_flag_unused_command_line_argument
>  	CFLAGS="$xorg_testset_save_CFLAGS"
>  fi
>  
> @@ -1491,12 +1494,19 @@ m4_foreach([flag], m4_cdr($@), [
>  
>  		CFLAGS="$CFLAGS ]flag["
>  
> +dnl Some hackery here since AC_CACHE_VAL can't handle a non-literal varname and
> +dnl flag may contains spaces as it may contain multiple flags, but a cacheid can't
>  		AC_MSG_CHECKING([if $CC supports ]flag[])
> -		AC_LINK_IFELSE([AC_LANG_PROGRAM([int i;])],
> -		                  [supported=yes], [supported=no])
> -		AC_MSG_RESULT([$supported])
> +		cacheid=`AS_ECHO_N([xorg_cv_cc_flag_]flag[]) | $SED 's/ /_/'`
Can you have a look at:
http://www.gnu.org/software/autoconf/manual/autoconf.html#index-AS_005fTR_005fSH-1519

          # This outputs "Have it!".
          header="sys/some file.h"
          eval AS_TR_SH([HAVE_$header])=yes
          if test "x$HAVE_sys_some_file_h" = xyes; then echo "Have it!"; fi

This example seem to indicate you would not need to replace spaces with
underscore yourself.
> +		AC_CACHE_VAL(AS_TR_SH($cacheid),
> +			     [AC_LINK_IFELSE([AC_LANG_PROGRAM([int i;])],
> +					     [eval AS_TR_SH($cacheid)=yes],
> +					     [eval AS_TR_SH($cacheid)=no])])
> +
>  		CFLAGS="$xorg_testset_save_CFLAGS"
>  
> +		eval supported=$AS_TR_SH($cacheid)
> +		AC_MSG_RESULT([$supported])
>  		if test "$supported" = "yes" ; then
>  			$1="$$1 ]flag["
>  			found="yes"

I have one question regarding how you use the cache file. You are aware
that some of the Automake variables we create in the various xorg
modules do not have a unique name across packages. For example. in the
driver section, PKG_CHECK MODULES(XORG, ...) is very common. The
variable pkg_cv_XORG_LIBS does not have the same value for all drivers.
Using a cache file will almost always return the wrong value. How do you
work around this issue?

For compiler and OS related variables, it is safe to use a cache file as
these values are the same for all modules. Only a software
install/uninstall might make the cache invalid. The patch you propose is
still fine as the warnings will not change unless you change the compiler.