On Sat, Jan 18, 2014 at 2:24 AM, Uros Bizjak <ubiz...@gmail.com> wrote:
> On Mon, Dec 23, 2013 at 3:14 PM, H.J. Lu <hjl.to...@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...@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:
---
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 89669bd..62dcca7 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,5 +1,12 @@
2014-01-18 H.J. Lu <hongjiu...@intel.com>
+ * config.gcc (x86_archs): Use sed to make sure that each
+ processor is separated by exactly one space.
+ (x86_64_archs): Likewise.
+ (x86_cpus): Likewise.
+
+2014-01-18 H.J. Lu <hongjiu...@intel.com>
+
PR bootstrap/59580
PR bootstrap/59583
* config.gcc (x86_archs): New variable.
diff --git a/gcc/config.gcc b/gcc/config.gcc
index c3124be..11e948a 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -596,23 +596,27 @@ 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.
+# 32-bit x86 processors supported by --with-arch=.
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"
+ 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"
+# Make sure that each processor is separated by exactly one space.
+x86_archs=`echo $x86_archs | sed -e "s/[ ]+/ /g"`
-# 64-bit x86 processors supported by --with-arch=. Each processor
-# MUST be separated by exactly one space.
+# 64-bit x86 processors supported by --with-arch=.
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"
+ 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"
+# Make sure that each processor is separated by exactly one space.
+x86_64_archs=`echo $x86_64_archs | sed -e "s/[ ]+/ /g"`
-# Additional x86 processors supported by --with-cpu=. Each processor
-# MUST be separated by exactly one space.
+# Additional x86 processors supported by --with-cpu=.
x86_cpus="generic intel"
+# Make sure that each processor is separated by exactly one space.
+x86_cpus=`echo $x86_cpus | sed -e "s/[ ]+/ /g"`
# Common parts for widely ported systems.
case ${target} in
----
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.
--
H.J.
