RE: [PATCH, MIPS] Ensure default ASEs are applied for an architecture

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

Maciej Rozycki <Maciej.Rozycki@imgtec.com> writes:
> On Wed, 22 Mar 2017, Maciej W. Rozycki wrote:
> 
> > > @@ -3918,8 +3926,6 @@ mips_check_options (struct mips_set_options *opts, bfd_boolean
> abi_checks)
> > >  static void
> > >  file_mips_check_options (void)
> > >  {
> > > -  const struct mips_cpu_info *arch_info = 0;
> > > -
> > >    if (file_mips_opts_checked)
> > >      return;
> > >
> > > @@ -3962,8 +3968,6 @@ file_mips_check_options (void)
> > >  	file_mips_opts.fp = 32;
> > >      }
> > >
> > > -  arch_info = mips_cpu_info_from_arch (file_mips_opts.arch);
> > > -
> > >    /* Disable operations on odd-numbered floating-point registers by default
> > >       when using the FPXX ABI.  */
> > >    if (file_mips_opts.oddspreg < 0)
> > > @@ -4007,7 +4011,7 @@ file_mips_check_options (void)
> > >
> > >    /* If the user didn't explicitly select or deselect a particular ASE,
> > >       use the default setting for the CPU.  */
> > > -  file_mips_opts.ase |= (arch_info->ase & ~file_ase_explicit);
> > > +  file_mips_opts.ase |= (file_mips_opts.init_ase & ~file_ase_explicit);
> >
> >  This is the first place where `file_mips_opts.ase' is ever set AFAICT, so
> > I think all that you need here is:
> >
> >   file_mips_opts.ase &= ~file_ase_explicit;
> >
> > (and `arch_info' can of course go, as you did), with the hunk below
> > changed accordingly:
> >
> > > @@ -14769,6 +14773,7 @@ mips_after_parse_args (void)
> > >
> > >    file_mips_opts.arch = arch_info->cpu;
> > >    file_mips_opts.isa = arch_info->isa;
> > > +  file_mips_opts.init_ase = arch_info->ase;
> >
> >   file_mips_opts.ase = arch_info->ase;
> >
> > The rest of the change can then go.  Please verify that this does what's
> > intended.
> 
>  Hmm, I missed how `mips_set_ase' infers the ASE list set on the command
> line.  Apologies for misleading you.
> 
>  I still think we don't want to have an extra `init_ase' member only used
> once.  I think we should reinstate the global `file_ase' variable removed
> with commit 919731affbef (and complementing `file_ase_explicit' still
> present there) and adjust the API of `mips_set_ase' so that it operates on
> `unsigned int *ase_flags' instead of `struct mips_set_options *opts', and
> then adjust call sites accordingly to refer to `&file_ase' or
> `&mips_opts.ase' as appropriate.  Then in `file_mips_check_options':
> 
>    file_mips_opts.ase &= ~file_ase_explicit;
>    file_mips_opts.ase |= file_ase;
> 
> will do.  Can you implement this?

I'll need to think about this more as although initially I thought your
proposal was going to clean up the solution it has some issues. The main one
being that file_ase, if re-introduced, must be used only to track the ASEs
coming from the selected arch not the explicitly selected ASEs.  This is
because the logic needed in file_mips_check_options is to set up the
overall ASEs to be anything that was explicitly selected and bits from the
architecture ASEs that were not explicitly disabled.  In the long run, and
to some extent just to solve this issue, we are going to want to track all
of:

1) Which ASEs have been explicitly enabled/disabled
2) Which ASEs are enabled overall
3) Which ASEs the chosen architecture enables by default

This data is needed on a global and per .push stack level to calculate the
appropriate ASEs at any given time. (1) needs a new mips_set_options field
and the global file_ase_explicit moving to file_mips_opts.ase_explicit. (2)
is mips_set_options 'ase' field. (3) would be a new field but could actually
be implemented by removing 'arch' and 'isa' from mips_set_options and instead
just keeping a reference to the mips_cpu_info entry for the selected arch
which would then get the ASE list for free. (this also solves the name issue
for warnings and errors).

I believe this scheme would also directly help with having .set arch affect
the ASEs as you indicated should be done with follow up work.

Hope that makes some sense; I can try and elaborate or do a draft implementation
if not.

Matthew