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.
Richard Sandiford wrote:
> 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.
What about initializing mips_tune in one place in gcc, too? With my
patch first arch gets initialized, then tune. AFAICS there's no need
to split it.
> Now that you're setting a local variable as well,
I decided to remove the global arch_info, because it's not constant
over runtime any more, and using it from anywhere else than the
initial setup will interfere with .set march=FOO.
> 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.
It is, because mips_set_architecture sets the mips_opt.isa value,
and those use is intermingled with the mips_set_architecture calls.
> 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.
I tried this first, and it broke. mips_opt.isa is also needed for the
-mipsX compatibility check.
Thiemo