Re: [PATCH] Better checking of ISA/ASE/ABI options for MIPS gas

[Richard Sandiford <richard at codesourcery dot com>]

Thiemo Seufer <ths@networkno.de> writes:
> Richard Sandiford wrote:
> [snip]
>> > -  /* ??? GAS treats single-float processors as though they had 64-bit
>> > -     float registers (although it complains when double-precision
>> > -     instructions are used).  As things stand, saying they have 32-bit
>> > -     registers would lead to spurious "register must be even" messages.
>> > -     So here we assume float registers are always the same size as
>> > -     integer ones, unless the user says otherwise.  */
>> > -  if (file_mips_fp32 < 0)
>> > -    file_mips_fp32 = file_mips_gp32;
>> > +  switch (file_mips_fp32)
>> > +    {
>> > +    default:
>> > +    case -1:
>> > +      /* No user specified float register size.  */
>> > +      if (file_mips_gp32 == 0)
>> > +	/* 64-bit integer registers implies 64-bit float registers.  */
>> > +	file_mips_fp32 = 0;
>> > +      else if ((mips_opts.ase_mips3d > 0 || mips_opts.ase_mdmx > 0)
>> > +	       && ISA_HAS_64BIT_FPRS (mips_opts.isa))
>> > +	/* -mips3d and -mdmx imply 64-bit float registers, if possible.  */
>> > +	file_mips_fp32 = 0;
>> > +      else
>> > +	/* 32-bit float registers. */
>> > +	file_mips_fp32 = 1;
>> > +      break;
>> > +
>> > +    /* The user specified the size of the float registers.  Check if it
>> > +       agrees with the ABI and ISA.  */
>> > +    case 0:
>> > +      if (!ISA_HAS_64BIT_FPRS (mips_opts.isa))
>> > +	as_bad (_("-mfp64 used with a 32-bit fpu"));
>> > +      else if (ABI_NEEDS_32BIT_REGS (mips_abi)
>> > +	       && !ISA_HAS_MXHC1 (mips_opts.isa))
>> > +	as_warn (_("-mfp64 used with a 32-bit ABI"));
>> > +      break;
>> > +    case 1:
>> > +      if (ABI_NEEDS_64BIT_REGS (mips_abi))
>> > +	as_warn (_("-mfp32 used with a 64-bit ABI"));
>> > +      break;
>> > +    }
>> 
>> Doesn't the ??? comment still hold?  I thought it would be valuable
>> to keep it.
>
> The defaulting is now more complicated than the comment suggests.

True.  I was meaning the first bit though, about single-float processors.

OTOH, the whole comment might make sense as-is in place of
"/* 32-bit float registers. */".

>> > +  else if (strcmp (name, "gp64") == 0)
>> > +    {
>> > +      if (!ISA_HAS_64BIT_REGS (mips_opts.isa))
>> > +	as_warn ("%s isa does not support 64-bit registers", 
>> > +		 mips_cpu_info_from_isa (mips_opts.isa)->name);
>> > +      mips_opts.gp32 = 0;
>> > +    }
>> > +  else if (strcmp (name, "gp32") == 0)
>> > +    {
>> > +      mips_opts.gp32 = 1;
>> > +    }
>> > +  else if (strcmp (name, "nogp64") == 0 || strcmp (name, "nogp32") == 0)
>> > +    {
>> > +      mips_opts.gp32 = file_mips_gp32;
>> > +    }
>> 
>> Ugh.  I don't like the "nogp32" and "nogp64". 
>
> Well, it is what MIPS' SDE toolchain supports.

I'm not sure whether you're giving that as a reason not to change it,
or whether it's just an FYI.

>> All other ".set X"/".set noX"
>> pairs are used for turning a particular feature on and off.  Having "nogp32"
>> restore the prevailing size (even if that prevailing size _is_ gp32)
>> seems very counter-intuitive to me.  Could we not have ".set gp=" instead,
>> with a special value to select the prevailing size?  That would be more
>> consistent with .set mipsX and .set arch=X, for example.
>
> Hm, .set gp={32,64,default} or .set gp{32,64,0} ?

The former sounds better to me.

Richard