Re: RFC: Mips option parsing patch

[Daniel Jacobowitz <drow at mvista dot com>]

On Tue, Nov 27, 2001 at 07:31:28PM +0100, Thiemo Seufer wrote:
> Daniel Jacobowitz wrote:
> [snip]
> > Binaries built with -march=r8000 -mips2 are now marked:
> >   Flags:                             0x30000100, mips4 UNKNOWN
> > 
> > The mips4 marking is wrong, but that's a BFD bug - it sets mips_arch
> > unconditionally ignoring what GAS may have set.
> 
> IMHO BFD does the right thing here if -mipsN is regarded as an
> alias for the ISA's default architecture. (This isn't true when
> using gcc 3.0, but it's the way it should be handled.)

OK.

> > The UNKNOWN is a
> > readelf bug, and actually E_MIPS_32BITMODE - perfect.
> 
> It actually isn't EF_MIPS_32BITMODE. UNKNOWN is the cpu type given
> in the header flag (like 4111 or 3900). I wonder why readelf doesn't
> show 32bitmode for you, support is already there.
> 
> Btw, it's possibly a good idea to ignore this part of the header flags
> for cpu's not defined there. At least it would avoid confusion. :-)

Well, then, it's still a bug :)

> [snip]
> >  /* True if -mgp32 was passed.  */
> > -static int file_mips_gp32 = 0;
> > +static int file_mips_gp32 = -1;
> >  
> >  /* True if -mfp32 was passed.  */
> > -static int file_mips_fp32 = 0;
> > +static int file_mips_fp32 = -1;
> >  
> >  /* This is the struct we use to hold the current set of options.  Note
> >     that we must set the isa field to ISA_UNKNOWN and the mips16 field to
> > @@ -1077,6 +1077,37 @@ md_begin ()
> >        assert (ci != NULL);
> >        if (mips_opts.isa != ci->isa)
> >  	{
> > +#if 1
> > +	  /* This code should go away, to be replaced with the block below it.
> 
> Err, no. :-) Both blocks should be replaced by an error message.

That works too.  I'll change the comment and delete the other block,
then.

> > +	     Until GCC 3.1 has been released for some reasonable amount of time,
> > +	     however, we need to support this.  */
> 
> There should also be a comment to change the file_mips_*
> initializations back.

No need.  -1 is useful to mean "not set" in case some other code ever
needs this.

> > +	  /* Translate -mipsN to the appropriate settings of file_mips_gp32
> > +	     and file_mips_fp32.  Tag binaries as using the mipsN ISA.  */
> 
> This could be easily misunderstood, the ISA tagging isn't done
> explicitly.

Yes, I'll change this comment too.  This was before I noticed BFD was
setting the ISA tagging itself.

> > +	  if (file_mips_gp32 < 0)
> > +	    {
> > +	      if (ISA_HAS_64BIT_REGS (mips_opts.isa))
> > +		file_mips_gp32 = 0;
> > +	      else
> > +		file_mips_gp32 = 1;
> > +	    }
> > +	  if (file_mips_fp32 < 0)
> > +	    {
> > +	      if (ISA_HAS_64BIT_REGS (mips_opts.isa))
> > +		file_mips_fp32 = 0;
> > +	      else
> > +		file_mips_fp32 = 1;
> > +	    }
> > +	  if (ISA_HAS_64BIT_REGS (mips_opts.isa))
> > +	    {
> > +	      if (mips_opts.abi == O32_ABI)
> > +		mips_opts.abi = NO_ABI;
> > +	    }
> 
> This breaks e.g. -march=r8000 -mips3 -mabi=32. Hopefully nobody
> needs it.

How?  Oh, wait, I see.  I don't really like the NO_ABI stuff but I was
just following what was already there.  If an ABI is specified on the
command line it behooves us to honor it.

I'm willing to let -march=r8000 -mips3 -mabi=32 fall by the roadside,
though.  -march=r3000 -mips3 -mabi=32 breaks in the same way and is
even sillier.

So, comments aside, any objection?

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer