From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-f43.google.com (mail-pj1-f43.google.com [209.85.216.43]) by sourceware.org (Postfix) with ESMTPS id 17E613858CD1 for ; Mon, 31 Jul 2023 10:32:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 17E613858CD1 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=debian.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-pj1-f43.google.com with SMTP id 98e67ed59e1d1-26854159c05so2462156a91.2 for ; Mon, 31 Jul 2023 03:32:30 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690799549; x=1691404349; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=o4+uHSgnXD7hRNJGMKfF5klrfbb93FUi8tSS2bDY9Mw=; b=ZGnpa+D+J+hyxB/VTayJs1u4gR5z3/6cvypzmiAVjwokuizwFOjX4pyBVvACKDSF6l ybKz2vVny+7q6IKCRGZq4ggY7RLV3N7hHiuegX6veUpWYgtm5L/sbedsUbuRkLsOt+ZF /7c0+0wAZaSpTfMZ/E0CKxtgRHH35+1aPSRF9dE4uheNLv5rM3HBTcpD7Z2w+8wax+uS XBeqTzseKP9nMOUEXUvtWoPpkkkD6ybzrImPp0XgB/C09XIpoK8VXvA6Gs+AB+4yKl/s tK3pSnYzC4+dLe7Ya+J0fNM1vfiuJveUJvnl8PpRVvtJoToC18qzrETxsZH/fzefY+Lx wWiw== X-Gm-Message-State: ABy/qLY6dBdhtfGumybzKrGb0OVYeSsJxzZN2t86xt0ft604Kkz35i4k Ul44p4vK2cQyYpCyqiJpGHrGDcR0W/tpIyBY+kY= X-Google-Smtp-Source: APBJJlG5XtbJn6YqLqJ+HdegzAw+2vOn/FVCaPx509M/IGVOyb9gegCQHdpxTUsfvIQg9DQunfnKO2FndXsutYf/gLw= X-Received: by 2002:a17:90a:c254:b0:268:1d18:a19f with SMTP id d20-20020a17090ac25400b002681d18a19fmr8976005pjx.25.1690799548911; Mon, 31 Jul 2023 03:32:28 -0700 (PDT) MIME-Version: 1.0 References: <20230414072046.1639896-1-yunqiang.su@cipunited.com> <20230418140019.2195551-1-yunqiang.su@cipunited.com> In-Reply-To: From: YunQiang Su Date: Mon, 31 Jul 2023 18:32:17 +0800 Message-ID: Subject: Re: [PATCH v4 1/2] MIPS: support mips*64 as CPU and gnuabi64 as ABI To: "Maciej W. Rozycki" Cc: Richard Sandiford , Nick Clifton , YunQiang Su , binutils@sourceware.org, xry111@xry111.site, jiaxun.yang@flygoat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,BODY_8BITS,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM,HEADER_FROM_DIFFERENT_DOMAINS,KAM_DMARC_STATUS,KAM_SHORT,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Maciej W. Rozycki =E4=BA=8E2023=E5=B9=B47=E6=9C=8831=E6= =97=A5=E5=91=A8=E4=B8=80 18:05=E5=86=99=E9=81=93=EF=BC=9A > > On Fri, 21 Jul 2023, YunQiang Su wrote: > > > Debian, as a *REAL* OS, instead of anything you are imagining, is *USIN= G* it. > > I don't agree that you make any "features" heavenly. > > If so, we cannot fix any bug. > > Sometimes bugs have to live forever if the removal would cause more harm > than keeping them. So we best don't introduce them in the first place. > Of course a human beeings we are all bound to make mistakes, but this is > the very reason we have come up with thorough peer reviews: to catch any > mistakes early on, before they cause actual harm. Some will escape > regardless, but we need to try hard and prevent that from happening > rather than being lax. > > The use of `mipsisa64-*-*' triplets for o32 ABI configurations wasn't a > bug though, it was a design decision. Now making these triplets mean n32 > after they meant something else for 20+ years seems like a bug to me. > > > More and more software in the *REAL* world, use the CPU section (here m= ipsisa64) > > to determine the 64bit OS/env. > > Anyway, the *REAL* world is much more important the the world only in > > your *MIND*. > > Personal insults are not going to help you with getting changes accepted= , > as we're making decisions based on technical merits rather than opinions. > > I don't think a fait accompli ("Debian does it") is a valid argument in = a > technical review. It is just a statement of fact and we do not have to > accept the same solution while Debian is free to continue using it. > > At the risk of repeating myself, you do need to justify your change > before it can be accepted, that is you have to provide technical argument= s > to prove it to the reviewer that taking your change is the right thing to > do. You may certainly reuse the arguments that were given at the time th= e > choice was evaluated for Debian and if we find them convincing, we will > accept the change. > > I don't think we have a document written down for submitting patches > specifically for binutils, but we have been broadly following the rules > for the GNU toolchain, as documented on pages for our sibling projects: > > , > , > . > > Please familiarise yourself with these documents before posting further > changes and follow the patch submission guidelines set out there. > > I'd expect you, now as a nominated MIPS GCC backend maintainer, to be > especially familiar with the first of these documents, the rules of which > you are expected not only to apply to your own changes, but to enforce > them for patches submitted by other people, whether by the general public > or your fellow employees. As far I can tell it wasn't regrettably the > case with the MIPS16e2 patchset I volunteered to properly review (having > been involved with the instruction set design and having implemented the > binutils side) and which offer was ignored. > > And in any case you can do whatever you want in Debian as long as it's > within what the licence allows you to, but we do not necessarily have to > accept your changes. Ideally Debian developers would have asked us in > advance if an incompatible change they want to make locally would be > accepted, or actually would have submitted it so that it lands here first > (or is rejected, in which case they'd have a chance to find an alternativ= e > solution). > > I would have argued then you probably want `mips64isa64*-*-*' or suchlik= e > a triplet for your 64-bit ABI configuration, leaving `mipsisa64*-*-*' one= s > for 32-bit ABI configurations (at least as far as the default choice is > concerned), as that would be consistent with our current `mips*-*-*' vs > `mips64*-*-*' naming scheme. > > In that scheme for the machine part we have, in this order: > > 1. The base prefix: `mips'/`mips64' to tell 32-bit/64-bit configurations > apart. > > 2. An optional implementation specification, e.g. `vr4300', `r5900', > `isa32r2', etc., which sets the CPU default as with the `-march=3D' > option. > > 3. An optional `el' endianness suffix to make the little endianness the > default rather than the big one. > > So `mipstx39el-*-*' is a 32-bit configuration with the TX39 CPU chosen by > default and little endianness. Likewise `mips64r10000-*-*' is a 64-bit > big-endian one with the R10k default. Then `mipsocteon+-*-*' is a 32-bit > one with Octeon+, and for a 64-bit one then say `mips64octeon+-*-*'. > > Why would you want to make it different for `isa64' then? Say > `mipsisa64-*-*' for a 32-bit configuration and then `mips64isa64-*-*' for > a 64-bit one. In fact it already works like this: you can use > `mips64isa64r6-linux-gnu' and `mips64isa64r6el-linux-gnu' for your Debian > system right away and get what you need, there's nothing to change. > Let's go back to 10 years ago, the commit 5afd44e33b13b922760a41580020f941dbdd473e of GCC which is the previous commit of MIPS r6 support is added. Let's have a glance of gcc/config.gcc: There are bellow lines: mips*-*-linux*) # Linux MIPS, either endian. tm_file=3D"dbxelf.h elfos.h gnu-user.h linux.h linux-android.h glibc-stdint.h ${tm_file} mips/gnu-user.h mips/linux.h mips/linux-common.h" extra_options=3D"${extra_options} linux-android.opt" case ${target} in mipsisa32r2*) default_mips_arch=3Dmips32r2 ;; mipsisa32*) default_mips_arch=3Dmips32 ;; mips64el-st-linux-gnu) default_mips_abi=3Dn32 tm_file=3D"${tm_file} mips/st.h" tmake_file=3D"${tmake_file} mips/t-st" enable_mips_multilibs=3D"yes" ;; mips64octeon*-*-linux*) default_mips_abi=3Dn32 tm_defines=3D"${tm_defines} MIPS_CPU_STRING_DEFAULT=3D\\\"octeon\\\"" target_cpu_default=3DMASK_SOFT_FLOAT_ABI enable_mips_multilibs=3D"yes" ;; mipsisa64r2*-*-linux*) default_mips_abi=3Dn32 default_mips_arch=3Dmips64r2 enable_mips_multilibs=3D"yes" ;; mips64*-*-linux* | mipsisa64*-*-linux*) default_mips_abi=3Dn32 enable_mips_multilibs=3D"yes" ;; esac I don't think that it is a good idea that they are different between GCC and binutils. > We do have a couple of legacy aberrations from very old, pre-MIPS32 days= , > most notably `mips-sgi-irix6', but I think there's no need to go back > there. > > Of course not all configurations are valid, e.g. `mips64isa32-*-*' is > not, but neither is say `mips64tx39-*-*', so that's no news. And then > `mipsinteraptiv-mr2el-*-*' is broken due to the presence of the hyphen in > `mipsinteraptiv-mr2', which is an oversight of mine: we need to define an > `interaptiv_mr2' (or `interaptiv+mr2') alias to allow this configuration, > similarly to how say `74kf3_2' has been named, and remember not to use a > hyphen in any future CPU names. > > There's really no need to break anything here, we've had it from time > immemorial. > > I note however that `config.sub' hasn't been correctly updated for the > MIPS CPU names and consequently we have a random choice only that works > and other ones don't. In fact given the variability of our MIPS > configurations I fail to see why it doesn't just let it all through. I > have submitted a fix now. > > Maciej