diff mbox series

[FFmpeg-devel] configure: add LTO optarg

Message ID 20230323134628.88073-1-jdek@itanimul.li
State New
Headers show
Series [FFmpeg-devel] configure: add LTO optarg | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

J. Dekker March 23, 2023, 1:46 p.m. UTC
This allows users to call with './configure --enable-lto' (or more
explicitly './configure --enable-lto=full') for the old functionality or
specify './configure --enable-lto=thin' to use ThinLTO.

Signed-off-by: J. Dekker <jdek@itanimul.li>
---
 configure | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

Comments

Martin Storsjö March 23, 2023, 1:56 p.m. UTC | #1
On Thu, 23 Mar 2023, J. Dekker wrote:

> This allows users to call with './configure --enable-lto' (or more
> explicitly './configure --enable-lto=full') for the old functionality or
> specify './configure --enable-lto=thin' to use ThinLTO.
>
> Signed-off-by: J. Dekker <jdek@itanimul.li>
> ---
> configure | 23 ++++++++++++++---------
> 1 file changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/configure b/configure
> index 8980cec7ee..b65001d39d 100755
> --- a/configure
> +++ b/configure
> @@ -412,7 +412,7 @@ Toolchain options:
>   --build-suffix=SUFFIX    library name suffix []
>   --enable-pic             build position-independent code
>   --enable-thumb           compile for Thumb instruction set
> -  --enable-lto             use link-time optimization
> +  --enable-lto[=arg]       use link-time optimization
>   --env="ENV=override"     override the environment variables
>
> Advanced options (experts only):
> @@ -2524,7 +2524,6 @@ CMDLINE_SELECT="
>     debug
>     extra_warnings
>     logging
> -    lto
>     optimizations
>     rpath
>     stripping
> @@ -4170,6 +4169,12 @@ for opt do
>         --enable-sdl)
>             enable sdl2
>         ;;
> +        --enable-lto)
> +            lto=full
> +        ;;
> +        --enable-lto=*)
> +            lto="$optval"
> +        ;;
>         --enable-*=*|--disable-*=*)
>             eval $(echo "${opt%%=*}" | sed 's/--/action=/;s/-/ thing=/')
>             is_in "${thing}s" $COMPONENT_LIST || die_unknown "$opt"
> @@ -4639,7 +4644,7 @@ icl_flags(){
>             # on Windows, does enable remarks so disable them here.
>             -Wall)                echo $flag -Qdiag-disable:remark ;;
>             -std=c99)             echo -Qstd=c99 ;;
> -            -flto)                echo -ipo ;;
> +            -flto*)               echo -ipo ;;
>         esac
>     done
> }
> @@ -4647,7 +4652,7 @@ icl_flags(){
> icc_flags(){
>     for flag; do
>         case $flag in
> -            -flto)                echo -ipo ;;
> +            -flto*)               echo -ipo ;;
>             *)                    echo $flag ;;
>         esac
>     done
> @@ -7182,17 +7187,17 @@ fi
>
> check_optflags(){
>     check_cflags "$@"
> -    enabled lto && check_ldflags "$@"
> +    [ -z "$lto" ] || check_ldflags "$@"

With [ -n "$lto" ] you could retain the old logic with && instead of ||, 
although I don't think it matters much.

Also, this extra check when LTO is enabled, mirrors kinda what we've seen 
that we could need in meson, if LTO would be enabled surprisingly there.

> }
>
> check_optflags $optflags
> check_optflags -fno-math-errno
> check_optflags -fno-signed-zeros
>
> -if enabled lto; then
> +if [ ! -z "$lto" ]; then

This can be [ -n "$lto" ] to avoid the negation.

>     test "$cc_type" != "$ld_type" && die "LTO requires same compiler and linker"
> -    check_cflags  -flto
> -    check_ldflags -flto $cpuflags
> +    check_cflags  -flto=$lto
> +    check_ldflags -flto=$lto $cpuflags

Does GCC support -flto=full too, and is that the same thing as just -flto? 
Or should we stick to just passing -flto to the compiler without any 
argument if the user configured with plain --enable-lto?

>     disable inline_asm_direct_symbol_refs
> fi
>
> @@ -7223,7 +7228,7 @@ if enabled icc; then
>     # icc 11.0 and 11.1 work with ebp_available, but don't pass the test
>     enable ebp_available
>     # The test above does not test linking
> -    enabled lto && disable symver_asm_label
> +    [ -z "$lto" ] || disable symver_asm_label

[ -n "$lto" ] would retain the previous code structure, making the patch 
even clearer.

// Martin
J. Dekker March 23, 2023, 2:16 p.m. UTC | #2
On 23 Mar 2023, at 14:56, Martin Storsjö wrote:
[...]
>>
>> @@ -4647,7 +4652,7 @@ icl_flags(){
>> icc_flags(){
>>     for flag; do
>>         case $flag in
>> -            -flto)                echo -ipo ;;
>> +            -flto*)               echo -ipo ;;
>>             *)                    echo $flag ;;
>>         esac
>>     done
>> @@ -7182,17 +7187,17 @@ fi
>>
>> check_optflags(){
>>     check_cflags "$@"
>> -    enabled lto && check_ldflags "$@"
>> +    [ -z "$lto" ] || check_ldflags "$@"
>
> With [ -n "$lto" ] you could retain the old logic with && instead of ||, although I don't think it matters much.
>
> Also, this extra check when LTO is enabled, mirrors kinda what we've seen that we could need in meson, if LTO would be enabled surprisingly there.

Since not all checks are working with only cflags checks.

>> }
>>
>> check_optflags $optflags
>> check_optflags -fno-math-errno
>> check_optflags -fno-signed-zeros
>>
>> -if enabled lto; then
>> +if [ ! -z "$lto" ]; then
>
> This can be [ -n "$lto" ] to avoid the negation.

Thanks.

>>     test "$cc_type" != "$ld_type" && die "LTO requires same compiler and linker"
>> -    check_cflags  -flto
>> -    check_ldflags -flto $cpuflags
>> +    check_cflags  -flto=$lto
>> +    check_ldflags -flto=$lto $cpuflags
>
> Does GCC support -flto=full too, and is that the same thing as just -flto? Or should we stick to just passing -flto to the compiler without any argument if the user configured with plain --enable-lto?

No, actually checking it. We might benefit from gcc's -ffat-lto-objects and/or -fwhole-program which is maybe more similar to LLVM's -flto=full. Seems like there is non-trivial mapping between LTO features.

>>     disable inline_asm_direct_symbol_refs
>> fi
>>
>> @@ -7223,7 +7228,7 @@ if enabled icc; then
>>     # icc 11.0 and 11.1 work with ebp_available, but don't pass the test
>>     enable ebp_available
>>     # The test above does not test linking
>> -    enabled lto && disable symver_asm_label
>> +    [ -z "$lto" ] || disable symver_asm_label
>
> [ -n "$lto" ] would retain the previous code structure, making the patch even clearer.
>
Will do.
psykose March 23, 2023, 11:38 p.m. UTC | #3
On Thu Mar 23, 2023 at 3:16 PM CET, J. Dekker wrote:
> On 23 Mar 2023, at 14:56, Martin Storsjö wrote:
> >>     test "$cc_type" != "$ld_type" && die "LTO requires same compiler and linker"
> >> -    check_cflags  -flto
> >> -    check_ldflags -flto $cpuflags
> >> +    check_cflags  -flto=$lto
> >> +    check_ldflags -flto=$lto $cpuflags
> >
> > Does GCC support -flto=full too, and is that the same thing as just -flto? Or should we stick to just passing -flto to the compiler without any argument if the user configured with plain --enable-lto?
>
> No, actually checking it. We might benefit from gcc's -ffat-lto-objects and/or -fwhole-program which is maybe more similar to LLVM's -flto=full. Seems like there is non-trivial mapping between LTO features.

-ffat-lto-objects is only about keeping non-lto object code as well as the
intermediate (lto) code in the .o objects.

this is 'required' if one wants to then later link the resulting .a's *without*
using lto, as having only the 'intermediate' output forces using lto during the
link step.

by default for gcc, the only difference between -flto and -flto=[n]/auto is the
amount of threads used during linking. =auto is generally preferred, since under
the gnu make jobserver it controls the jobs instead (or defaults to nproc). the
existing `-flto` forces 1-thread link under gcc, and it prints a warning for
this:

 lto-wrapper: warning: using serial compilation of 12 LTRANS jobs
 lto-wrapper: note: see the '-flto' option documentation for more information

(the gcc(1) manpage documents this quite well.)

however, i'd say the existing behaviour here is fine. there are indeed a lot of
toolchain-specifics between all this- and people building that care about this
would usually know better than a 'magic' configure script that tries to learn
all of it. simply respecting -flto[=arg] with a user-supplied arg is all that is
generally needed.

as for -fwhole-program, i'm not sure it has any use:

 With -flto this option has a limited use. In most cases the precise list of
 symbols used or exported from the binary is known the resolution info passed to
 the link-time optimizer by the linker plugin. It is still useful if no linker
 plugin is used or during incremental link step when final code is produced (with
 -flto -flinker-output=nolto-rel).

(by default, the linker plugin is automatically used for modern-ish gcc (since
maybe 4.9?))
diff mbox series

Patch

diff --git a/configure b/configure
index 8980cec7ee..b65001d39d 100755
--- a/configure
+++ b/configure
@@ -412,7 +412,7 @@  Toolchain options:
   --build-suffix=SUFFIX    library name suffix []
   --enable-pic             build position-independent code
   --enable-thumb           compile for Thumb instruction set
-  --enable-lto             use link-time optimization
+  --enable-lto[=arg]       use link-time optimization
   --env="ENV=override"     override the environment variables
 
 Advanced options (experts only):
@@ -2524,7 +2524,6 @@  CMDLINE_SELECT="
     debug
     extra_warnings
     logging
-    lto
     optimizations
     rpath
     stripping
@@ -4170,6 +4169,12 @@  for opt do
         --enable-sdl)
             enable sdl2
         ;;
+        --enable-lto)
+            lto=full
+        ;;
+        --enable-lto=*)
+            lto="$optval"
+        ;;
         --enable-*=*|--disable-*=*)
             eval $(echo "${opt%%=*}" | sed 's/--/action=/;s/-/ thing=/')
             is_in "${thing}s" $COMPONENT_LIST || die_unknown "$opt"
@@ -4639,7 +4644,7 @@  icl_flags(){
             # on Windows, does enable remarks so disable them here.
             -Wall)                echo $flag -Qdiag-disable:remark ;;
             -std=c99)             echo -Qstd=c99 ;;
-            -flto)                echo -ipo ;;
+            -flto*)               echo -ipo ;;
         esac
     done
 }
@@ -4647,7 +4652,7 @@  icl_flags(){
 icc_flags(){
     for flag; do
         case $flag in
-            -flto)                echo -ipo ;;
+            -flto*)               echo -ipo ;;
             *)                    echo $flag ;;
         esac
     done
@@ -7182,17 +7187,17 @@  fi
 
 check_optflags(){
     check_cflags "$@"
-    enabled lto && check_ldflags "$@"
+    [ -z "$lto" ] || check_ldflags "$@"
 }
 
 check_optflags $optflags
 check_optflags -fno-math-errno
 check_optflags -fno-signed-zeros
 
-if enabled lto; then
+if [ ! -z "$lto" ]; then
     test "$cc_type" != "$ld_type" && die "LTO requires same compiler and linker"
-    check_cflags  -flto
-    check_ldflags -flto $cpuflags
+    check_cflags  -flto=$lto
+    check_ldflags -flto=$lto $cpuflags
     disable inline_asm_direct_symbol_refs
 fi
 
@@ -7223,7 +7228,7 @@  if enabled icc; then
     # icc 11.0 and 11.1 work with ebp_available, but don't pass the test
     enable ebp_available
     # The test above does not test linking
-    enabled lto && disable symver_asm_label
+    [ -z "$lto" ] || disable symver_asm_label
     if enabled x86_32; then
         icc_version=$($cc -dumpversion)
         test ${icc_version%%.*} -ge 11 &&