Cleanup MIPS preconfigure script
Commit Message
On Fri, 2014-09-05 at 21:55 +0000, Joseph S. Myers wrote:
> On Fri, 5 Sep 2014, Steve Ellcey wrote:
>
> > I agree, here is a new patch. I complete removed the setting of
> > base_machine since it is not used and I removed the line:
>
> base_machine *is* used, to set base-machine in config.make. But the only
> things that's used for now are (mach/Makefile for powerpc and) libc-abis
> handling (regarding which see
> <https://sourceware.org/ml/libc-alpha/2014-01/msg00375.html>) - and
> whatever the libc-abis handling does or does not work for, there is no use
> in a special setting of base_machine for mips64; just setting to mips for
> all MIPS cases seems most appropriate and consistent with other
> architectures.
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?
Steve Ellcey
sellcey@mips.com
2014-09-09 Steve Ellcey <sellcey@mips.com>
* sysdeps/mips/preconfigure: Modify ABI tests.
Comments
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
@@ -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