On Wed, 9 Jun 2021 14:35:01 +0300
Claudiu Zissulescu <claz...@gmail.com> wrote:

> > ISTM you only set the expected flags in the switch so i would have
> > set only that variable and have grepped only once after the switch for
> > brevity.  
> 
> ARC has various FPU extensions, some of them are common to EM and HS 
> architectures, others are specific for only one of them. Hence, the grep 
> commands are ensuring that we accept the right fpu extension for the 
> right ARC architecture.

Right. Which you'd accomplish more terse if you would set flags_ok in
the switch cited below and after the switch have one single grep à la

if [ -n "$flags_ok" ] \
  && ! grep -q -E "^ARC_CPU[[:blank:]]*\($new_cpu,[[:blank:]]*$flags_ok," \
         ${srcdir}/config/arc/arc-cpus.def
then
  echo "Unknown floating point type used in "\
       "--with-fpu=$with_fpu for cpu $new_cpu" 1>&2
  exit 1
fi

The reason is that all case statements in the $with_fpu switch just
differ in the allowed flags AFAICS. But as you prefer.

last comments below.

> diff --git a/gcc/config.gcc b/gcc/config.gcc
> index 13c2004e3c52..09886c8635e0 100644
> --- a/gcc/config.gcc
> +++ b/gcc/config.gcc
> @@ -4258,18 +4258,61 @@ case "${target}" in
>               ;;
>  
>       arc*-*-*)
> -             supported_defaults="cpu"
> +             supported_defaults="cpu fpu"
>  
> +             new_cpu=hs38_linux
>               if [ x"$with_cpu" = x ] \
> -                 || grep "^ARC_CPU ($with_cpu," \
> +                 || grep -E "^ARC_CPU \($with_cpu," \
>                      ${srcdir}/config/arc/arc-cpus.def \
>                      > /dev/null; then

Cosmetics: You may want to keep the non "-E" version but use -q
and drop the redirect to /dev/null.

>                # Ok
> -              true
> +              new_cpu=$with_cpu
>               else
>                echo "Unknown cpu used in --with-cpu=$with_cpu" 1>&2
>                exit 1
>               fi
> +
> +             # see if --with-fpu matches any of the supported FPUs
> +             case "$with_fpu" in
> +             "")
> +                     # OK
> +                     ;;
> +             fpus | fpus_div | fpus_fma | fpus_all)
> +                     # OK if em or hs
> +                     if ! grep -q -E 
> "^ARC_CPU[[:blank:]]*\($new_cpu,[[:space:]]*[emhs]+," \

you changed only the first :space: to :blank: (everywhere)

> +                        ${srcdir}/config/arc/arc-cpus.def
> +                     then
> +                      echo "Unknown floating point type used in "\
> +                      "--with-fpu=$with_fpu for cpu $new_cpu" 1>&2
> +                      exit 1
> +                     fi
> +             ;;
> +             fpuda | fpuda_div | fpuda_fma | fpuda_all)
> +                     # OK only em
> +                     if ! grep -q -E 
> "^ARC_CPU[[:blank:]]*\($new_cpu,[[:space:]]*em," \
> +                        ${srcdir}/config/arc/arc-cpus.def
> +                     then
> +                      echo "Unknown floating point type used in "\
> +                           "--with-fpu=$with_fpu for cpu $new_cpu" 1>&2
> +                      exit 1
> +                     fi
> +                     ;;
> +             fpud | fpud_div | fpud_fma | fpud_all)
> +                     # OK only hs
> +                     if ! grep -q -E 
> "^ARC_CPU[[:blank:]]*\($new_cpu,[[:space:]]*hs," \
> +                        ${srcdir}/config/arc/arc-cpus.def
> +                     then
> +                      echo "Unknown floating point type used in"\
> +                           "--with-fpu=$with_fpu for cpu $new_cpu" 1>&2

missing trailing space after 'in' in "used in"\

LGTM with that fixed with or without cutting down on grep lines,
but of course i cannot approve it.
thanks,

> +                      exit 1
> +                     fi
> +                     ;;
> +             *)
> +                     echo "Unknown floating point type used in "\
> +                          "--with-fpu=$with_fpu" 1>&2
> +                     exit 1
> +                     ;;
> +             esac
>               ;;
>  
>      csky-*-*)

Reply via email to