From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from angie.orcam.me.uk (angie.orcam.me.uk [78.133.224.34]) by sourceware.org (Postfix) with ESMTP id 62B9F3858D1E for ; Thu, 17 Aug 2023 12:36:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 62B9F3858D1E Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=orcam.me.uk Authentication-Results: sourceware.org; spf=none smtp.mailfrom=orcam.me.uk Received: by angie.orcam.me.uk (Postfix, from userid 500) id 41EF392009C; Thu, 17 Aug 2023 14:36:50 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by angie.orcam.me.uk (Postfix) with ESMTP id 3B21292009B; Thu, 17 Aug 2023 13:36:50 +0100 (BST) Date: Thu, 17 Aug 2023 13:36:50 +0100 (BST) From: "Maciej W. Rozycki" To: YunQiang Su cc: binutils@sourceware.org Subject: Re: [PATCH] MIPS: recoginze mipsisa64 as 64bit CPU In-Reply-To: <20230815104821.41855-1-yunqiang.su@cipunited.com> Message-ID: References: <20230815104821.41855-1-yunqiang.su@cipunited.com> User-Agent: Alpine 2.21 (DEB 202 2017-01-01) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-1169.2 required=5.0 tests=BAYES_00,GIT_PATCH_0,KAM_DMARC_STATUS,KAM_INFOUSMEBIZ,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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 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. 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). 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. > 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 `targ_extra_size' also set? Please resubmit with these issues addressed. Maciej