diff mbox

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

Message ID 20131223141410.GA1684@gmail.com
State New
Headers show

Commit Message

H.J. Lu Dec. 23, 2013, 2:14 p.m. UTC 
On Sun, Dec 22, 2013 at 11:11:12PM +0100, Uros Bizjak 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}.

Comments

Uros Bizjak Jan. 18, 2014, 10:24 a.m. UTC | #1 
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
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"
diff mbox

Patch

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"
+# 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"
+# Additional x86 processors supported by --with-cpu=.  Each processor
+# MUST be separated by exactly one space.
+x86_cpus="generic intel"
+
 # Common parts for widely ported systems.
 case ${target} in
 *-*-darwin*)
@@ -1392,20 +1408,21 @@  i[34567]86-*-linux* | i[34567]86-*-kfreebsd*-gnu | i[34567]86-*-knetbsd*-gnu | i
 			done
 			TM_MULTILIB_CONFIG=`echo $TM_MULTILIB_CONFIG | sed 's/^,//'`
 			need_64bit_isa=yes
-			case X"${with_cpu}" in
-			Xgeneric|Xintel|Xatom|Xslm|Xcore2|Xcorei7|Xcorei7-avx|Xnocona|Xx86-64|Xbdver4|Xbdver3|Xbdver2|Xbdver1|Xbtver2|Xbtver1|Xamdfam10|Xbarcelona|Xk8|Xopteron|Xathlon64|Xathlon-fx|Xathlon64-sse3|Xk8-sse3|Xopteron-sse3)			
-				;;
-			X)
+			if test x$with_cpu = x; then
 				if test x$with_cpu_64 = x; then
 					with_cpu_64=generic
 				fi
-				;;
-			*)
-				echo "Unsupported CPU used in --with-cpu=$with_cpu, supported values:" 1>&2
-				echo "generic intel atom slm core2 corei7 corei7-avx nocona x86-64 bdver4 bdver3 bdver2 bdver1 btver2 btver1 amdfam10 barcelona k8 opteron athlon64 athlon-fx athlon64-sse3 k8-sse3 opteron-sse3" 1>&2
-				exit 1
-				;;
-			esac
+			else
+				case " $x86_cpus $x86_archs $x86_64_archs " in
+				*" $with_cpu "*)
+					;;
+				*)
+					echo "Unsupported CPU used in --with-cpu=$with_cpu, supported values:" 1>&2
+					echo "$x86_cpus $x86_archs $x86_64_archs " 1>&2
+					exit 1
+					;;
+				esac
+			fi
 		else
 			tm_file="${tm_file} i386/gnu-user-common.h i386/gnu-user.h i386/linux-common.h i386/linux.h"
 		fi
@@ -1514,20 +1531,21 @@  i[34567]86-*-solaris2* | x86_64-*-solaris2.1[0-9]*)
 		tm_defines="${tm_defines} TARGET_BI_ARCH=1"
 		tmake_file="$tmake_file i386/t-sol2-64"
 		need_64bit_isa=yes
-		case X"${with_cpu}" in
-		Xgeneric|Xintel|Xatom|Xslm|Xcore2|Xcorei7|Xcorei7-avx|Xnocona|Xx86-64|Xbdver4|Xbdver3|Xbdver2|Xbdver1|Xbtver2|Xbtver1|Xamdfam10|Xbarcelona|Xk8|Xopteron|Xathlon64|Xathlon-fx|Xathlon64-sse3|Xk8-sse3|Xopteron-sse3)
-			;;
-		X)
+		if test x$with_cpu = x; then
 			if test x$with_cpu_64 = x; then
 				with_cpu_64=generic
 			fi
-			;;
-		*)
-			echo "Unsupported CPU used in --with-cpu=$with_cpu, supported values:" 1>&2
-			echo "generic intel atom slm core2 corei7 corei7-avx nocona x86-64 bdver4 bdver3 bdver2 bdver1 btver2 btver1 amdfam10 barcelona k8 opteron athlon64 athlon-fx athlon64-sse3 k8-sse3 opteron-sse3" 1>&2
-			exit 1
-			;;
-		esac
+		else
+			case " $x86_cpus $x86_archs $x86_64_archs " in
+			*" $with_cpu "*)
+				;;
+			*)
+				echo "Unsupported CPU used in --with-cpu=$with_cpu, supported values:" 1>&2
+				echo "$x86_cpus $x86_archs $x86_64_archs" 1>&2
+				exit 1
+				;;
+			esac
+		fi
 		;;
 	esac
 	;;
@@ -1599,20 +1617,21 @@  i[34567]86-*-mingw* | x86_64-*-mingw*)
 			tm_file="${tm_file} i386/mingw-w64.h"
 			if test x$enable_targets = xall; then
 				tm_defines="${tm_defines} TARGET_BI_ARCH=1"
-				case X"${with_cpu}" in
-				Xgeneric|Xintel|Xatom|Xslm|Xcore2|Xcorei7|Xcorei7-avx|Xnocona|Xx86-64|Xbdver4|Xbdver3|Xbdver2|Xbdver1|Xbtver2|Xbtver1|Xamdfam10|Xbarcelona|Xk8|Xopteron|Xathlon64|Xathlon-fx|Xathlon64-sse3|Xk8-sse3|Xopteron-sse3)
-					;;
-				X)
+				if test x$with_cpu = x; then
 					if test x$with_cpu_64 = x; then
 						with_cpu_64=generic
 					fi
-					;;
-				*)
-					echo "Unsupported CPU used in --with-cpu=$with_cpu, supported values:" 1>&2
-					echo "generic intel atom slm core2 corei7 Xcorei7-avx nocona x86-64 bdver4 bdver3 bdver2 bdver1 btver2 btver1 amdfam10 barcelona k8 opteron athlon64 athlon-fx athlon64-sse3 k8-sse3 opteron-sse3" 1>&2
-					exit 1
-					;;
-				esac
+				else
+					case " $x86_cpus $x86_archs $x86_64_archs " in
+					*" $with_cpu "*)
+						;;
+					*)
+						echo "Unsupported CPU used in --with-cpu=$with_cpu, supported values:" 1>&2
+						echo "$x86_cpus $x86_archs $x86_64_archs" 1>&2
+						exit 1
+						;;
+					esac
+				fi
 			fi
 			;;
 		*)
@@ -3650,13 +3669,8 @@  case "${target}" in
 		supported_defaults="abi arch arch_32 arch_64 cpu cpu_32 cpu_64 tune tune_32 tune_64"
 		for which in arch arch_32 arch_64 cpu cpu_32 cpu_64 tune tune_32 tune_64; do
 			eval "val=\$with_$which"
-			case ${val} in
-			i386 | i486 \
-			| i586 | pentium | pentium-mmx | winchip-c6 | winchip2 \
-			| c3 | c3-2 | i686 | pentiumpro | pentium2 | pentium3 \
-			| pentium4 | k6 | k6-2 | k6-3 | athlon | athlon-tbird \
-			| athlon-4 | athlon-xp | athlon-mp | geode \
-			| prescott | pentium-m | pentium4m | pentium3m)
+			case " $x86_archs " in
+			*" ${val} "*)
 				case "${target}" in
 				  x86_64-*-*)
 				      case "x$which" in
@@ -3671,17 +3685,34 @@  case "${target}" in
 				esac
 				# OK
 				;;
-			"" | x86-64 | generic | intel | native \
-			| k8 | k8-sse3 | athlon64 | athlon64-sse3 | opteron \
-			| opteron-sse3 | athlon-fx | bdver4 | bdver3 | bdver2 \
-			| bdver1 | btver2 |  btver1 | amdfam10 | barcelona \
-			| nocona | core2 | corei7 | corei7-avx | core-avx-i \
-			| core-avx2 | broadwell | atom | slm)
-				# OK
-				;;
 			*)
-				echo "Unknown CPU given in --with-$which=$val." 1>&2
-				exit 1
+				if test x${val} != x; then
+					case " $x86_64_archs " in
+					*" ${val} "*)
+						# OK
+						;;
+					*)
+						# Allow $x86_cpus --with-cpu=/--with-tune=
+						case "x$which" in
+						xcpu*|xtune*)
+							case " $x86_cpus " in
+							*" ${val} "*)
+								# OK
+								;;
+							*)
+								echo "Unknown CPU given in --with-$which=$val." 1>&2
+								exit 1
+								;;
+							esac
+							;;
+						*)
+							echo "Unknown CPU given in --with-$which=$val." 1>&2
+							exit 1
+							;;
+						esac
+					;;
+					esac
+				fi
 				;;
 			esac
 		done