RE: [PATCH][MIPS] Implement .module directive

[Matthew Fortune <Matthew dot Fortune at imgtec dot com>]

Richard Sandiford <rdsandiford@googlemail.com> writes:
> Matthew Fortune <Matthew.Fortune@imgtec.com> writes:
> > +  if (file_mips_opts.fp < 0)
> > +    {
> > +      /* No user specified float register size.
> > +	 ??? 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 never smaller than the
> > +	 integer ones.  */
> > +      if (file_mips_opts.gp == 64)
> > +	/* 64-bit integer registers implies 64-bit float registers.  */
> > +	file_mips_opts.fp = 64;
> > +      else if ((file_mips_opts.ase & FP64_ASES)
> > +	       && ISA_HAS_64BIT_FPRS (file_mips_opts.isa))
> > +	/* -mips3d and -mdmx imply 64-bit float registers, if possible.  */
> > +	file_mips_opts.fp = 64;
> 
> I assume you'll update this for the other patch.  Or we could just
> make the comment vaguer: Handle ASEs that imply 64-bit float registers.

Yes, I thought I'd done these but clearly not.

> > +  /* MIPS3D and MDMX require 64-bit FPRs, so -mfp32 should stop those
> > +     ASEs from being selected implicitly.  */
> > +  if (file_mips_opts.fp != 64)
> > +    file_ase_explicit |= ASE_MIPS3D | ASE_MDMX | ASE_MSA;
> 
> Same here.
>
> > @@ -787,7 +787,7 @@ if { [istarget mips*-*-vxworks*] } {
> >      run_dump_test "relax-swap1-mips2"
> >      run_dump_test "relax-swap2"
> >      run_dump_test_arches "relax-swap3"	[mips_arch_list_all]
> > -    run_list_test_arches "relax-bc1any" "-mips3d -relax-branch" \
> > +    run_list_test_arches "relax-bc1any" "-mips3d -64 -relax-branch" \
> >  					[mips_arch_list_matching mips64 \
> >  					    !micromips]
> >      run_list_test_arches "relax-bposge" "-mdsp -relax-branch" \
> 
> Not all targets compile in support for n32 and n64, so -mabi=o64 would
> be better than -64.

Will do. I had only run the assembler tests for this patch but on running
the linker tests it showed a problem with .dc.a which is dependent on
TC_ADDRESS_BYTES and that in turn is dependent on the GPR size.  The
mips_address_bytes function therefore has to finalise the options (because
that is where the inferred GP register width is set up). This means that
.dc.a is another pseudo that prevents further .module directives.

Is that OK? (patch below to be applied along with the overall patch if
approved)

Regards,
Matthew

diff --git a/gas/config/tc-mips.c b/gas/config/tc-mips.c
index 0f89bb9..6a26b99 100644
--- a/gas/config/tc-mips.c
+++ b/gas/config/tc-mips.c
@@ -1307,6 +1307,7 @@ static bfd_boolean pic_need_relax (symbolS *, asection *);
 static int relaxed_branch_length (fragS *, asection *, int);
 static int relaxed_micromips_16bit_branch_length (fragS *, asection *, int);
 static int relaxed_micromips_32bit_branch_length (fragS *, asection *, int);
+static void file_mips_check_options (void);

 /* Table and functions used to map between CPU/ISA names, and
    ISA levels, and CPU numbers.  */
@@ -1760,6 +1761,7 @@ static const pseudo_typeS mips_nonecoff_pseudo_table[] =
 int
 mips_address_bytes (void)
 {
+  file_mips_check_options ();
   return HAVE_64BIT_ADDRESSES ? 8 : 4;
 }

@@ -3676,7 +3678,7 @@ mips_check_options (struct mips_set_options *opts, bfd_boolean abi_checks)
 /* Perform consistency checks on the module level options exactly once.
    This is a deferred check that happens:
      at the first .set directive
-     or, at the first pseudo op that generates code
+     or, at the first pseudo op that generates code (inc .dc.a)
      or, at the first instruction
      or, at the end.  */