RE: [PATCH][MIPS] Optionally disable odd-numbered single-precision registers

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

Richard Sandiford <rdsandiford@googlemail.com> writes:
> Matthew Fortune <Matthew.Fortune@imgtec.com> writes:
> > Richard Sandiford <rdsandiford@googlemail.com> writes:
> >> Matthew Fortune <Matthew.Fortune@imgtec.com> writes:
> >> > This patch adds support for disabling the use of odd-numbered
> >> > single-precision registers for arithmetic options. It aims
> >> > to implement the restriction on loongson-3a where these registers
> >> > cannot be used for single-precision operations (in FR=0/FP32 mode).
> >> > We have also decided to limit the number of single-precision
> >> > registers usable by default for generic MIPS architectures so that
> >> > generic code can run on the loongson-3a. Targeting a specific
> >> > architecture will however allow access to all registers by default.
> >> >
> >> > The corresponding (as yet un-committed) GCC patch is:
> >> > http://gcc.gnu.org/ml/gcc-patches/2014-04/msg01975.html
> >> >
> >> > After this patch is applied then pre-existing GCC compilers will
> >> > end up with warnings from the assembler when building mips32 or
> >> > mips32r2 code as the pre-existing compilers will use 32 single-
> >> > precision registers. Is this acceptable?
> >>
> >> Hmm, it sounds likely to surprise people.  Maybe this is one of
> >> the few cases where GCC and GAS should have different defaults.
> >> In the GCC case it's controlling whether the code uses odd
> >> registers, so -mno-oddspreg is the safe choice.  But in the
> >> GAS code it controls a warning, so -moddspreg is the safe choice.
> >
> > I agree it will surprise people. Though I would like to keep the
> > defaults for GCC and GAS the same, see below.
> >
> >> Maybe we should define ISA_HAS_ODD_SINGLE_FPR so that it takes
> >> the mips_opts.arch as well as the mips_opts.isa, then make it
> >> true for CPU_R5900 and false for CPU_LOONGSON_3A.  Then the error:
> >>
> >> > +  if (opts->fp != 64 && opts->arch == CPU_LOONGSON_3A && !opts-
> >> >nooddspreg)
> >> > +    as_bad (_("`arch=loongson3a' requires `nooddspreg' when using "
> >> > +	      "`fp=32' or `fp=xx'"));
> >>
> >> could apply whenever !ISA_HAS_ODD_SINGLE_FPR.  The default could also
> >> be set based on ISA_HAS_ODD_SINGLE_FPR.
> >
> > Given the way that the assembler handles odd-numbered registers today
> > I don't actually think we need an error at all. If the architecture
> > does not support odd-numbered singles then you will get a warning
> > during code generation anyway so -modd-spreg with loongson-3a can just
> > have no effect and a warning will be seen if odd singles are used.
> >
> > In order to keep GCC and GAS defaults the same (and avoid having to
> > set defaults in GCC specs which would be unpleasant) then I'd define
> > -mno-odd-spreg to be on by default for -mips* -mfpxx and do this for
> > both GCC and GAS. FPXX is after all the way to achieve compatible code
> > so it makes sense to just tie this default up with -mfpxx. A user can
> > still do -mips* -mfpxx -modd-spreg if they wish as well.
> 
> OK, just to make sure I understand, the old proposal was:
> 
> 1. imply -moddspreg for specific processors that support it
> 2. imply -mno-oddspreg for specific processors that don't support -moddspreg
> 3. retroactively imply -mno-oddspreg for generic architectures

Yes

> And the idea is to change that to
> 
> 1. imply -mno-oddspreg for generic architectures if -mfpxx
> 2. otherwise imply -moddspreg and rely on architecture-specific tests
>    to catch problems.

Almost except in the patch I am allowing -moddspreg with loongson but odd
registers still cannot be used because the architecture does not support them.
I think this is inkeeping with how odd registers are already enabled disabled
for different architecture levels it is just that I am now correctly recording
the fact that loongson does not have the registers. I.e. I do not expect it
to be an error to use -moddspreg with -march=mips2 but you will get an error
if you use and instruction that targets an odd spreg.


> Is that right?  If so, I agree that would work as far as the diagnostics go.
> But would it be useful to keep track of which objects are -moddspreg and
> which aren't, especially if we're using -mfpxx with both?  E.g. maybe we
> could use one of the flags1 bits in the abiflags structure.  If we did that
> then I imagine we'd want to know which -moddspreg setting was "really"
> in use.

I did think about this and I suspect the only reason I did not do it in the
end was actually laziness so I think I should add a flag. The overall
effect of the flag should record if all modules only use 16 registers so the
flag would have to be 'nooddspreg' and be AND (rather than OR as flags1 is
defined) so would need to go in flags2.

To determine whether oddspreg is really in force or not then the flag would
be set if "ISA_HAS_ODD_SINGLE_FPR && -moddspreg".

> I'm not strongly opposed to doing what the patch does, just thought I'd ask.
> 
> Keying the default off -mfpxx definitely sounds good to me though FWIW.

Excellent.

Regards,
Matthew