Summary: | [4.5 Regression] c-c++-common/pr41779.c doesn't work | ||
---|---|---|---|
Product: | gcc | Reporter: | H.J. Lu <hjl.tools> |
Component: | c | Assignee: | Manuel López-Ibáñez <manu> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | gcc-bugs, jsm28, manu |
Priority: | P3 | ||
Version: | 4.5.0 | ||
Target Milestone: | 4.5.0 | ||
Host: | Target: | ||
Build: | Known to work: | 4.4.3 | |
Known to fail: | 4.5.0 | Last reconfirmed: | 2010-02-23 00:27:14 |
Description
H.J. Lu
2010-02-20 07:12:11 UTC
What are the excess messages? Subject: Re: c-c++-common/pr41779.c doesn't work Sent from my iPhone On Feb 20, 2010, at 2:40 AM, "manu at gcc dot gnu dot org" <gcc-bugzilla@gcc.gnu.org > wrote: > > > ------- Comment #1 from manu at gcc dot gnu dot org 2010-02-20 > 10:40 ------- > What are the excess messages? The problem is simple c does nit have overloaded functions. I am testing the obvious patch which fixes this; renaming the functions. > > > -- > > manu at gcc dot gnu dot org changed: > > What |Removed |Added > --- > --- > ---------------------------------------------------------------------- > Status|UNCONFIRMED |NEW > Ever Confirmed|0 |1 > Last reconfirmed|0000-00-00 00:00:00 |2010-02-20 10:40:49 > date| | > > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43128 > Subject: Bug 43128 Author: manu Date: Sat Feb 20 11:51:02 2010 New Revision: 156924 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=156924 Log: 2010-02-20 Manuel López-Ibáñez <manu@gcc.gnu.org> PR 43128 * c-c++-common/pr41779.c: Fix broken testcase. Modified: trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/c-c++-common/pr41779.c Andrew, Sorry I read your message after hitting commit! Is it fixed now? Fixed. On Linux/ia32, I still got Executing on host: /export/gnu/import/svn/gcc-test/bld/gcc/xgcc -B/export/gnu/import/svn/gcc-test/bld/gcc/ /export/gnu/import/svn/gcc-test/src-trunk/gcc/testsuite/c-c++-common/pr41779.c -Wc++-compat -std=c99 -Wconversion -S -o pr41779.s (timeout = 300) FAIL: c-c++-common/pr41779.c -Wc++-compat (test for warnings, line 30) PASS: c-c++-common/pr41779.c -Wc++-compat (test for excess errors) It may be an ILP32/32bit HW INT issue. It also failed on Linux/x86-64 with -m32: FAIL: c-c++-common/pr41779.c -Wc++-compat (test for warnings, line 30) It is C99 and ILP32: [hjl@gnu-6 gcc]$ ./xgcc -B./ -S /export/gnu/import/git/gcc/gcc/testsuite/c-c++-common/pr41779.c -Wconversion -m32 -std=c99 [hjl@gnu-6 gcc]$ ./xgcc -B./ -S /export/gnu/import/git/gcc/gcc/testsuite/c-c++-common/pr41779.c -Wconversion -m32 /export/gnu/import/git/gcc/gcc/testsuite/c-c++-common/pr41779.c: In function ‘f5’: /export/gnu/import/git/gcc/gcc/testsuite/c-c++-common/pr41779.c:30:3: warning: conversion to ‘float’ from ‘int’ may alter its value [hjl@gnu-6 gcc]$ On Linux/ia32, -std=c99 changes silently float to long double and we don't get a warning. It is a regression from gcc 4.4. [hjl@gnu-6 gcc]$ cat /tmp/x.i float f5(float x, int y) { return x * y; } [hjl@gnu-6 gcc]$ gcc -S /tmp/x.i -Wconversion -m32 -std=c99 /tmp/x.i: In function ‘f5’: /tmp/x.i:3: warning: conversion to ‘float’ from ‘int’ may alter its value [hjl@gnu-6 gcc]$ ./xgcc -B./ -S /tmp/x.i -Wconversion -m32 -std=c99 [hjl@gnu-6 gcc]$ I may have miscompared the bootstrap results to miss this because I do test -m32. Yes, the excess precision stuff changes the common type to long double in build_binary_op in C and -m32 (but not in C++!). Joseph, is this the correct thing to do or a latent bug? Subject: Re: [4.5 Regression] c-c++-common/pr41779.c doesn't work There is a technical bug here, in that the semantics I intended to implement and said I was implementing were that implicit conversions from integers to floating-point types result in a value representable in the new type. It's not the missing warning that's the bug - right now there is a conversion to long double of a value representable in long double, so the lack of warning is in accordance with the semantics implemented. What is a bug is that the semantics implemented as not as intended. That they are not proper excess precision semantics (and remember that no previous release had proper excess precision semantics, so this is barely a regression) relies on such implicit conversions not being "operations with floating operands" and so not being liable to have excess precision under 5.2.4.2.2 paragraph 8. (In reply to comment #12) > Subject: Re: [4.5 Regression] c-c++-common/pr41779.c doesn't > work Sorry I do not understand completely your answer. Shouldn't we warn at all? If the semantics were implemented as intended, should we warn? Also, the casting to long double seems to be introduced by the float, not by the integer, if I debugged the code correctly. That is true that the previous release didn't have proper excess precision semantics. But from the user perspective, the previous release handles -- float f5(float x, int y) { return x * y; } -- correctly with "-Wconversion -std=c99". For this particular testcase, gcc 4.5 has a user visible regression. Subject: Re: [4.5 Regression] c-c++-common/pr41779.c doesn't work On Sun, 21 Feb 2010, manu at gcc dot gnu dot org wrote: > Sorry I do not understand completely your answer. Shouldn't we warn at all? If > the semantics were implemented as intended, should we warn? With the intended semantics, we should warn; there would be an actual conversion from integer to float there, that could change the value. (In reply to comment #15) > > With the intended semantics, we should warn; there would be an actual > conversion from integer to float there, that could change the value. Great. Any hints where could be the problem or fix? Subject: Re: [4.5 Regression] c-c++-common/pr41779.c doesn't work On Sun, 21 Feb 2010, manu at gcc dot gnu dot org wrote: > ------- Comment #16 from manu at gcc dot gnu dot org 2010-02-21 18:25 ------- > (In reply to comment #15) > > > > With the intended semantics, we should warn; there would be an actual > > conversion from integer to float there, that could change the value. > > Great. Any hints where could be the problem or fix? Where convert_and_check is called, for (a) conditional expressions and (b) binary operations, to convert two operands to a common type, the conversion is (correctly) to the type with excess precision (possibly long double where the semantic type is narrower, float or double). I would suggest having a c_ep_convert_and_check or similar function that handles excess precision: it would take the result type, the semantic result type (the type that gets used eventually to build an EXCESS_PRECISION_EXPR) and the value to convert. It would just call convert_and_check, ignoring the semantic type, *except* when the operand has integer type *and* the semantic type is non-NULL; in that case, it would first convert to the semantic type them to the result type. (In reply to comment #17) > suggest having a c_ep_convert_and_check or similar function that handles > excess precision: it would take the result type, the semantic result type > (the type that gets used eventually to build an EXCESS_PRECISION_EXPR) and > the value to convert. It would just call convert_and_check, ignoring the > semantic type, *except* when the operand has integer type *and* the > semantic type is non-NULL; in that case, it would first convert to the > semantic type them to the result type. Wouldn't that change the normal result of promotion rules? Also, why call convert_and_check ignoring the semantic type and not just call convert? The excess precision type should be large enough to not cause any problem that needs checking. > Subject: Re: [4.5 Regression] c-c++-common/pr41779.c doesn't work On Mon, 22 Feb 2010, manu at gcc dot gnu dot org wrote: > ------- Comment #18 from manu at gcc dot gnu dot org 2010-02-22 23:56 ------- > (In reply to comment #17) > > suggest having a c_ep_convert_and_check or similar function that handles > > excess precision: it would take the result type, the semantic result type > > (the type that gets used eventually to build an EXCESS_PRECISION_EXPR) and > > the value to convert. It would just call convert_and_check, ignoring the > > semantic type, *except* when the operand has integer type *and* the > > semantic type is non-NULL; in that case, it would first convert to the > > semantic type them to the result type. > > Wouldn't that change the normal result of promotion rules? No. The present logic is: convert (with convert_and_check) both operands to a common type, which may have excess precision; then, later, after producing the tree for the result of the operation, wrap that in an EXCESS_PRECISION_EXPR, using the semantic type, if there is a semantic type different from the type with excess precision. The proposed logic is the same, *except* that the conversion to a common type goes via the semantic type, *if* there is excess precision involved *and* the operand being converted had integer type. > Also, why call convert_and_check ignoring the semantic type and not just call > convert? The excess precision type should be large enough to not cause any > problem that needs checking. Yes, you could just use convert for the second conversion (semantic type to type with excess precision). (In reply to comment #19) > > The proposed logic is the same, *except* that the conversion to a common > type goes via the semantic type, *if* there is excess precision involved > *and* the operand being converted had integer type. In order to know the common_type I need to move the test for build_type before the calls to convert_and_check. What is build_type for? (In reply to comment #19) > > The present logic is: convert (with convert_and_check) both operands to a > common type, which may have excess precision; then, later, after producing > the tree for the result of the operation, wrap that in an > EXCESS_PRECISION_EXPR, using the semantic type, if there is a semantic > type different from the type with excess precision. This is only true for build_binary_op. In build_conditional_expr both operands are converted to the semantic type first, if I am reading the code correctly, so there is nothing to fix (in fact, I cannot build a testcase that misses the warning when using conditional expression). The code of build_binary_op is a bit complex, specially the interaction between result_type, final_type, buid_type, converted and real_result_type. Subject: Re: [4.5 Regression] c-c++-common/pr41779.c doesn't work On Tue, 23 Feb 2010, manu at gcc dot gnu dot org wrote: > ------- Comment #20 from manu at gcc dot gnu dot org 2010-02-23 08:39 ------- > (In reply to comment #19) > > > > The proposed logic is the same, *except* that the conversion to a common > > type goes via the semantic type, *if* there is excess precision involved > > *and* the operand being converted had integer type. > > In order to know the common_type I need to move the test for build_type before > the calls to convert_and_check. What is build_type for? build_type is for comparisons, when the (semantic) type of the result of the expression is not the same as the (semantic) common type of the operands. Subject: Re: [4.5 Regression] c-c++-common/pr41779.c doesn't work On Tue, 23 Feb 2010, manu at gcc dot gnu dot org wrote: > ------- Comment #21 from manu at gcc dot gnu dot org 2010-02-23 10:23 ------- > (In reply to comment #19) > > > > The present logic is: convert (with convert_and_check) both operands to a > > common type, which may have excess precision; then, later, after producing > > the tree for the result of the operation, wrap that in an > > EXCESS_PRECISION_EXPR, using the semantic type, if there is a semantic > > type different from the type with excess precision. > > This is only true for build_binary_op. In build_conditional_expr both operands > are converted to the semantic type first, if I am reading the code correctly, > so there is nothing to fix (in fact, I cannot build a testcase that misses the > warning when using conditional expression). No, the conversion is to the type with excess precision. Try: float a; int b; int c; long double f(void) { return c ? a + a : b; } If you use "a" instead of "a + a", then it's just a float rather than a long double wrapped in an EXCESS_PRECISION_EXPR. build_binary_op explicitly adds excess precision early on for various operations where it is necessary to describe the semantics of what the processor can do. But there is no need to add excess precision for a conditional expression - just to handle operands that already have it. OK, I get it. Thanks for the explanation. Testing a patch. Subject: Bug 43128 Author: manu Date: Wed Feb 24 13:09:37 2010 New Revision: 157040 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=157040 Log: 2010-02-24 Manuel López-Ibáñez <manu@gcc.gnu.org> PR c/43128 * c-typeck.c (ep_convert_and_check): New. (build_conditional_expr): Use it. (build_binary_op): Likewise. testsuite/ * c-c++-common/pr41779.c: Update. Modified: trunk/gcc/ChangeLog trunk/gcc/c-typeck.c trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/c-c++-common/pr41779.c It should be FIXED for GCC 4.5 |