This is the mail archive of the
binutils@sources.redhat.com
mailing list for the binutils project.
Re: [PATCH] Add .set arch=FOO support to MIPS gas.
- From: Richard Sandiford <rsandifo at redhat dot com>
- To: Thiemo Seufer <ica2_ts at csv dot ica dot uni-stuttgart dot de>
- Cc: binutils at sources dot redhat dot com
- Date: 29 Jun 2003 09:05:16 +0100
- Subject: Re: [PATCH] Add .set arch=FOO support to MIPS gas.
- References: <20030628224804.GC31215@rembrandt.csv.ica.uni-stuttgart.de>
Thanks for doing this! One minor nit though:
Thiemo Seufer <ica2_ts@csv.ica.uni-stuttgart.de> writes:
> @@ -11131,17 +11129,17 @@ mips_after_parse_args ()
> as much as possible. */
>
> if (mips_arch_string != 0)
> - mips_set_architecture (mips_parse_cpu ("-march", mips_arch_string));
> -
> - if (mips_tune_string != 0)
> - mips_set_tune (mips_parse_cpu ("-mtune", mips_tune_string));
> + {
> + arch_info = mips_parse_cpu ("-march", mips_arch_string);
> + mips_set_architecture (arch_info);
> + }
> [...]
> + /* Optimize for file_mips_arch, unless -mtune selects a different processor. */
> + if (mips_tune_string != 0)
> + tune_info = mips_parse_cpu ("-mtune", mips_tune_string);
Could you keep the mips_tune_string handling in the original place?
That should make it easier to compare the gas and gcc code.
Now that you're setting a local variable as well, it almost looks
like the two (twice-repeated) lines:
arch_info = mips_parse_cpu ("-march", mips_arch_string);
and:
mips_set_architecture (arch_info);
are doing logically different things. I.e. that there's something
significant about calling mips_set_architecture() right away.
But as far as I can tell, there isn't.
Maybe it would be better to just set arch_info during the main
arch-detection stuff and have one call mips_set_architecture()
right before the ABI_NEEDS_64BIT_REGS check? Likewise tune_info,
mips_set_tune() and the file_mips_gp32 check.
Like I say, really minor stuff. ;)
Richard