This is the mail archive of the binutils@sources.redhat.com mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]