Message ID | 1410279506.2740.158.camel@ubuntu-sellcey |
---|---|
State | New |
Headers | show |
On Tue, 9 Sep 2014, Steve Ellcey wrote: > Here is a new patch. I set base_machine to mips and I put back the > 'machine=$machine/$config_machine' line along with a comment about why > it is needed. I rebuilt mips-mti-linux-gnu, mipsel-linux-gnu, and > mips64-linux-gnu toolchains to test the change. > > OK to checkin? OK.
On Tue, 9 Sep 2014, Steve Ellcey wrote: > +if test "$abiflag" != "_ABIO32" -a "$mips16flag" = "1"; then > + as_fn_error $? "mips16 is only supported with the O32 ABI." "$LINENO" 5 > +fi Hmm, I think the capitalisation is weird here, why not: + as_fn_error $? "MIPS16 is only supported with the o32 ABI." "$LINENO" 5 ? Maciej
On Tue, 2014-09-09 at 18:08 +0100, Maciej W. Rozycki wrote: > On Tue, 9 Sep 2014, Steve Ellcey wrote: > > > +if test "$abiflag" != "_ABIO32" -a "$mips16flag" = "1"; then > > + as_fn_error $? "mips16 is only supported with the O32 ABI." "$LINENO" 5 > > +fi > > Hmm, I think the capitalisation is weird here, why not: > > + as_fn_error $? "MIPS16 is only supported with the o32 ABI." "$LINENO" 5 > > ? > > Maciej I already checked it in but I can go back and tweak the capitalization if you want. Steve Ellcey sellcey@mips.com
On 09-09-2014 14:13, Steve Ellcey wrote: > On Tue, 2014-09-09 at 18:08 +0100, Maciej W. Rozycki wrote: >> On Tue, 9 Sep 2014, Steve Ellcey wrote: >> >>> +if test "$abiflag" != "_ABIO32" -a "$mips16flag" = "1"; then >>> + as_fn_error $? "mips16 is only supported with the O32 ABI." "$LINENO" 5 >>> +fi >> Hmm, I think the capitalisation is weird here, why not: >> >> + as_fn_error $? "MIPS16 is only supported with the o32 ABI." "$LINENO" 5 >> >> ? >> >> Maciej > I already checked it in but I can go back and tweak the capitalization > if you want. > > Steve Ellcey > sellcey@mips.com > And patch 0febba23ddabcd971be5259ee20236b9e3efa690 broke powerpc64 configure: $ /home/azanella/glibc/glibc-git/configure --prefix=/usr --with-cpu=power8 checking build system type... powerpc64-unknown-linux-gnu checking host system type... powerpc64-unknown-linux-gnu checking for gcc... gcc checking for suffix of object files... o checking whether we are using the GNU C compiler... yes checking whether gcc accepts -g... yes checking for g++... g++ checking whether we are using the GNU C++ compiler... yes checking whether g++ accepts -g... yes checking for readelf... readelf checking for sysdeps preconfigure fragments... aarch64 alpha arm hppa i386 m68k microblaze mips configure: error: Unable to determine ABI.
On Tue, 9 Sep 2014, Adhemerval Zanella wrote:
> And patch 0febba23ddabcd971be5259ee20236b9e3efa690 broke powerpc64 configure:
Steve, you need to put preconfigure back inside a case statement so
nothing runs for non-mips* machines.
On Tue, 9 Sep 2014, Steve Ellcey wrote: > > Hmm, I think the capitalisation is weird here, why not: > > > > + as_fn_error $? "MIPS16 is only supported with the o32 ABI." "$LINENO" 5 > > > > ? > > I already checked it in but I can go back and tweak the capitalization > if you want. That would be my preference, thanks. While historically across the toolchain we don't have a very good record of keeping the spelling of such stuff correct, I think we really ought to keep it consistent with published documentation. Especially in messages shown to the user or external documentation, although it won't hurt doing that everywhere including internal documentation, debug messages and comments, to make getting good habits easier if nothing else. Inconsistent or bad spelling gives users the impression code itself is sloppy and that is something we'd rather avoid. Having also made sure code actually is not sloppy that is of course, that we strive to achieve through our review process. Maciej
On Tue, 2014-09-09 at 19:21 +0100, Maciej W. Rozycki wrote: > On Tue, 9 Sep 2014, Steve Ellcey wrote: > > > > Hmm, I think the capitalisation is weird here, why not: > > > > > > + as_fn_error $? "MIPS16 is only supported with the o32 ABI." "$LINENO" 5 > > > > > > ? > > > > I already checked it in but I can go back and tweak the capitalization > > if you want. > > That would be my preference, thanks. I included this change with the bug fix I checked in. https://sourceware.org/ml/libc-alpha/2014-09/msg00136.html Steve Ellcey sellcey@mips.com
diff --git a/sysdeps/mips/preconfigure b/sysdeps/mips/preconfigure index b215eb2..fb572d7 100644 --- a/sysdeps/mips/preconfigure +++ b/sysdeps/mips/preconfigure @@ -1,34 +1,24 @@ -case "$machine" in -mips64*) base_machine=mips64 - case "$CC $CFLAGS $CPPFLAGS " in - *" -mabi=n32 "*) mips_cc_abi=n32 ;; - *" -mabi=64 "*|*" -mabi=n64 "*) mips_cc_abi=64 ;; - *" -mabi=32 "*|*" -mabi=o32 "*) mips_cc_abi=32 ;; - *) mips_cc_abi=default ;; - esac - case $config_os in - *abin32*) mips_config_abi=n32 ;; - *abi64*|*abin64*) mips_config_abi=64 ;; - *abi32*|*abio32*) mips_config_abi=32 ;; - *) mips_config_abi=$mips_cc_abi ;; - esac - case $mips_config_abi in - default) machine=mips/mips64/n32 mips_config_abi=n32 ;; - n32) machine=mips/mips64/n32 ;; - 64) machine=mips/mips64/n64 ;; - 32) machine=mips/mips32/kern64 ;; - esac - machine=$machine/$config_machine - if test $mips_config_abi != $mips_cc_abi; then - # This won't make it to config.make, but we want to - # set this in case configure tests depend on it. - CPPFLAGS="$CPPFLAGS -mabi=$mips_config_abi" - fi - ;; -mips*) base_machine=mips - case "$CC $CFLAGS $CPPFLAGS " in - *" -mips16 "*) machine=mips/mips32/mips16/$machine ;; - *) machine=mips/mips32/$machine ;; - esac - ;; -esac +abiflag=`$CC $CFLAGS $CPPFLAGS -E -dM -xc /dev/null | sed -n 's/^#define _MIPS_SIM \(.*\)/\1/p'` +mips16flag=`$CC $CFLAGS $CPPFLAGS -E -dM -xc /dev/null | sed -n 's/^#define __mips16 \(.*\)/\1/p'` + +base_machine=mips +if test "$abiflag" = "_ABIO32" ; then + if test "$mips16flag" = "1" ; then + machine=mips/mips32/mips16 + else + machine=mips/mips32 + fi +elif test "$abiflag" = "_ABIN32" ; then + machine=mips/mips64/n32 +elif test "$abiflag" = "_ABI64" ; then + machine=mips/mips64/n64 +else + as_fn_error $? "Unable to determine ABI." "$LINENO" 5 +fi +# $config_machine is not really needed here but the slash after $machine is +# needed by the case statement in sysdeps/unix/sysv/linux/mips/configure.ac. +machine=$machine/$config_machine + +if test "$abiflag" != "_ABIO32" -a "$mips16flag" = "1"; then + as_fn_error $? "mips16 is only supported with the O32 ABI." "$LINENO" 5 +fi