This shows up in another bug too, see PR 19307 which uses "-msse2 -mno-80387" but I assume that 
this is the same bug.

Created attachment 7932 [details]
test case

This is the preprocessed output of newlib's e_atan2.c.	I cut out the cpp
directives and bzip'ed it.  

It does fail with the native x86 Linux gcc 4.0-20050109.  Please check if it is
the same as 19307 and update PRs/targets/cc's as you feel necessary.  

IMO this is important.	It breaks one of the most important targets for RTEMS.

Well, you have a problem.  What do you want the ABI for soft-float to be?
As RTEMS is probably the only user of -msoft-float, you get to choose.

Do you want -msoft-float to imply -mno-fp-ret-in-387, do you want to supply
it yourself, or do you want to admit that all shipping processors have an
fpu and/or the os has an emulator?

There really aren't many other options.

(In reply to comment #3)
> What do you want the ABI for soft-float to be?
> As RTEMS is probably the only user of -msoft-float, you get to choose.
-msoft-float basically is just a synomym for -no-80387 (-MASK_80387 in i386.h),
so there probably are more users.

> Do you want -msoft-float to imply -mno-fp-ret-in-387, do you want to supply
> it yourself, or do you want to admit that all shipping processors have an
> fpu and/or the os has an emulator?
Neither.

The actual problem is people using RTEMS on original i386dx's for whom previous
verions of gcc had required -mtune=i386 -mno-80387 -no-fp-ret-in-387 rsp.
-mcpu=i386 -msoft-float -no-fp-in-387.

For other cpus, coupling -msoft-float with -no-fp-in-387 has proven not to be
necessary.

This all is the reason for RTEMS gcc to use this kind of multilibs:
i386-rtems4.7-gcc --print-multi-lib
.;
m486;@mtune=i486
mpentium;@mtune=pentium
mpentiumpro;@mtune=pentiumpro
k6;@mtune=k6
athlon;@mtune=athlon
soft-float;@msoft-float
soft-float/nofp;@msoft-float@mno-fp-ret-in-387
m486/soft-float;@mtune=i486@msoft-float

(In reply to comment #4)
> (In reply to comment #3)
> > What do you want the ABI for soft-float to be?
> > As RTEMS is probably the only user of -msoft-float, you get to choose.
> -msoft-float basically is just a synomym for -no-80387 (-MASK_80387 in i386.h),
> so there probably are more users.

If you are tuly using soft-float, then the results can't be returned in the
non-existent FPU registers so I have never understood from a practical matter
why it didn't already imply avoiding returning values in FPU registers.
 
> > Do you want -msoft-float to imply -mno-fp-ret-in-387, do you want to supply
> > it yourself, or do you want to admit that all shipping processors have an
> > fpu and/or the os has an emulator?
> Neither.

Richard gave four options.  

  (1) -msoft-float to imply -mno-fp-ret-in-387
  (2) supply "it" yourself (?? do you mean explicitly state -mno-fp-ret-in-387)
  (3) admit that all shipping processors have an FPU
  (4) add an FPU emulator to RTEMS

(3) is quite an assertion.  There are lots of x86 clones now and many of those
are targetted to the embedded space.  All desktop x86 CPUs do have FPUs but
all x86's do not target that space.  Even Intel still ships some no FPU x86
variants for embedded space
(http://developer.intel.com/design/intarch/intel386/index.htm).  Ignoring that,
this choice would result in us being unable to provide OS upgrades to users of
non-FPU x86 CPUs.

(4) is not likely to happen.  It hasn't been needed yet and it is unlikely a 
volunteer is going to show up just because gcc suddenly needs one after 10
years of not needing it. 

(2) I am not sure what  this means exactly but if the user has to provide 
an extra argument, it is error prone and the multilib match has to be there
anyway. 

This leaves us with (1).  If that is all that is required to fix the problem,
then (a) how hard is this to implement and (b) what is the potential impact
on user code?  RTEMS, all support code, and all application would be
recompiled with the new ABI so that is OK.  The impact would be any assembly
routines.  I doubt there are many.

> The actual problem is people using RTEMS on original i386dx's for whom previous
> verions of gcc had required -mtune=i386 -mno-80387 -no-fp-ret-in-387 rsp.
> -mcpu=i386 -msoft-float -no-fp-in-387.
> 
> For other cpus, coupling -msoft-float with -no-fp-in-387 has proven not to be
> necessary.

I checked the i386ex BSPs and one used 
-msoft-float -mno-fp-ret-in-387 and the other only used -msoft-float.
The pc386dx BSP uses both options.

I am willing to go with (1).  Can this be done all the time
for -msoft-float?  Just change i386.h like this:

{ "soft-float",		(-MASK_80387|-MASK_FLOAT_RETURNS|, N_("Do not use hardware fp")
},  \

Is that enough?


> This all is the reason for RTEMS gcc to use this kind of multilibs:
> i386-rtems4.7-gcc --print-multi-lib
> .;
> m486;@mtune=i486
> mpentium;@mtune=pentium
> mpentiumpro;@mtune=pentiumpro
> k6;@mtune=k6
> athlon;@mtune=athlon
> soft-float;@msoft-float
> soft-float/nofp;@msoft-float@mno-fp-ret-in-387
> m486/soft-float;@mtune=i486@msoft-float

(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > What do you want the ABI for soft-float to be?
> > > As RTEMS is probably the only user of -msoft-float, you get to choose.
> > -msoft-float basically is just a synomym for -no-80387 (-MASK_80387 in i386.h),
> > so there probably are more users.
> 
> If you are tuly using soft-float, then the results can't be returned in the
> non-existent FPU registers so I have never understood from a practical matter
> why it didn't already imply avoiding returning values in FPU registers.
This is not how i386-gcc-rtems is set up until now.

> > > Do you want -msoft-float to imply -mno-fp-ret-in-387, do you want to supply
> > > it yourself, or do you want to admit that all shipping processors have an
> > > fpu and/or the os has an emulator?
> > Neither.
> 
> Richard gave four options.  
> 
>   (1) -msoft-float to imply -mno-fp-ret-in-387
IMO, this proposal is meaningless unless
-mno-80387 also is changed to imply -mno-fp-ret-in-387.

>   (2) supply "it" yourself (?? do you mean explicitly state -mno-fp-ret-in-387)
>   (3) admit that all shipping processors have an FPU
>   (4) add an FPU emulator to RTEMS
> 
> (3) is quite an assertion.
ACK, this is not feasible.

> (4) is not likely to happen.
Hmm? What is -msoft-float -mno-fp-ret-in-387 using, then?

> This leaves us with (1).
Cf. above.

Theoretically, letting -msoft-float imply -mno-fp-ret-in-387 should provide a
multilib variant applicable to i386dx's. 
 
> > The actual problem is people using RTEMS on original i386dx's for whom previous
> > verions of gcc had required -mtune=i386 -mno-80387 -no-fp-ret-in-387 rsp.
> > -mcpu=i386 -msoft-float -no-fp-in-387.
> > 
> > For other cpus, coupling -msoft-float with -no-fp-in-387 has proven not to be
> > necessary.
> 
> I checked the i386ex BSPs and one used 
> -msoft-float -mno-fp-ret-in-387
The pc386dx BSP uses this one.

> and the other only used -msoft-float.
> The pc386dx BSP uses both options.
No, the pc386 BSP uses this one.

The source code is identical, the only differnence is -mno-fp-ret-in-387

> I am willing to go with (1).  Can this be done all the time
> for -msoft-float?  Just change i386.h like this:
> 
> { "soft-float",		(-MASK_80387|-MASK_FLOAT_RETURNS|, N_("Do not use hardware fp")
> },  \
> 
> Is that enough?
To provide RTEMS with a work-around, yes, this should be sufficient.
However, cf. above. IMO, this is not sufficient for GCC.

(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > (In reply to comment #3)
> > If you are tuly using soft-float, then the results can't be returned in the
> > non-existent FPU registers so I have never understood from a practical matter
> > why it didn't already imply avoiding returning values in FPU registers.
> This is not how i386-gcc-rtems is set up until now.

Right but there is nothing preventing it from changing and all BSPs with
recent test reports specify -mno-fp-ret-in-387 flag anyway.   Since we don't
have a simulator which handle the FP not present trap handler, this is
really the only one which can work.
 
> > > > Do you want -msoft-float to imply -mno-fp-ret-in-387, do you want to supply
> > > > it yourself, or do you want to admit that all shipping processors have an
> > > > fpu and/or the os has an emulator?
> > > Neither.
> > 
> > Richard gave four options.  
> > 
> >   (1) -msoft-float to imply -mno-fp-ret-in-387
> IMO, this proposal is meaningless unless
> -mno-80387 also is changed to imply -mno-fp-ret-in-387.
> 
> >   (2) supply "it" yourself (?? do you mean explicitly state -mno-fp-ret-in-387)
> >   (3) admit that all shipping processors have an FPU
> >   (4) add an FPU emulator to RTEMS
> > 
> > (3) is quite an assertion.
> ACK, this is not feasible.

I don't even think it is an accurate reflection of shipping CPUs today.

> > (4) is not likely to happen.
> Hmm? What is -msoft-float -mno-fp-ret-in-387 using, then?

By FPU emulator I mean (and assume Richard means) a handler for the FPU
not present (trap #7) handler.  We don't have one so we have to eliminate
all FPU operations.  

Unfortunately, I think soft-float doesn't completely eliminate all uses of
the FPU so you have to add the no-fp-ret-in-387 argument.

> > This leaves us with (1).
> Cf. above.
> 
> Theoretically, letting -msoft-float imply -mno-fp-ret-in-387 should provide a
> multilib variant applicable to i386dx's. 

That's what I think and it gives us exactly what BSPs are specifying now.
It looks like in practical terms, when you really need soft-float, you need
no-fp-ret-in-i387 since you are saying there is no i387 and you are 
emulating one via a trap handler.
  
> > > The actual problem is people using RTEMS on original i386dx's for whom
previous
> > > verions of gcc had required -mtune=i386 -mno-80387 -no-fp-ret-in-387 rsp.
> > > -mcpu=i386 -msoft-float -no-fp-in-387.
> > > 
> > > For other cpus, coupling -msoft-float with -no-fp-in-387 has proven not to be
> > > necessary.
> > 
> > I checked the i386ex BSPs and one used 
> > -msoft-float -mno-fp-ret-in-387
> The pc386dx BSP uses this one.
> 
> > and the other only used -msoft-float.
> > The pc386dx BSP uses both options.
> No, the pc386 BSP uses this one.
> 
> The source code is identical, the only differnence is -mno-fp-ret-in-387
> 
> > I am willing to go with (1).  Can this be done all the time
> > for -msoft-float?  Just change i386.h like this:
> > 
> > { "soft-float",		(-MASK_80387|-MASK_FLOAT_RETURNS|, N_("Do not use hardware fp")
> > },  \
> > 
> > Is that enough?
> To provide RTEMS with a work-around, yes, this should be sufficient.
> However, cf. above. IMO, this is not sufficient for GCC.

Ignoring my syntax error, in the typing above.  Richard has said RTEMS is
the only target who cares about soft-float.  If this is a workable 
solution, then I say we go for it.  I am trying it now and will report in.

(In reply to comment #7)
> (In reply to comment #6)

> > > If you are tuly using soft-float, then the results can't be returned in the
> > > non-existent FPU registers so I have never understood from a practical matter
> > > why it didn't already imply avoiding returning values in FPU registers.
> > This is not how i386-gcc-rtems is set up until now.
> 
> Right but there is nothing preventing it from changing and all BSPs with
> recent test reports specify -mno-fp-ret-in-387 flag anyway.

> Unfortunately, I think soft-float doesn't completely eliminate all uses of
> the FPU so you have to add the no-fp-ret-in-387 argument.

True, but ... -msoft-float == -mno-80387

The RTEMS user using the i386dx had proven this not to work on original i386dx
w/o i387.

However, he has proven -msoft-float -mno-fp-ret-in-387 to work for him.

=> IMO, the actual fix would be to merge MASK_FLOAT_RETURNS into MASK_80387 and
to abandon MASK_FLOAT_RETURNS.

> Richard has said RTEMS is
> the only target who cares about soft-float.
I don't agree to his statement.

In reply to comment #5:

Perhaps I am out of touch with what's extant in the embedded space.
I havn't been paid to care about that in quite some time.  I'll defer.

Using "-MASK_80387|-MASK_FLOAT_RETURNS" is incorrect.  It should
be "-(MASK_80387|MASK_FLOAT_RETURNS)".  I agree with Ralf's later
comment that -mno-80387 and -msoft-float should probably stay in
sync.  Though of course -msoft-float doesn't really mean as much
as it sounds like it ought in conjunction with -mfpmath.

In reply to comment #7:

Yes, I did mean trap #7 handler for an fpu emulator.  Though of
course there are different levels of emulation.  One thing that
has been done before for MIPS is to provide a trap handler, but
only implement the move instructions.  This allows the ABI to
remain unchanged by pretending that the appropriate registers
exist, but not having to incur most of the overhead for trapping
for all arithmetic instructions.

Since RTEMS is multilibed, this is much less of an advantage.

In any case, Joel, would you please take ownership of this bug
and see it through?  You're the one that would have to be doing
the RTEMS testing anyway...

(In reply to comment #9)
> In reply to comment #5:
> 
> Perhaps I am out of touch with what's extant in the embedded space.
> I havn't been paid to care about that in quite some time.  I'll defer.

It is hard to keep up with all the new CPUs. Unfortunately, it  
isn't always about what is still recommended for new
design.  Old stuff lives on forever and I hate to see it become
unsupported.

> Using "-MASK_80387|-MASK_FLOAT_RETURNS" is incorrect.  It should
> be "-(MASK_80387|MASK_FLOAT_RETURNS)".  I agree with Ralf's later
> comment that -mno-80387 and -msoft-float should probably stay in
> sync.

How does this look?

Index: i386.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/i386/i386.h,v
retrieving revision 1.417
diff -u -r1.417 i386.h
--- i386.h	11 Jan 2005 21:33:13 -0000	1.417
+++ i386.h	13 Jan 2005 01:41:28 -0000
@@ -333,9 +333,11 @@
 
 #define TARGET_SWITCHES							
      \
 { { "80387",			 MASK_80387, N_("Use hardware fp") },	      \
-  { "no-80387",			-MASK_80387, N_("Do not use hardware fp") },  \
+  { "no-80387",			-(MASK_80387|MASK_FLOAT_RETURNS), \
+     N_("Do not use hardware fp") },  \
   { "hard-float",		 MASK_80387, N_("Use hardware fp") },	      \
-  { "soft-float",		-MASK_80387, N_("Do not use hardware fp") },  \
+  { "soft-float",		-(MASK_80387|MASK_FLOAT_RETURNS), \
+    N_("Do not use hardware fp") },  \
   { "no-soft-float",		 MASK_80387, N_("Use hardware fp") },	      \
   { "386",			 0, "" /*Deprecated.*/},		      \
   { "486",			 0, "" /*Deprecated.*/},		      \

>  Though of course -msoft-float doesn't really mean as much
> as it sounds like it ought in conjunction with -mfpmath.

No it doesn't but at least it looks like the code that parses those 
arguments will correctly complain if you try to use it with
soft-float or no-387 set.

> In reply to comment #7:
> 
> Yes, I did mean trap #7 handler for an fpu emulator.  Though of
> course there are different levels of emulation.  One thing that
> has been done before for MIPS is to provide a trap handler, but
> only implement the move instructions.  This allows the ABI to
> remain unchanged by pretending that the appropriate registers
> exist, but not having to incur most of the overhead for trapping
> for all arithmetic instructions.

I had thought this might be how some did it.  But why when the
entire store could just as easily been avoided by the compiler?
 
> Since RTEMS is multilibed, this is much less of an advantage.

Exactly.  We will add a multilib for a performance advantage 
in addition to an instruction mismatch.

> In any case, Joel, would you please take ownership of this bug
> and see it through?  You're the one that would have to be doing
> the RTEMS testing anyway...

As long as you promise to review it. :)

I think the above patch is very close anyway.

(In reply to comment #10)
>    { "hard-float",		 MASK_80387, N_("Use hardware fp") },	      \
> -  { "soft-float",		-MASK_80387, N_("Do not use hardware fp") },  \
> +  { "soft-float",		-(MASK_80387|MASK_FLOAT_RETURNS), \
> +    N_("Do not use hardware fp") },  \

One thing that I notice about this is that -msoft-float and -mhard-float are
no longer inverses.  Perhaps it would be better to modify override_options to
notice when, after all options are processed, that MASK_80387 is off and then
turn off MASK_FLOAT_RETURNS to match.

(In reply to comment #11)
> (In reply to comment #10)
> One thing that I notice about this is that -msoft-float and -mhard-float are
> no longer inverses. 
Good point. How about these alternatives:

1. Systematically substitute all occurences of MASK_80387 with
(MASK_80387|MASK_FLOAT_RETURN) in i386.h.
This would keep -msoft-float and -mno-80387, rsp. -mno-soft-float, -mhard-float
and -m80387 as synonyms.
IMO, this would be the least intrusive way of a proper fix.

2. Completely eliminate MASK_FLOAT_RETURN and to use 
MASK_80837, instead.  This would be a pretty radical way, I am not sure about
its consquences, but I don't expect it to do much harm.

3. Keep -[no-]hard-float, -m[no-]80387 and -m[no-]fp-ret-in-387 as they
currently are, but only change soft-float to (MASK_80387|MASK_FLOAT_RETURN) and
-mno-soft-float to -(MASK_80387|MASK_FLOAT_RETURN).
This would satisfy RTEMS without disturbing other targets/OSes (We'd have to
change our multilibs, but this wouldn't be an issue).
I'd consider this to be a more a hack than an actual fix ;)

4. Fix the cause of this PR. I.e. prevent GCC from generating FPU code in the
case this breakdown occurred. Unfortunately, I don't know the cause nor how to
work around it.

> Perhaps it would be better to modify override_options to
> notice when, after all options are processed, that MASK_80387 is off and then
> turn off MASK_FLOAT_RETURNS to match.
I don't understand. Could you elaborate?

I would consider (1) wrong.

I'm not sure about (2); I think at one time there was a target that put fp
return values in the integer registers even when a coprocessor was present.
But there doesn't seem to be such a target in the tree now.  Perhaps it got
purged.

I don't like (3) because that forces -mhard-float to imply MASK_FLOAT_RETURN,
which would interfere with (2) if it still existed.

As for (4), what do you think the problem *is* anyway?  It's the fact
that "fxch" is marked TARGET_80387, and the only reason *that* got emitted
is to put the return value in the right place for MASK_FLOAT_RETURN.  Duh.

And my suggestion is to add

  if (!TARGET_80387)
    target_flags &= ~MASK_FLOAT_RETURNS;

somewhere in the middle of override_options.  Preferably near the other 
bits that talk about fpmath etc.

(In reply to comment #13)
> As for (4), what do you think the problem *is* anyway?

To me the problem is:
"i386-rtems-gcc-4.0 ices when building the '-msoft-float -mtune=i386' multilib
variant."

Now the observation is, the '-soft-float -mtune=i386 -fno-fp-ret-in-387'
multilib variant to be building fine.

From this I conclude, that i386-*gcc-4.0 probably has a bug somewhere which
causes it to generate incorrect code for '-msoft-float -mtune=i386'. 

I.e. I would be satisfied if we can manage to find a way to let the
"-msoft-float -mtune=i386" multilib variant imply -fno-fp-ret-in-387".

My proposals above were along the consideration of
* Users reported "-fno-80387" not to be sufficient for real i386s w/o i387
* gcc-4.0 also seems to have encountered problems with it
.. so why not merge "-mno-80387" and "-no-fp-ret-in-387"?

> It's the fact
> that "fxch" is marked TARGET_80387, and the only reason *that* got emitted
> is to put the return value in the right place for MASK_FLOAT_RETURN.  Duh.
Sorry, I am as much an i386 expect to be able to comment on this.

> And my suggestion is to add
> 
>   if (!TARGET_80387)
>     target_flags &= ~MASK_FLOAT_RETURNS;
> 
> somewhere in the middle of override_options.  Preferably near the other 
> bits that talk about fpmath etc.
I need some more time to think about this.

Created attachment 7976 [details]
patch based upon RH';s suggestion

I think this implements what Richard has asked for in override.

I should have mentioned that I can build and install a C/C++ toolset.  If you
think this is OK, can I commit it?

Yes, the patch in comment 15 is ok.

(In reply to comment #17)
> Yes, the patch in comment 15 is ok.

I just committed it.  I hope it is still OK. :)

If you think it's OK, this can be closed.

Subject: Bug 19379

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	joel@gcc.gnu.org	2005-01-18 14:13:23

Modified files:
	gcc            : ChangeLog 
	gcc/config/i386: i386.c 

Log message:
	2005-01-18  Joel Sherrill <joel.sherrill@oarcorp.com>
	
	PR target/19379
	* config/i386/i386.c (override_options): If the 80387 is disabled,
	then do not return FP values using FP registers.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.7175&r2=2.7176
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/i386/i386.c.diff?cvsroot=gcc&r1=1.780&r2=1.781

Fixed.