Bug 54130 - Recognize builtins with bool return type
Summary: Recognize builtins with bool return type
Status: UNCONFIRMED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 4.8.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-07-30 20:43 UTC by Marc Glisse
Modified: 2016-01-08 04:54 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Marc Glisse 2012-07-30 20:43:24 UTC
Hello,

extern "C" int isnan(double);
int f(){return isnan(3);}

is optimized to "return 0;" because isnan is recognized as a builtin. However, if I change the program to:

extern "C" bool isnan(double);
int f(){return isnan(3);}

the optimization is not done. I haven't seen any example in gcc sources of a builtin that may be declared with several different prototypes.
Comment 1 Richard Biener 2012-07-31 09:32:18 UTC
I think your non-matching prototype disables builtin recognition.  The C
standard specifies isnan as returning int, so I think GCC is correct here.
Comment 2 Marc Glisse 2012-07-31 09:38:38 UTC
(In reply to comment #1)
> I think your non-matching prototype disables builtin recognition.

Yes.

> The C standard specifies isnan as returning int, so I think GCC is correct here.

Note the "component: C++" for this bug. The C++11 standard says it returns bool. I believe we want gcc to recognize both prototypes.
Comment 3 joseph@codesourcery.com 2012-07-31 14:43:18 UTC
In C it's a macro not a function and there is no guarantee that there 
exists a function with that name, or what the semantics of such a function 
would be.  In C++, unlike C, I think you are required to include the 
standard headers to get any standard library facilities.  (For C, you can 
use functions, not macros, without including headers if the functions' 
prototypes do not involve any type defined in a standard header, only 
built-in C types.)
Comment 4 Marc Glisse 2012-07-31 19:20:09 UTC
Joseph,

I understand your comments, but I don't know what conclusion to take from them about whether gcc should recognize int/bool isnan(double) as a builtin...
Comment 5 joseph@codesourcery.com 2012-07-31 19:59:50 UTC
On Tue, 31 Jul 2012, glisse at gcc dot gnu.org wrote:

> I understand your comments, but I don't know what conclusion to take from them
> about whether gcc should recognize int/bool isnan(double) as a builtin...

Even for C, declaring library functions yourself is very much a legacy 
possibility, not good coding practice.  GCC matches builtins to declared 
functions primarily to optimize code using the standard headers, not code 
declaring them directly, and the cases where the type matching is fuzzy 
are for the sake of some old system headers.

If your system headers declare isnan with bool return type I advise making 
fixincludes fix them.  If the declaration doesn't come from system headers 
I don't think it's worth GCC optimizing for it.
Comment 6 Marc Glisse 2012-07-31 20:13:38 UTC
(In reply to comment #5)
> If your system headers declare isnan with bool return type I advise making 
> fixincludes fix them.

But the C++ standard requires a bool return type, using fixinclude to replace a standard prototype with a non-standard one is strange...
Comment 7 joseph@codesourcery.com 2012-07-31 22:12:51 UTC
On Tue, 31 Jul 2012, glisse at gcc dot gnu.org wrote:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54130
> 
> --- Comment #6 from Marc Glisse <glisse at gcc dot gnu.org> 2012-07-31 20:13:38 UTC ---
> (In reply to comment #5)
> > If your system headers declare isnan with bool return type I advise making 
> > fixincludes fix them.
> 
> But the C++ standard requires a bool return type, using fixinclude to replace a
> standard prototype with a non-standard one is strange...

C++ may also require functions rather than macros in various cases.  It's 
the job of the libstdc++ headers, provided by GCC, to provide a (probably 
inline) isnan function meeting the C++ requirements, replacing the C 
library's macro when building for C++.  Likewise all the other cases where 
C++ and C requirements for standard headers differ: the fix is in headers 
in one form or another rather than built in to the compiler.
Comment 8 Marc Glisse 2012-08-01 05:51:26 UTC
(In reply to comment #7)
> C++ may also require functions rather than macros in various cases.  It's 
> the job of the libstdc++ headers, provided by GCC, to provide a (probably 
> inline) isnan function meeting the C++ requirements, replacing the C 
> library's macro when building for C++.  Likewise all the other cases where 
> C++ and C requirements for standard headers differ: the fix is in headers 
> in one form or another rather than built in to the compiler.

That makes solving PR 48891 quite hard. glibc provides a function int isnan(double), which is illegal in C++. I was thinking of asking them to change the return type to bool in C++, then libstdc++ could just use that function (having gcc recognize isnan as a builtin is not a prerequisite for this, but it would avoid a regression). Another solution I can think of is that libstdc++ provides a math.h wrapper that does #define isnan __removed_isnan, then #include_next <math.h>, #undef isnan, and finally provides its own declaration of isnan (it might actually be better to do that first). If the C library doesn't #undef isnan and then declare an isnan function, that may work, but it may also break stuff if the C library uses isnan. I could also ask glibc to not declare the isnan function at all in C++, but that's a bit strange and other C libraries may define an isnan function.

All of those ideas are rather fragile. If you have better ideas about PR 48891, please do tell.


Note also that in C, we are currently failing to optimize this to a constant:
#include <math.h>
int f(){return isnan(3);}
Comment 9 Marc Glisse 2012-08-01 09:22:37 UTC
I realize that several (not all) of the things discussed here assume that functions returning bool and int are binary compatible, which is likely true on most platforms but there might be exceptions.
Comment 10 Richard Biener 2012-08-01 09:26:53 UTC
(In reply to comment #9)
> I realize that several (not all) of the things discussed here assume that
> functions returning bool and int are binary compatible, which is likely true on
> most platforms but there might be exceptions.

It's not true on x86_64 - return values are not extended to word_mode thus
you may have garbage in the upper parts of %eax for bool.
Comment 11 Richard Biener 2012-08-01 09:28:43 UTC
(In reply to comment #10)
> (In reply to comment #9)
> > I realize that several (not all) of the things discussed here assume that
> > functions returning bool and int are binary compatible, which is likely true on
> > most platforms but there might be exceptions.
> 
> It's not true on x86_64 - return values are not extended to word_mode thus
> you may have garbage in the upper parts of %eax for bool.

Testcase:

_Bool foo (_Bool *p)
{
  return *p;
}

compile at -Os and get

foo:
.LFB0:
        .cfi_startproc
        movb    (%rdi), %al
        ret

which I believe at least preserves %ah (not sure about bits 16 .. 32).
Comment 12 Marc Glisse 2012-08-01 09:49:12 UTC
(In reply to comment #10)
> (In reply to comment #9)
> > I realize that several (not all) of the things discussed here assume that
> > functions returning bool and int are binary compatible, which is likely true on
> > most platforms but there might be exceptions.
> 
> It's not true on x86_64 - return values are not extended to word_mode thus
> you may have garbage in the upper parts of %eax for bool.

Ok thanks, that seems to put the nail in the coffin for these techniques. I guess I should close this bug, and file the only part that remains (the missed optimization at the end of Comment 8) separately.
Comment 13 Marc Glisse 2012-08-01 09:52:19 UTC
(In reply to comment #10)
> (In reply to comment #9)
> > I realize that several (not all) of the things discussed here assume that
> > functions returning bool and int are binary compatible, which is likely true on
> > most platforms but there might be exceptions.
> 
> It's not true on x86_64 - return values are not extended to word_mode thus
> you may have garbage in the upper parts of %eax for bool.

Wait, actually, it only requires compatibility in one direction, that a function returning int (isnan for instance) can be used as a function returning bool, which seems ok in that case (there could still be other counter-examples).
Comment 14 joseph@codesourcery.com 2012-08-01 15:41:01 UTC
On Wed, 1 Aug 2012, glisse at gcc dot gnu.org wrote:

> may also break stuff if the C library uses isnan. I could also ask glibc to not
> declare the isnan function at all in C++, but that's a bit strange and other C
> libraries may define an isnan function.

The isnan function declaration is for compatibility with some old 
standards such as Unix98 that had such a function instead of the 
type-generic macro.  It would seem reasonable to disable it for C++, just 
as the glibc headers allow for C++ requirements for overloads in string.h, 
for example.  It is, however, really for the libstdc++ maintainers to work 
with the glibc maintainers regarding any changes to C headers that would 
be helpful for meeting C++ standard requirements; we can't make such a 
change in isolation without knowing it fits in with the libstdc++ 
maintainer plans.

> Note also that in C, we are currently failing to optimize this to a constant:
> #include <math.h>
> int f(){return isnan(3);}

The isnan macro definition is an old one, maybe predating GCC's 
type-generic __builtin_isnan, that calls functions such as __isnan, so 
effectively preventing such constant folding.  It would seem reasonable, 
given recent enough GCC versions, for it to use the type-generic built-in 
function instead, and likewise for various other such macros in glibc 
(maybe not fpclassify because of code size, and for isinf you have the 
complication of glibc providing additional semantics beyond the standard 
version).  If making such a change, it would seem appropriate also to 
change the internal calls to functions such as __isnan to call the macros 
instead, letting the compiler generate the most appropriate code.
Comment 15 Jonathan Wakely 2012-08-01 16:30:54 UTC
(In reply to comment #14)
> The isnan function declaration is for compatibility with some old 
> standards such as Unix98 that had such a function instead of the 
> type-generic macro.  It would seem reasonable to disable it for C++, just 
> as the glibc headers allow for C++ requirements for overloads in string.h, 
> for example.  It is, however, really for the libstdc++ maintainers to work 
> with the glibc maintainers regarding any changes to C headers that would 
> be helpful for meeting C++ standard requirements; we can't make such a 
> change in isolation without knowing it fits in with the libstdc++ 
> maintainer plans.

There are several areas I'd like to see more cooperation between glibc and libstdc++, I keep meaning to make a list, but haven't. Maybe we should start a page on the GCC wiki where suggestions can be noted until we have something concrete enough to discuss.