Re: [PATCH 2/2] configure: Check -falign-jumps=1 -falign-functions=1 beside -falign-loops=1

[Daniel Kiper]

On Wed, Aug 25, 2021 at 09:34:41AM -0700, Fangrui Song wrote:
> On 2021-08-25, Daniel Kiper wrote:
> > On Mon, Aug 23, 2021 at 11:18:38PM -0700, Fangrui Song wrote:
> > > Clang does not support -falign-jumps and only recently gained support
> > > for -falign-loops. Test all the alignment options to avoid passing
> > > unrecognized options to Clang:
> > >
> > >     clang-14: error: optimization flag '-falign-jumps=1' is not supported 
> > > [-Werror,-Wignored-optimization-argument]
> > >
> > > When compiled with older Clang, the alignment options are not added, so
> > > just avoid the complexity for each option.
> > >
> > > Signed-off-by: Fangrui Song <maskray@google.com>
> > > ---
> > >  configure.ac | 5 +++--
> > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/configure.ac b/configure.ac
> > > index 9a12151bd..8625e6776 100644
> > > --- a/configure.ac
> > > +++ b/configure.ac
> > > @@ -798,8 +798,9 @@ fi
> > >
> > >  # Force no alignment to save space on i386.
> > >  if test "x$target_cpu" = xi386; then
> > > -  AC_CACHE_CHECK([whether -falign-loops works], 
> > > [grub_cv_cc_falign_loop], [
> > > -    CFLAGS="$TARGET_CFLAGS -falign-loops=1 -Werror"
> > > +  AC_CACHE_CHECK([whether -falign-jumps=1 -falign-loops=1 
> > > -falign-functions=1 works],
> > > +    [grub_cv_cc_falign_loop], [
> > > +    CFLAGS="$TARGET_CFLAGS -falign-jumps=1 -falign-loops=1 
> > > -falign-functions=1 -Werror"
> >
> > I would prefer if you add a separate check for every -falign-* option.
> >
> > Daniel
>
> The attached patch checks both -falign-jumps=1 and -falign-loops=1

> From fbef8198d26babeb402b480d333a4a5212069afb Mon Sep 17 00:00:00 2001
> From: Fangrui Song <maskray@google.com>
> Date: Wed, 25 Aug 2021 09:28:46 -0700
> Subject: [PATCH] configure: Check -falign-jumps=1 beside -falign-loops=1
>
> Clang does not support -falign-jumps and only recently gained support
> for -falign-loops. -falign-jumps=1 should be tested beside
> -fliang-loops=1 to avoid passing unrecognized options to Clang:
>
>     clang-14: error: optimization flag '-falign-jumps=1' is not supported 
> [-Werror,-Wignored-optimization-argument]
>
> -falign-functions=1 is supported by GCC 5.1.0/Clang 3.8.0, so just add
> the option unconditionally.
>
> Signed-off-by: Fangrui Song <maskray@google.com>
> ---
>  configure.ac | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/configure.ac b/configure.ac
> index 9a12151bd..91a6ef128 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -798,6 +798,8 @@ fi
>
>  # Force no alignment to save space on i386.
>  if test "x$target_cpu" = xi386; then
> +  TARGET_CFLAGS="$TARGET_CFLAGS -falign-functions=1"
> +
>    AC_CACHE_CHECK([whether -falign-loops works], [grub_cv_cc_falign_loop], [
>      CFLAGS="$TARGET_CFLAGS -falign-loops=1 -Werror"
>      AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[]], [[]])],
> @@ -806,7 +808,18 @@ if test "x$target_cpu" = xi386; then
>    ])
>
>    if test "x$grub_cv_cc_falign_loop" = xyes; then
> -    TARGET_CFLAGS="$TARGET_CFLAGS -falign-jumps=1 -falign-loops=1 
> -falign-functions=1"
> +    TARGET_CFLAGS="$TARGET_CFLAGS -falign-loops=1"
> +  fi
> +
> +  AC_CACHE_CHECK([whether -falign-jumps works], [grub_cv_cc_falign_jumps], [
> +    CFLAGS="$TARGET_CFLAGS -falign-jumps=1 -Werror"
> +    AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[]], [[]])],
> +        [grub_cv_cc_falign_jumps=yes],
> +     [grub_cv_cc_falign_jumps=no])

It seems to me formatting is broken somehow here. It should be the same
like for -falign-loops detection.

Otherwise LGTM...

Please repost both patches using "git send-email" as v3.

You can add Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com> and
Paul's Acked-by to the patches before posting. If there are no
objections I will merge them and other reviewed patches next week...

Daniel