Re: [PATCH v2 1/3] autoconf: prefer an unrolled loop for trivial AC_CHECK_FUNCS

[Eric Blake]

On 10/31/2016 12:08 PM, Paolo Bonzini wrote:
> An unrolled loop avoids the cost of spawning sed in AS_TR_SH and
> AS_TR_CPP.  Prefer it if there is nothing in the second and third
> argument of AC_CHECK_FUNCS and the first argument is a literal.
> Modify AC_CHECK_FUNCS_ONCE to avoid the variable indirection too.
> 
> * lib/autoconf/functions.m4 (AC_CHECK_FUNCS): Unroll loop if safe.
> (_AC_CHECK_FUNCS): Move basic implementation here.
> (_AC_CHECK_FUNC_ONCE): Expand AC_CHECK_FUNCS here...
> (_AC_FUNCS_EXPANSION): ... and not here, so remove.
> 

> 
>  # Check for a single FUNCTION once.
>  m4_define([_AC_CHECK_FUNC_ONCE],
> -[_AH_CHECK_FUNC([$1])AC_DEFUN([_AC_Func_$1],

This one made me do a double-take - you are no longer calling
_AH_CHECK_FUNC(), which can be essential to getting the correct template
into config.h.  But then I realized that in the old code, you were
calling AC_CHECK_FUNC($shell_var), while the new code is calling
AC_CHECK_FUNC(literal); and AC_CHECK_FUNC() also takes care of calling
_AH_CHECK_FUNC (but only for literals).  So removing it is correct.

>  # AC_CHECK_FUNCS_ONCE(FUNCTION...)
>  # --------------------------------
> @@ -107,13 +111,6 @@ _AC_FUNCS_EXPANSION])AC_REQUIRE([_AC_Func_$1])])
>  AC_DEFUN([AC_CHECK_FUNCS_ONCE],
>  [m4_map_args_w([$1], [_AC_CHECK_FUNC_ONCE(], [)])])
>  
> -m4_define([_AC_FUNCS_EXPANSION],
> -[
> -  m4_divert_text([DEFAULTS], [ac_func_list=])
> -  AC_CHECK_FUNCS([$ac_func_list])
> -  m4_define([_AC_FUNCS_EXPANSION], [])

The old code used to check for ALL functions across any
AC_CHECK_FUNC_ONCE() calls in a single loop up front; the new code
scatters the checks into the first place any given func is encountered
in an AC_CHECK_FUNC_ONCE macro.  This is a subtle semantic change, and
the up-front checking is behavior that we documented:

> @defmac AC_CHECK_FUNCS_ONCE (@address@hidden)
> @acindex{CHECK_FUNCS_ONCE}
> @cvindex address@hidden
> For each @var{function} enumerated in the blank-or-newline-separated argument
> list, define @address@hidden (in all capitals) if it is available.
> This is a once-only variant of @code{AC_CHECK_FUNCS}.  It generates the
> checking code at most once, so that @command{configure} is smaller and
> faster; but the checks cannot be conditionalized and are always done once,
> early during the @command{configure} run.

Our use of AC_REQUIRE hoists the check outside of any AS_IF or similar
code.  However, while I don't think any well-written configure.ac script
will be checking $ac_cv_func_foo prior to calling
AC_CHECK_FUNC_ONCE(foo), I _am_ a bit worried that poorly written
scripts that do:

if condition
  AC_CHECK_FUNC_ONCE(foo)
fi
test $ac_cv_func_foo

instead of

AS_IF([condition], [AC_CHECK_FUNC_ONCE(foo)])
test $ac_cv_func_foo

will now fail when condition fails, because ac_cv_func_foo is no longer
set early and the unrolled version is not encountered, where they used
to succeed regardless of the result of condition.

Potential solution: collect the list of AC_CHECK_FUNC_ONCE functions in
an m4 list, and unroll that list where we used to do the AS_FOR, so that
we aren't changing the semantics of hoisting all the checks up front
early during configure.  I'm playing with the idea now...

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature