This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH][MIPS] Implement O32 FPXX ABI (GCC)


Matthew Fortune <Matthew.Fortune@imgtec.com> writes:
> Richard Sandiford <rdsandiford@googlemail.com> writes:
>> Matthew Fortune <Matthew.Fortune@imgtec.com> writes:
>> >> > *) Because GCC can be built to have mfpxx or mfp64 as the default
>> option
>> >> >    the ASM_SPEC has to handle these specially such that they are not
>> >> >    passed in conjunction with -msingle-float. Depending on how all
>> this
>> >> >    option handling pans out then this may also need to account for
>> >> >    msoft-float as well. It is an error to have -msoft-float and -mfp64
>> in
>> >> >    the assembler.
>> >>
>> >> The assembler and GCC shouldn't treat the options differently though.
>> >> Either it should be OK for both or neither.
>> >
>> > I wasn't sure if you were applying this rule to options set by --with- at
>> > configure time as well as user supplied options. If I move this logic
>> > to OPTION_DEFAULT_SPECS instead and not apply the --with-fp option if
>> > -msingle-float is given then that will fix the issue too. Is that OK?
>> 
>> --with-foo is just a way of setting the default -mfoo option when no
>> explicit -mfoo option is given.  We shouldn't generally distinguish
>> between them.
>
> OK. I don't mind what we do here so I'm happy to just relax the (new)
> assembler restrictions that would prevent -msingle-float -mfp64. I need to
> check if these ended up in the .module binutils patch or are in the FPXX
> patch.

Let's step back a bit.  Please explain which case you were trying to
handle with the specs patch.  Rejecting -msingle-float -mfp64 seems fine.
Fiddling with the specs to stop that combination reaching the assembler
is what seemed odd.

>> >> > diff --git a/gcc/testsuite/gcc.target/mips/call-clobbered-2.c
>> >> b/gcc/testsuite/gcc.target/mips/call-clobbered-2.c
>> >> > new file mode 100644
>> >> > index 0000000..f47cdda
>> >> > --- /dev/null
>> >> > +++ b/gcc/testsuite/gcc.target/mips/call-clobbered-2.c
>> >> > @@ -0,0 +1,21 @@
>> >> > +/* Check that we handle call-clobbered FPRs correctly.  */
>> >> > +/* { dg-skip-if "code quality test" { *-*-* } { "-O0" } { "" } } */
>> >> > +/* { dg-options "-mabi=32 -modd-spreg -mfp32 -ffixed-f0 -ffixed-f1 -
>> >> ffixed-f2 -ffixed-f3 -ffixed-f4 -ffixed-f5 -ffixed-f6 -ffixed-f7 -
>> ffixed-f8
>> >> -ffixed-f9 -ffixed-f10 -ffixed-f11 -ffixed-f12 -ffixed-f13 -ffixed-f14 -
>> >> ffixed-f15 -ffixed-f16 -ffixed-f17 -ffixed-f18 -ffixed-f19 -ffixed-f20 -
>> >> ffixed-f22 -ffixed-f24 -ffixed-f26 -ffixed-f28 -ffixed-f30" } */
>> >> > +
>> >> > +void bar (void);
>> >> > +float a;
>> >> > +float
>> >> > +foo ()
>> >> > +{
>> >> > +  float b = a + 1.0f;
>> >> > +  bar();
>> >> > +  return b;
>> >> > +}
>> >> > +/* { dg-final { scan-assembler-times "lwc1" 2 } } */
>> >> > +/* { dg-final { scan-assembler-not "swc1" } } */
>> >> > +/* { dg-final { scan-assembler-times "sdc1" 2 } } */
>> >> > +/* { dg-final { scan-assembler-not "mtc" } } */
>> >> > +/* { dg-final { scan-assembler-not "mfc" } } */
>> >> > +/* { dg-final { scan-assembler-not "mthc" } } */
>> >> > +/* { dg-final { scan-assembler-not "mfhc" } } */
>> >>
>> >> Why require sdc1 here?  Would Chao-Ying's patch make this use SWC1 and
>> LWC1
>> >> exclusively?
>> >
>> > The SDC1 instructions are for callee-saved registers. I'll add the
>> > check for two corresponding LDC1 instructions in the epilogue though
>> > since I've noticed them being missing.
>> 
>> I'm probably being dense, sorry, but why is SDC1 needed for -mfp32
>> -modd-spreg?  Can't we just save and restore the odd register?
>> That's technically more correct too, since we shouldn't really touch
>> -ffixed registers.
>
> You are correct and I am/was aware of this...
>
> GCC is saving too much of the callee-saved registers when single-precision
> values are live in them but this is historic behaviour. The code which
> controls this is very complex and I'd be worried about breaking it. The
> fix would be invasive as all the logic is honed towards the notion of
> 64-bit callee-saved registers.

But that's because it was written before separate odd spregs came along.
I'm not convinced the situation is as bad as you say.

> One thing to say is that this test is
> a very aggressive test of ABI not normal compiler behaviour. Normally
> an even-numbered callee-saved register would be used first followed by the
> odd meaning that it is rare to only use the odd register. That flips the
> problem round to single precision data in even registers...
>
> I believe that prior to mips32 introducing odd-numbered single-precision
> registers then GCC will have been saving and restoring using sdc1/ldc1
> for even-numbered registers regardless of whether they are used for
> single or double precision data (for mips1 it also will do pairs of
> lwc1/swc1).

Sure.  And saving and restoring both using LDC1 and SDC1 is still
the right thing to do if both registers are clobbered.  But if only
one is clobbered then we should save just that.  It's really just
the same principle as Chao-ying's patch, but applied to the prologue
and epilogue.

> My vote FWIW is to leave this behaviour as is, if you would like it looking
> in to then it is a separate issue to FPXX I believe. Going forward with
> the FPXX ABI becoming the norm then we have to save/restore 64-bit registers
> anyway so the fix may not be of much value for end users.

It isn't really that separate from -modd-spreg.  We shouldn't add tests
for wrong output.

Thanks,
Richard


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]