Re: [PATCH] MIPS: recoginze mipsisa64 as 64bit CPU

[YunQiang Su <wzssyqa@gmail.com>]

Maciej W. Rozycki <macro@orcam.me.uk> 于2023年8月17日周四 20:37写道：
>
> On Tue, 15 Aug 2023, YunQiang Su wrote:
>
> > In GCC, mipsisa64* in triples are recoginzed as 64bit CPU.
> > Let's do the same.
> >
> > The default ABI is determined by the abi section of triples,
> > which is same with the `mips64*' CPU:
> >    -gnuabi64 and -openbsd for N64
> >    otherwise, N32.
>
>  The change description has to be explicit in that we only refer to Linux
> configurations here, as it doesn't change the semantics of `mipsisa64*'
> CPUs for other OSes.  And it's not that we don't consider the CPU 64-bit
> for `mipsisa64*-*-linux*' configurations.  We just don't use a 64-bit ABI
> by default.  Finally there's no need to repeat the rules for ABI selection
> as we just follow the existing ones for `mips64*-*-linux*' configurations.
>
>  How about:
>
> MIPS: Use a 64-bit ABI by default for `mipsisa64*-*-linux*' targets
>

I use ABIs here, since they are both N32 and/or N64.

> Following the arrangement in GCC select a 64-bit ABI by default, either
> n32 or n64, rather than o32 for `mipsisa64*-*-linux*' targets, just as
> with the corresponding `mips64*-*-linux*' targets.
>
> then?
>
>  NB please remember to capitalise the sentence in change headings, whether
> it's there on its own or prefixed with a subsystem name such as "MIPS:".
> And use a hyphen in combined number-word adjectives such as "64-bit".
>
> > diff --git a/gas/configure.ac b/gas/configure.ac
> > index c3bd1178d41..617a5ca1473 100644
> > --- a/gas/configure.ac
> > +++ b/gas/configure.ac
> > @@ -394,10 +394,10 @@ changequote([,])dnl
> >       esac
> >       # Decide which ABI to target by default.
> >       case ${target} in
> > -       mips64*-openbsd* | mips64*-linux-gnuabi64)
> > +       mips64*-openbsd* | mips64*-linux-gnuabi64 | mipsisa64*-linux-gnuabi64)
> >           mips_default_abi=N64_ABI
> >           ;;
> > -       mips64*-linux* | mips-sgi-irix6* | mips64*-freebsd* \
> > +       mips64*-linux* | mipsisa64*-linux* | mips-sgi-irix6* | mips64*-freebsd* \
>
>  These lines overrun 79 columns and need to be wrapped; there's a reason
> for the existing wrapping.
>
> > diff --git a/gold/configure.tgt b/gold/configure.tgt
> > index 4b54e08d27f..d09bb76ef02 100644
> > --- a/gold/configure.tgt
> > +++ b/gold/configure.tgt
> > @@ -153,13 +153,27 @@ aarch64*-*)
> >   targ_big_endian=false
> >   targ_extra_big_endian=true
> >   ;;
> > -mips*el*-*-*|mips*le*-*-*)
> > +mips64*el*-*-* | mipsisa64*le*-*-*)
> > + targ_obj=mips
> > + targ_machine=EM_MIPS_RS3_LE
> > + targ_size=64
> > + targ_big_endian=false
> > + targ_extra_big_endian=true
> > + ;;
> > +mips*el*-*-*)
>
>  You are removing the `mips*le*-*-*' configuration here.  It may well be
> the right move given that no other binutils component has it, but it has
> to be a separate change.
>

You are right. I will add it back.

>  Also the use of EM_MIPS_RS3_LE has been deprecated since forever, so
> please don't introduce a new case.  I have no idea why the existing case
> has been accepted into GOLD in the first place as BFD has never emitted it
> and it was never intended to be used for newly-produced ELF files (the
> extra MIPS ELF machine type allocation was essentially an accident in the
> psABI design, it's not even named correctly).
>

Thank you. I will change it to EM_MIPS.

>  NB `mips*el*-*-*' isn't right either, it should be `mips*el-*-*' just as
> elsewhere, because we'll otherwise take a CPU name with "el" in the middle
> for the endianness.
>

Sure... Let's correct them.

> >   targ_obj=mips
> >   targ_machine=EM_MIPS_RS3_LE
> >   targ_size=32
> >   targ_big_endian=false
> >   targ_extra_big_endian=true
> >   ;;
> > +mips64*-*-* | mipsisa64*-*-*)
> > + targ_obj=mips
> > + targ_machine=EM_MIPS
> > + targ_size=64
> > + targ_big_endian=true
> > + targ_extra_big_endian=false
> > + ;;
>
>  Also you're adding new 64-bit MIPS support to GOLD here, so it has to be
> a separate patch too from the changes to the other binutils components.
> Is it a working configuration for GOLD even?  Doesn't it need to have

In fact, the current configure.tgt makes something wrong, if we configure
it with --with-targets=mips64-linux-gnuabi64.
https://buildd.debian.org/status/fetch.php?pkg=binutils-mipsen&arch=amd64&ver=10%2Bc5&stamp=1692086940&raw=0

That's why I'd like to put them in a single patch.

> `targ_extra_size' also set?
>

Maybe no.
Since `configure.tgt` is used for --enable-targets=xx,yy option.
WIth this option, only the list extra targets should be support.
If we set it, 32bit targets will be supported anyway.

If user wants to support all targets, they need to use
   --with-targets=all

>  Please resubmit with these issues addressed.
>
>   Maciej



-- 
YunQiang Su