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

[Richard Sandiford <rdsandiford at googlemail dot com>]

Matthew Fortune <Matthew.Fortune@imgtec.com> writes:
> 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.

Yeah, by "rely on architecture tests to catch problems" I meant
"diagnose the problem when odd registers are seen on architectures
that don't have them".  Wasn't very clear, sorry.

>> 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.

I was thinking of it the other way around: set the flag if the odd registers
might be used and OR the input flags.  I think that fits the scheme better,
with the bit saying whether a particular architecture feature
(independent odd-numbered registers) is used.

Whether we assume 0 or 1 for old objects is kind-of a separate question.
I suppose there are arguments both ways...

Thanks,
Richard