[Bug middle-end/35509] [4.3/4.4 Regression] builtin isinf() mismatch to compile-time substitution
--- Comment #15 from ghazi at gcc dot gnu dot org 2008-05-19 06:16 --- Addressed this by adding __builtin_isinf_sign() -- ghazi at gcc dot gnu dot org changed: What|Removed |Added Status|RESOLVED|REOPENED Resolution|INVALID | http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35509
[Bug middle-end/35509] [4.3/4.4 Regression] builtin isinf() mismatch to compile-time substitution
-- ghazi at gcc dot gnu dot org changed: What|Removed |Added AssignedTo|unassigned at gcc dot gnu |ghazi at gcc dot gnu dot org |dot org | Status|REOPENED|ASSIGNED Last reconfirmed|2008-04-13 16:27:57 |2008-05-19 06:16:49 date|| Target Milestone|4.3.1 |4.4.0 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35509
[Bug middle-end/35509] [4.3/4.4 Regression] builtin isinf() mismatch to compile-time substitution
--- Comment #16 from ghazi at gcc dot gnu dot org 2008-05-19 06:19 --- Fixed in 4.4.0 by adding a new builtin for systems that care about the sign of isinf's return value. E.g. do this: #define isinf(x) __builtin_isinf_sign(x) -- ghazi at gcc dot gnu dot org changed: What|Removed |Added Status|ASSIGNED|RESOLVED Known to work||4.4.0 Resolution||FIXED http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35509
[Bug middle-end/35509] [4.3/4.4 Regression] builtin isinf() mismatch to compile-time substitution
--- Comment #13 from ghazi at gcc dot gnu dot org 2008-05-18 15:51 --- Patch to implement a separate builtin isinf_sign posted here: http://gcc.gnu.org/ml/gcc-patches/2008-05/msg01067.html -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35509
[Bug middle-end/35509] [4.3/4.4 Regression] builtin isinf() mismatch to compile-time substitution
--- Comment #14 from ghazi at gcc dot gnu dot org 2008-05-18 23:20 --- Subject: Bug 35509 Author: ghazi Date: Sun May 18 23:19:38 2008 New Revision: 135517 URL: http://gcc.gnu.org/viewcvs?root=gccview=revrev=135517 Log: PR middle-end/35509 * builtins.c (mathfn_built_in_1): Renamed from mathfn_built_in. Add `implicit' parameter. Handle BUILT_IN_SIGNBIT. (mathfn_built_in): Rewrite in terms of mathfn_built_in_1. (fold_builtin_classify): Handle BUILT_IN_ISINF_SIGN. (fold_builtin_1): Likewise. * builtins.def (BUILT_IN_ISINF_SIGN): New. c-common.c (check_builtin_function_arguments): Handle BUILT_IN_ISINF_SIGN. * doc/extend.texi: Document __builtin_isinf_sign. * fold-const.c (operand_equal_p): Handle COND_EXPR. testsuite: * gcc.dg/builtins-error.c: Test __builtin_isinf_sign. * gcc.dg/tg-tests.h: Likewise. Mark variables volatile. * gcc.dg/torture/builtin-isinf_sign-1.c: New test. Modified: trunk/gcc/ChangeLog trunk/gcc/builtins.c trunk/gcc/builtins.def trunk/gcc/c-common.c trunk/gcc/doc/extend.texi trunk/gcc/fold-const.c trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/gcc.dg/builtins-error.c trunk/gcc/testsuite/gcc.dg/tg-tests.h -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35509
[Bug middle-end/35509] [4.3/4.4 Regression] builtin isinf() mismatch to compile-time substitution
--- Comment #12 from mmitchel at gcc dot gnu dot org 2008-04-28 04:37 --- I don't see that this is a bug, given that the compiler's optimization is within the bounds set by the C standard. I agree with Kaveh's comment that it is desirable that C libraries be able to implement external functions in terms of the builtin, if they choose. If you disagree with that conclusion, please raise the issue on [EMAIL PROTECTED] If there is consensus that QoI dictates that we should do something different, then we can reopen this issue. -- mmitchel at gcc dot gnu dot org changed: What|Removed |Added Status|NEW |RESOLVED Resolution||INVALID http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35509
[Bug middle-end/35509] [4.3/4.4 Regression] builtin isinf() mismatch to compile-time substitution
--- Comment #11 from rguenther at suse dot de 2008-04-15 08:58 --- Subject: Re: [4.3/4.4 Regression] builtin isinf() mismatch to compile-time substitution On Tue, 15 Apr 2008, ghazi at gcc dot gnu dot org wrote: --- Comment #10 from ghazi at gcc dot gnu dot org 2008-04-15 01:36 --- (In reply to comment #8) I propose that we do the following: - add a warning to the C/C++ frontends that isinf (x) CMP isinf (y) is only well-defined for !isinf (x) !isinf (y). this doesn't result in a runtime penalty and informs people about the possible problem in their code (it is non-portable). IMHO the above warning would have limited coverage because there are other ways users can test the results of isinf that would run into the sign problem. Well, of course. A different workaround would be to canonicalize the return value of isinf to bool. Doesn't this change the interface defined by c99? Yes, we would need to hide that fact and always add promotion code to int via isinf() != 0. Richard. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35509
[Bug middle-end/35509] [4.3/4.4 Regression] builtin isinf() mismatch to compile-time substitution
--- Comment #7 from dmixm at marine dot febras dot ru 2008-04-14 10:55 --- Possible the (isgreater(fabs(x), DBL_MAX) ? (signbit(x) ? -1 : 1) : 0) will be fast with common finite numbers? Question: in case of AVR target, is it possible to call extern isinf() function regardless of language dialect? Inline code is very undesirable with small memory. Thanks. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35509
[Bug middle-end/35509] [4.3/4.4 Regression] builtin isinf() mismatch to compile-time substitution
--- Comment #8 from rguenth at gcc dot gnu dot org 2008-04-14 11:06 --- I propose that we do the following: - add a warning to the C/C++ frontends that isinf (x) CMP isinf (y) is only well-defined for !isinf (x) !isinf (y). this doesn't result in a runtime penalty and informs people about the possible problem in their code (it is non-portable). A different workaround would be to canonicalize the return value of isinf to bool. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35509
[Bug middle-end/35509] [4.3/4.4 Regression] builtin isinf() mismatch to compile-time substitution
--- Comment #9 from ghazi at gcc dot gnu dot org 2008-04-15 00:38 --- (In reply to comment #7) Possible the (isgreater(fabs(x), DBL_MAX) ? (signbit(x) ? -1 : 1) : 0) will be fast with common finite numbers? Yes, it seems to be slightly faster (~5%), but you get larger code generated. I think the best thing would be to find a way to fold if (isinf(x)) to the boolean version and other uses could remain as the sign dependent one. Question: in case of AVR target, is it possible to call extern isinf() function regardless of language dialect? Inline code is very undesirable with small memory. Thanks. Yes, you can override the middle-end builtins using your own or delete them altogether. See config/pa/pa.c:pa_init_builtins(). If you want an extern isinf, you'll need some magic in the header file to define a macro based on the type size. The nice thing about gcc's type-generic builtins is that you can define isinf(x) to __builtin_isinf(x) and it'll work for all three types: float, double or long double. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35509
[Bug middle-end/35509] [4.3/4.4 Regression] builtin isinf() mismatch to compile-time substitution
--- Comment #10 from ghazi at gcc dot gnu dot org 2008-04-15 01:36 --- (In reply to comment #8) I propose that we do the following: - add a warning to the C/C++ frontends that isinf (x) CMP isinf (y) is only well-defined for !isinf (x) !isinf (y). this doesn't result in a runtime penalty and informs people about the possible problem in their code (it is non-portable). IMHO the above warning would have limited coverage because there are other ways users can test the results of isinf that would run into the sign problem. A different workaround would be to canonicalize the return value of isinf to bool. Doesn't this change the interface defined by c99? -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35509
[Bug middle-end/35509] [4.3/4.4 Regression] builtin isinf() mismatch to compile-time substitution
--- Comment #6 from ghazi at gcc dot gnu dot org 2008-04-13 16:27 --- So did we decide to fix this or not? FWIW, I could fix the generic isinf transformation using: isinf(x) - isless(x,DBL_MAX) ? -1 : (isgreater(x,DBL_MAX)) and just always take the runtime penalty calculating for the -inf case when the user says if (isinf(x)) and doesn't care about the sign. Maybe __builtin_expect preferring zero would help here. (?) I have an preliminary patch to do the ?: transformation, however the obtabs isinf versions will still fail to honor sign. This makes trouble in the testcases if some platforms get -1 and others get 1. So switching it would need to be coordinated. Thoughts? -- ghazi at gcc dot gnu dot org changed: What|Removed |Added Last reconfirmed|2008-03-08 10:40:32 |2008-04-13 16:27:57 date|| http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35509
[Bug middle-end/35509] [4.3/4.4 Regression] builtin isinf() mismatch to compile-time substitution
--- Comment #1 from rguenth at gcc dot gnu dot org 2008-03-08 10:40 --- We run into the machine-independent inline expansion that replaces isinf(x) with isgreater(fabs(x),DBL_MAX). This is indeed inconsistend with the constant folding we do. -- rguenth at gcc dot gnu dot org changed: What|Removed |Added Status|UNCONFIRMED |NEW Component|target |middle-end Ever Confirmed|0 |1 Keywords||wrong-code Last reconfirmed|-00-00 00:00:00 |2008-03-08 10:40:32 date|| Summary|[avr] 4.3.0: builtin isinf()|[4.3/4.4 Regression] builtin |mismatch to compile-time|isinf() mismatch to compile- |substitution|time substitution Target Milestone|--- |4.3.1 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35509
[Bug middle-end/35509] [4.3/4.4 Regression] builtin isinf() mismatch to compile-time substitution
--- Comment #2 from rguenth at gcc dot gnu dot org 2008-03-08 10:52 --- Note that one might argue that the testcase is invalid as as you say, C99 only says isinf returns non-zero. So strictly speaking comparing the return value of two invocations is not a way to check if both values are an infinity. Of course this is at least an QOI issue. The question is what behavior to retain? The glibc manpage documents returning -1 for -Inf and +1 for +Inf, which we could preserve on glibc targets then not expanding via isgreater(fabs(x),DBL_MAX). (Only i386 and s390 have target dependent expansions for isinf - what is their behavior here?) Kaveh, you introduced the generic fallback, Andreas, what does s390 return here, Uros, how is the situation on i386? -- rguenth at gcc dot gnu dot org changed: What|Removed |Added CC||ghazi at gcc dot gnu dot ||org, uros at gcc dot gnu dot ||org, rguenth at gcc dot gnu ||dot org, krebbel at gcc dot ||gnu dot org http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35509
[Bug middle-end/35509] [4.3/4.4 Regression] builtin isinf() mismatch to compile-time substitution
--- Comment #3 from ghazi at gcc dot gnu dot org 2008-03-08 15:45 --- (In reply to comment #2) Note that one might argue that the testcase is invalid as as you say, C99 only says isinf returns non-zero. So strictly speaking comparing the return value of two invocations is not a way to check if both values are an infinity. Of course this is at least an QOI issue. The question is what behavior to retain? The glibc manpage documents returning -1 for -Inf and +1 for +Inf, which we could preserve on glibc targets then not expanding via isgreater(fabs(x),DBL_MAX). When I wrote this, I was relying on the minimal C99 requirements, and Rth's blessing here: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30652#c2 One of my goals in writing these builtins was that GCC could always rely on the inline expansion, perhaps allowing libc implementors to just define isinf into the builtin. I'd like to keep that as much as possible. So on glibc systems, instead of bypassing the expansion, perhaps we could instead enhance it to return the sign dependent value using builtin signbit to determine the sign and paste it (bitop it) into the result. Another option would be to inline a variant of the ieee glibc isinf function code which seems to be very small. I don't know how generic it is. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35509
[Bug middle-end/35509] [4.3/4.4 Regression] builtin isinf() mismatch to compile-time substitution
--- Comment #4 from ubizjak at gmail dot com 2008-03-08 15:51 --- (In reply to comment #2) (Only i386 and s390 have target dependent expansions for isinf - what is their behavior here?) Kaveh, you introduced the generic fallback, Andreas, what does s390 return here, Uros, how is the situation on i386? When compiled on 32bit target with -std=c99, testcase aborts. For !c99, library call is emitted and the test passes OK. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35509
[Bug middle-end/35509] [4.3/4.4 Regression] builtin isinf() mismatch to compile-time substitution
--- Comment #5 from pinskia at gcc dot gnu dot org 2008-03-08 15:55 --- I don't think this is even a QOI bug. The values are defined by the C99 standard. If the person used -std=c89, then he would get what the target's libc defines them. The inconstaint value is ok at different optimization level too because only non-zero and zero are defined as return values. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35509
