public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Richard Sandiford <richard.sandiford@arm.com>
To: YunQiang Su <yunqiang.su@cipunited.com>
Cc: gcc-patches@gcc.gnu.org,  macro@orcam.me.uk,  syq@debian.org,
	 jiaxun.yang@flygoat.com
Subject: Re: [PATCH] MIPS: use arch_32/64 instead of default_mips_arch
Date: Fri, 03 Feb 2023 12:28:51 +0000	[thread overview]
Message-ID: <mptr0v7aq64.fsf@arm.com> (raw)
In-Reply-To: <20230203090544.2528175-1-yunqiang.su@cipunited.com> (YunQiang Su's message of "Fri, 3 Feb 2023 17:05:44 +0800")

YunQiang Su <yunqiang.su@cipunited.com> writes:
> The value of default_mips_arch will be always used for -march by default,
> no matter what value is given to -mabi.
> It will produce abnormal elf file like:
>  ELF 32-bit LSB relocatable, MIPS, MIPS64 rel2 version 1 (SYSV)

Is that really wrong though?  There's nothing in principle that
prevents a 64-bit ISA being used with a 32-bit ABI, even in the
object file's metadata.

> So we use with_arch_32 and with_arch_64 instead of default_mips_arch
> for all mipsisa[32,64]rN triples.

I agree there's no benefit to using a stock MIPS64rN ISA over
a stock MIPS32rN ISA with a 32-bit ABI, and the patch is only
changing those cases.  But things are different when using
(say) MIPS4 with a 32-bit ABI, or a 64-bit processor that has
proprietary extensions.

And, for example, a mips-linux-gnu toolchain would (IIRC) require
an -march as well as an -mabi in order to generate 64-bit code.
There would be no implicit selection of a new -march.

I'm not opposed to the patch.  I just think we should be clear
about the underlying principle.  If it's just that all MIPS32/64rN
toolchains should behave in the same way (like the sde and mti ones
do), then the patch looks good.  But I don't think we should create
a general principle that -mabi determines/changes/downgrades -march.

Thanks,
Richard

>
> gcc/ChangeLog:
> 	* config.gcc: use with_arch_32 and with_arch_64 instead of
> 	default_mips_arch for mipsisa[32,64]rN triples.
> ---
>  gcc/config.gcc | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/gcc/config.gcc b/gcc/config.gcc
> index f0958e1c959..0b6d093d847 100644
> --- a/gcc/config.gcc
> +++ b/gcc/config.gcc
> @@ -2518,13 +2518,16 @@ mips*-*-linux*)				# Linux MIPS, either endian.
>  	extra_options="${extra_options} linux-android.opt"
>  	case ${target} in
>  		mipsisa32r6*)
> -			default_mips_arch=mips32r6
> +			with_arch_32="mips32r6"
> +			with_arch_64="mips64r6"
>  			;;
>  		mipsisa32r2*)
> -			default_mips_arch=mips32r2
> +			with_arch_32="mips32r2"
> +			with_arch_64="mips64r2"
>  			;;
>  		mipsisa32*)
> -			default_mips_arch=mips32
> +			with_arch_32="mips32"
> +			with_arch_64="mips64"
>  			;;
>  		mips64el-st-linux-gnu)
>  			default_mips_abi=n32
> @@ -2540,22 +2543,26 @@ mips*-*-linux*)				# Linux MIPS, either endian.
>  			;;
>  		mipsisa64r6*-*-linux-gnuabi64)
>  			default_mips_abi=64
> -			default_mips_arch=mips64r6
> +			with_arch_32="mips32r6"
> +			with_arch_64="mips64r6"
>  			enable_mips_multilibs="yes"
>  			;;
>  		mipsisa64r6*-*-linux*)
>  			default_mips_abi=n32
> -			default_mips_arch=mips64r6
> +			with_arch_32="mips32r6"
> +			with_arch_64="mips64r6"
>  			enable_mips_multilibs="yes"
>  			;;
>  		mipsisa64r2*-*-linux-gnuabi64)
>  			default_mips_abi=64
> -			default_mips_arch=mips64r2
> +			with_arch_32="mips32r2"
> +			with_arch_64="mips64r2"
>  			enable_mips_multilibs="yes"
>  			;;
>  		mipsisa64r2*-*-linux*)
>  			default_mips_abi=n32
> -			default_mips_arch=mips64r2
> +			with_arch_32="mips32r2"
> +			with_arch_64="mips64r2"
>  			enable_mips_multilibs="yes"
>  			;;
>  		mips64*-*-linux-gnuabi64 | mipsisa64*-*-linux-gnuabi64)

  reply	other threads:[~2023-02-03 12:28 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-03  9:05 YunQiang Su
2023-02-03 12:28 ` Richard Sandiford [this message]
2023-02-03 13:23   ` YunQiang Su
2023-02-03 14:08     ` Richard Sandiford
2023-02-03 14:34       ` Xi Ruoyao
2023-02-03 15:22         ` YunQiang Su

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=mptr0v7aq64.fsf@arm.com \
    --to=richard.sandiford@arm.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jiaxun.yang@flygoat.com \
    --cc=macro@orcam.me.uk \
    --cc=syq@debian.org \
    --cc=yunqiang.su@cipunited.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).