[PATCHv3] powerpc: Automatic CPU detection in preconfigure

Message ID 20200511221012.293305-1-murphyp@linux.vnet.ibm.com
State Superseded
Headers
Series [PATCHv3] powerpc: Automatic CPU detection in preconfigure |

Commit Message

Paul E. Murphy May 11, 2020, 10:10 p.m. UTC
  Changes for 3:
   * update conditional to avoid running on non-ppc machines

Followup from
<https://sourceware.org/pipermail/libc-alpha/2020-May/113765.html>

Changes:
   * Scan for the first instance of .machine or -mcpu= within the
     generated assembly file.
   * Don't try to be smart when guessing.  Only choose valid targets.
   * Don't make any extra noise
   * Remove comment about trying to pretty print an error message, just
     fail in the usual autoconf way.

Followup from <https://sourceware.org/legacy-ml/libc-alpha/2017-02/msg00164.html>

---8<---

Added a check to detect the CPU value in preconfigure, so that glibc is
built with the correct --with-cpu value.  And move existing checks into
preconfigure.ac.

Co-Authored-By: Carlos Eduardo Seo  <cseo@linux.vnet.ibm.com>
Co-Authored-By: Tulio Magno Quites Machado Filho  <tuliom@linux.vnet.ibm.com>
---
 sysdeps/powerpc/preconfigure    | 62 +++++++++++++++++++++++++++++----
 sysdeps/powerpc/preconfigure.ac | 58 ++++++++++++++++++++++++++++++
 2 files changed, 113 insertions(+), 7 deletions(-)
 create mode 100644 sysdeps/powerpc/preconfigure.ac
  

Comments

Paul E Murphy June 2, 2020, 8:45 p.m. UTC | #1
Ping

On 5/11/20 5:10 PM, Paul E. Murphy via Libc-alpha wrote:
> Changes for 3:
>     * update conditional to avoid running on non-ppc machines
> 
> Followup from
> <https://sourceware.org/pipermail/libc-alpha/2020-May/113765.html>
> 
> Changes:
>     * Scan for the first instance of .machine or -mcpu= within the
>       generated assembly file.
>     * Don't try to be smart when guessing.  Only choose valid targets.
>     * Don't make any extra noise
>     * Remove comment about trying to pretty print an error message, just
>       fail in the usual autoconf way.
> 
> Followup from <https://sourceware.org/legacy-ml/libc-alpha/2017-02/msg00164.html>
> 
> ---8<---
> 
> Added a check to detect the CPU value in preconfigure, so that glibc is
> built with the correct --with-cpu value.  And move existing checks into
> preconfigure.ac.
> 
> Co-Authored-By: Carlos Eduardo Seo  <cseo@linux.vnet.ibm.com>
> Co-Authored-By: Tulio Magno Quites Machado Filho  <tuliom@linux.vnet.ibm.com>
  
Matheus Castanho June 8, 2020, 3:15 p.m. UTC | #2
Hi Paul,

On 5/11/20 7:10 PM, Paul E. Murphy via Libc-alpha wrote:
> Changes for 3:
>    * update conditional to avoid running on non-ppc machines
> 
> Followup from
> <https://sourceware.org/pipermail/libc-alpha/2020-May/113765.html>
> 
> Changes:
>    * Scan for the first instance of .machine or -mcpu= within the
>      generated assembly file.
>    * Don't try to be smart when guessing.  Only choose valid targets.
>    * Don't make any extra noise
>    * Remove comment about trying to pretty print an error message, just
>      fail in the usual autoconf way.
> 
> Followup from <https://sourceware.org/legacy-ml/libc-alpha/2017-02/msg00164.html>
> 
> ---8<---
> 
> Added a check to detect the CPU value in preconfigure, so that glibc is
> built with the correct --with-cpu value.  And move existing checks into
> preconfigure.ac.
> 
> Co-Authored-By: Carlos Eduardo Seo  <cseo@linux.vnet.ibm.com>
> Co-Authored-By: Tulio Magno Quites Machado Filho  <tuliom@linux.vnet.ibm.com>
> ---
>  sysdeps/powerpc/preconfigure    | 62 +++++++++++++++++++++++++++++----
>  sysdeps/powerpc/preconfigure.ac | 58 ++++++++++++++++++++++++++++++
>  2 files changed, 113 insertions(+), 7 deletions(-)
>  create mode 100644 sysdeps/powerpc/preconfigure.ac
> 

[..]

> +# Lets ask the compiler which Power processor we've got, in case the user did
> +# not choose a --with-cpu value.  Scan a trivial generated assembly program
> +# and scrape the first
> +#   .machine <machine>
> +# or
> +#   .ascii "-mcpu=<machine>"
> +# directive which shows up, and try using it.
> +case "${machine}:${submachine}" in
> +*powerpc*:)
> +  archcpu=`echo "int foo () { return 0; }" \
> +	   | $CC $CFLAGS $CPPFLAGS -S -frecord-gcc-switches -xc -o - - \
> +	   | grep -E "mcpu=|[.]machine" -m 1 \
> +	   | sed -e "s/.*machine /mcpu=/" -e "s/.*mcpu=\(.*\)\"/\1/"`

I believe this last pattern is not parsing the machine name correctly
when .machine is used. I found the problem when testing with actual
output from GCC, but below I'm just emulating the problem with a simple
echo.

With .ascii it's OK:

$ echo '.ascii  "-mcpu=power9"' | grep -E "mcpu=|[.]machine" -m 1 | sed
-e "s/.*machine /mcpu=/" -e "s/.*mcpu=\(.*\)\"/\1/"
power9

But when .machine is used:

$ echo '.machine power9' | grep -E "mcpu=|[.]machine" -m 1 | sed -e
"s/.*machine /mcpu=/" -e "s/.*mcpu=\(.*\)\"/\1/"
mcpu=power9

The last pattern expects a closing ", which the .machine case does not
have. You could drop the \" and just remove all quotes prior to invoking
sed:

$ echo '.machine power9' | grep -E "mcpu=|[.]machine" -m 1 | tr -d '"' |
sed -e "s/.*machine /mcpu=/" -e "s/.*mcpu=\(.*\)/\1/"
power9

$ echo '.ascii  "-mcpu=power9"' | grep -E "mcpu=|[.]machine" -m 1 | tr
-d '"' | sed -e "s/.*machine /mcpu=/" -e "s/.*mcpu=\(.*\)/\1/"
power9

Instead of using tr, an extra -e 's/"//g' before the others should also
do the trick.

--
Matheus Castanho
  
Tulio Magno Quites Machado Filho June 8, 2020, 7:07 p.m. UTC | #3
Matheus Castanho via Libc-alpha <libc-alpha@sourceware.org> writes:

> On 5/11/20 7:10 PM, Paul E. Murphy via Libc-alpha wrote:
>> +# Lets ask the compiler which Power processor we've got, in case the user did
>> +# not choose a --with-cpu value.  Scan a trivial generated assembly program
>> +# and scrape the first
>> +#   .machine <machine>
>> +# or
>> +#   .ascii "-mcpu=<machine>"
>> +# directive which shows up, and try using it.
>> +case "${machine}:${submachine}" in
>> +*powerpc*:)
>> +  archcpu=`echo "int foo () { return 0; }" \
>> +	   | $CC $CFLAGS $CPPFLAGS -S -frecord-gcc-switches -xc -o - - \
>> +	   | grep -E "mcpu=|[.]machine" -m 1 \
>> +	   | sed -e "s/.*machine /mcpu=/" -e "s/.*mcpu=\(.*\)\"/\1/"`
>
> I believe this last pattern is not parsing the machine name correctly
> when .machine is used. I found the problem when testing with actual
> output from GCC, but below I'm just emulating the problem with a simple
> echo.

This issue can only be reproduced on GCC >= 10.

I tested this patch on different combinations of distros, GCC versions and
ABIs confirming this works well, except for the issue that Matheus pointed out.
I modified that regex to "s/.*machine //" and it worked.

On a side note, on Debian ppc (32-bit), it does generate the right result, but
it isn't able to select a submachine because Debian's GCC is configured with
--with-cpu=default32, generating a `.machine ppc`.

LGTM with that issue fixed.
  

Patch

diff --git a/sysdeps/powerpc/preconfigure b/sysdeps/powerpc/preconfigure
index a0ea745bb4..0e7920f6de 100644
--- a/sysdeps/powerpc/preconfigure
+++ b/sysdeps/powerpc/preconfigure
@@ -1,4 +1,5 @@ 
-# preconfigure fragment for powerpc.
+# This file is generated from configure.ac by Autoconf.  DO NOT EDIT!
+ # Local preconfigure fragment for sysdeps/powerpc
 
 case "$machine" in
 powerpc64le)
@@ -13,12 +14,59 @@  powerpc*)
   case "$host_os" in
     *gnuspe*)
       # SPE support was dropped in glibc 2.30.
-      # We can't use AC_MSG_ERROR here.
-      # The parent script is in the middle of printing the
-      # "checking for sysdeps preconfigure fragments" line.
-      echo >&2
-      echo "Host system type $host is no longer supported." >&2
-      exit 1
+      as_fn_error $? "Host system type $host is no longer supported." "$LINENO" 5
+    ;;
+  esac
+  ;;
+esac
+
+# Lets ask the compiler which Power processor we've got, in case the user did
+# not choose a --with-cpu value.  Scan a trivial generated assembly program
+# and scrape the first
+#   .machine <machine>
+# or
+#   .ascii "-mcpu=<machine>"
+# directive which shows up, and try using it.
+case "${machine}:${submachine}" in
+*powerpc*:)
+  archcpu=`echo "int foo () { return 0; }" \
+	   | $CC $CFLAGS $CPPFLAGS -S -frecord-gcc-switches -xc -o - - \
+	   | grep -E "mcpu=|.machine" -m 1 \
+	   | sed -e "s/.*machine /mcpu=/" -e "s/.*mcpu=\(.*\)\"/\1/"`
+  # Note if you add patterns here you must ensure that an appropriate
+  # directory exists in sysdeps/powerpc.  Likewise, if we find a
+  # cpu, don't let the generic configure append extra compiler options.
+  case "$archcpu" in
+  405fp|440fp|464fp|476fp)
+    submachine=${archcpu%fp}
+    if ${libc_cv_cc_submachine+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  libc_cv_cc_submachine=""
+fi
+
+    ;;
+  405|440|464|476)
+    submachine=${archcpu}
+    if ${libc_cv_cc_submachine+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  libc_cv_cc_submachine=""
+fi
+
+    ;;
+
+  a2|970|power[4-9]|power5x|power6+)
+    submachine=${archcpu}
+    if ${libc_cv_cc_submachine+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  libc_cv_cc_submachine=""
+fi
+
+    ;;
+  *)
+    # We couldn't figure it out, assume none
     ;;
   esac
   ;;
diff --git a/sysdeps/powerpc/preconfigure.ac b/sysdeps/powerpc/preconfigure.ac
new file mode 100644
index 0000000000..12e9fa5fad
--- /dev/null
+++ b/sysdeps/powerpc/preconfigure.ac
@@ -0,0 +1,58 @@ 
+GLIBC_PROVIDES dnl See aclocal.m4 in the top level source directory.
+# Local preconfigure fragment for sysdeps/powerpc
+
+case "$machine" in
+powerpc64le)
+  base_machine=powerpc machine=powerpc/powerpc64/le
+  ;;
+powerpc64*)
+  base_machine=powerpc machine=powerpc/powerpc64/be
+  ;;
+powerpc*)
+  base_machine=powerpc machine=powerpc/powerpc32
+  with_fp_cond="!defined __NO_FPRS__"
+  case "$host_os" in
+    *gnuspe*)
+      # SPE support was dropped in glibc 2.30.
+      AC_MSG_ERROR([Host system type $host is no longer supported.])
+    ;;
+  esac
+  ;;
+esac
+
+# Lets ask the compiler which Power processor we've got, in case the user did
+# not choose a --with-cpu value.  Scan a trivial generated assembly program
+# and scrape the first
+#   .machine <machine>
+# or
+#   .ascii "-mcpu=<machine>"
+# directive which shows up, and try using it.
+case "${machine}:${submachine}" in
+*powerpc*:)
+  archcpu=`echo "int foo () { return 0; }" \
+	   | $CC $CFLAGS $CPPFLAGS -S -frecord-gcc-switches -xc -o - - \
+	   | grep -E "mcpu=|[.]machine" -m 1 \
+	   | sed -e "s/.*machine /mcpu=/" -e "s/.*mcpu=\(.*\)\"/\1/"`
+  # Note if you add patterns here you must ensure that an appropriate
+  # directory exists in sysdeps/powerpc.  Likewise, if we find a
+  # cpu, don't let the generic configure append extra compiler options.
+  case "$archcpu" in
+  405fp|440fp|464fp|476fp)
+    submachine=${archcpu%fp}
+    AC_CACHE_VAL(libc_cv_cc_submachine,libc_cv_cc_submachine="")
+    ;;
+  405|440|464|476)
+    submachine=${archcpu}
+    AC_CACHE_VAL(libc_cv_cc_submachine,libc_cv_cc_submachine="")
+    ;;
+
+  a2|970|power[[4-9]]|power5x|power6+)
+    submachine=${archcpu}
+    AC_CACHE_VAL(libc_cv_cc_submachine,libc_cv_cc_submachine="")
+    ;;
+  *)
+    # We couldn't figure it out, assume none
+    ;;
+  esac
+  ;;
+esac