[PATCH] Add .set arch=FOO support to MIPS gas.

Thiemo Seufer ica2_ts@csv.ica.uni-stuttgart.de
Sun Jun 29 10:33:00 GMT 2003


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



More information about the Binutils mailing list