[Bug c++/54130] New: Recognize builtins with bool return type

[Bug c++/54130] New: Recognize builtins with bool return type

[glisse at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54130

             Bug #: 54130
           Summary: Recognize builtins with bool return type
    Classification: Unclassified
           Product: gcc
           Version: 4.8.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: glisse@gcc.gnu.org


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.

[Bug c++/54130] Recognize builtins with bool return type

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54130

--- Comment #1 from Richard Guenther <rguenth at gcc dot gnu.org> 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.

[Bug c++/54130] Recognize builtins with bool return type

[glisse at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54130

--- Comment #2 from Marc Glisse <glisse at gcc dot gnu.org> 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.

[Bug c++/54130] Recognize builtins with bool return type

[joseph at codesourcery dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54130

--- Comment #3 from joseph at codesourcery dot com <joseph at codesourcery dot 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.)

[Bug c++/54130] Recognize builtins with bool return type

[glisse at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54130

--- Comment #4 from Marc Glisse <glisse at gcc dot gnu.org> 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...

[Bug c++/54130] Recognize builtins with bool return type

[joseph at codesourcery dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54130

--- Comment #5 from joseph at codesourcery dot com <joseph at codesourcery dot 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.

[Bug c++/54130] Recognize builtins with bool return type

[glisse at gcc dot gnu.org]

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

[Bug c++/54130] Recognize builtins with bool return type

[joseph at codesourcery dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54130

--- Comment #7 from joseph at codesourcery dot com <joseph at codesourcery dot 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.

[Bug c++/54130] Recognize builtins with bool return type

[glisse at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54130

--- Comment #8 from Marc Glisse <glisse at gcc dot gnu.org> 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);}

[Bug c++/54130] Recognize builtins with bool return type

[glisse at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54130

--- Comment #9 from Marc Glisse <glisse at gcc dot gnu.org> 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.

[Bug c++/54130] Recognize builtins with bool return type

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54130

--- Comment #10 from Richard Guenther <rguenth at gcc dot gnu.org> 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.

[Bug c++/54130] Recognize builtins with bool return type

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54130

--- Comment #11 from Richard Guenther <rguenth at gcc dot gnu.org> 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).

[Bug c++/54130] Recognize builtins with bool return type

[glisse at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54130

--- Comment #12 from Marc Glisse <glisse at gcc dot gnu.org> 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.

[Bug c++/54130] Recognize builtins with bool return type

[glisse at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54130

--- Comment #13 from Marc Glisse <glisse at gcc dot gnu.org> 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).

[Bug c++/54130] Recognize builtins with bool return type

[joseph at codesourcery dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54130

--- Comment #14 from joseph at codesourcery dot com <joseph at codesourcery dot 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.

[Bug c++/54130] Recognize builtins with bool return type

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54130

--- Comment #15 from Jonathan Wakely <redi at gcc dot gnu.org> 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.