PATCH: PRs bootstrap/59580/59583: Improve x86 --with-arch/--with-cpu= configure handling

On Sat, Jan 18, 2014 at 2:24 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Mon, Dec 23, 2013 at 3:14 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>
>>> Please get someone to review config.gcc changes. They are OK as far as
>>> x86 rename is concerned, but I can't review functional changes.
>>
>> Hi Paolo,
>>
>> Can you review this config.gcc change?
>>
>>>
>>> > @@ -588,6 +588,22 @@ esac
>>> >  # Common C libraries.
>>> >  tm_defines="$tm_defines LIBC_GLIBC=1 LIBC_UCLIBC=2 LIBC_BIONIC=3"
>>> >
>>> > +# 32-bit x86 processors supported by --with-arch=.  Each processor
>>> > +# MUST be separated by exactly one space.
>>> > +x86_archs="athlon athlon-4 athlon-fx athlon-mp athlon-tbird \
>>> > +athlon-xp k6 k6-2 k6-3 geode c3 c3-2 winchip-c6 winchip2 i386 i486 \
>>> > +i586 i686 pentium pentium-m pentium-mmx pentium2 pentium3 pentium3m \
>>> > +pentium4 pentium4m pentiumpro prescott"
>>>
>>> Missing "native".
>>
>> x86_archs contains 32-bit x86 processors.  "native" is allowed for
>> 64-bit targets and is included in x86_64_archs.  64-bit processors
>> can be used in --with-arch/--with-cpu= for 32-bit targets.
>>
>> Here is a patch to improve x86 x86 --with-arch/--with-cpu= configure
>> handling.  This patch defines 3 variables:
>>
>> 1. x86_archs: It contains 32-bit x86 processors supported by
>> --with-arch=, which aren't allowed for 64-bit targets.
>> 2. x86_64_archs: It contains 64-bit x86 processors supported by
>> --with-arch=, which are allowed for both 32-bit and 64-bit targets.
>> 3. x86_cpus.  It contains x86 processors supported by --with-cpu=,
>> which are allowed for both 32-bit and 64-bit targets.
>>
>> Each processor in those 3 variables are separated by exactly one space.
>>
>> Instead of checking if a value of --with-arch/--with-cpu= is valid in many
>> difference places with
>>
>> case ${val} in
>> valid pattern list)
>>   OK
>>   ;;
>> *)
>>   error
>>   exit 1
>>   ;;
>> esac
>>
>> and updating all pattern lists when adding a new processor, this patch
>> uses
>>
>> case " valid processor list separated by exactly one space " in
>> *" ${val} "*)
>>   OK
>>   ;;
>> *)
>>   error
>>   exit 1
>>   ;;
>> esac
>>
>> "valid processor list separated by exactly one space" is combination
>> of 3 processor variables.  It only needs separate a check for empty
>> value with
>>
>> if test x${val} != x; then
>>   $val isn't empty
>> else
>>   $val is empty
>> fi
>>
>> With this approach, we only need to add new 32-bit processors to x86_archs
>> and new 64-bit processors to x86_64_archs.  They will be supported by
>> --with-arch/--with-cpu= automatically.  OK to install?
>>
>> Thanks.
>>
>>
>> H.J.
>> ---
>> 2013-12-23   H.J. Lu  <hongjiu.lu@intel.com>
>>
>>         PR bootstrap/59580
>>         PR bootstrap/59583
>>         * config.gcc (x86_archs): New variable.
>>         (x86_64_archs): Likewise.
>>         (x86_cpus): Likewise.
>>         Use $x86_archs, $x86_64_archs and $x86_cpus to check valid
>>         --with-arch/--with-cpu= options.
>>         Support --with-arch=/--with-cpu={nehalem,westmere,
>>         sandybridge,ivybridge,haswell,broadwell,bonnell,silvermont}.
>
> Probably we can use some regex for whitespace to relax "MUST be
> separated by exacly one space" limitation, but nevertheless the patch

Here is a patch to remove this limitation:

---
----

I am tested for both Linux/x86 and Linux/x86-64 with different
configurations.  OK to install?

> looks like a much needed cleanup to me. OTOH, the comment clearly says
> what to do when changing strings.
>
> Under the assumption that build maintainers silently agree, and due to
> the fact that the patch touches x86 parts only, the patch is OK for
> mainline with a couple of nits below.
>
> Thanks,
> Uros.
>
>> diff --git a/gcc/config.gcc b/gcc/config.gcc
>> index 24dbaf9..51eb2b1 100644
>> --- a/gcc/config.gcc
>> +++ b/gcc/config.gcc
>> @@ -588,6 +588,22 @@ esac
>>  # Common C libraries.
>>  tm_defines="$tm_defines LIBC_GLIBC=1 LIBC_UCLIBC=2 LIBC_BIONIC=3"
>>
>> +# 32-bit x86 processors supported by --with-arch=.  Each processor
>> +# MUST be separated by exactly one space.
>> +x86_archs="athlon athlon-4 athlon-fx athlon-mp athlon-tbird \
>> +athlon-xp k6 k6-2 k6-3 geode c3 c3-2 winchip-c6 winchip2 i386 i486 \
>> +i586 i686 pentium pentium-m pentium-mmx pentium2 pentium3 pentium3m \
>> +pentium4 pentium4m pentiumpro prescott"
>
> Please put some vertical space here...
>
>> +# 64-bit x86 processors supported by --with-arch=.  Each processor
>> +# MUST be separated by exactly one space.
>> +x86_64_archs="amdfam10 athlon64 athlon64-sse3 barcelona bdver1 bdver2 \
>> +bdver3 bdver4 btver1 btver2 k8 k8-sse3 opteron opteron-sse3 nocona \
>> +core2 corei7 corei7-avx core-avx-i core-avx2 atom slm nehalem westmere \
>> +sandybridge ivybridge haswell broadwell bonnell silvermont x86-64 native"
>
> ... and here.
>
>> +# Additional x86 processors supported by --with-cpu=.  Each processor
>> +# MUST be separated by exactly one space.
>> +x86_cpus="generic intel"

Done.  I checked my updated initial patch in.

Thanks.