Bug 43128 - [4.5 Regression] c-c++-common/pr41779.c doesn't work
Summary: [4.5 Regression] c-c++-common/pr41779.c doesn't work
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: 4.5.0
: P3 normal
Target Milestone: 4.5.0
Assignee: Manuel López-Ibáñez
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-02-20 07:12 UTC by H.J. Lu
Modified: 2010-02-24 13:10 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work: 4.4.3
Known to fail: 4.5.0
Last reconfirmed: 2010-02-23 00:27:14


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description H.J. Lu 2010-02-20 07:12:11 UTC
On Linux/ia32, I got

FAIL: c-c++-common/pr41779.c  -Wc++-compat   (test for warnings, line 30)
FAIL: c-c++-common/pr41779.c  -Wc++-compat  (test for excess errors)
Comment 1 Manuel López-Ibáñez 2010-02-20 10:40:49 UTC
What are the excess messages?
Comment 2 pinskia@gmail.com 2010-02-20 11:00:44 UTC
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
>
Comment 3 Manuel López-Ibáñez 2010-02-20 11:51:15 UTC
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

Comment 4 Manuel López-Ibáñez 2010-02-20 11:51:46 UTC
Andrew,

Sorry I read your message after hitting commit!

Is it fixed now?
Comment 5 Richard Biener 2010-02-20 13:52:14 UTC
Fixed.
Comment 6 H.J. Lu 2010-02-20 15:53:22 UTC
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.
Comment 7 H.J. Lu 2010-02-21 15:39:13 UTC
It also failed on Linux/x86-64 with -m32:

FAIL: c-c++-common/pr41779.c  -Wc++-compat   (test for warnings, line 30)
Comment 8 H.J. Lu 2010-02-21 16:15:52 UTC
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]$ 
Comment 9 H.J. Lu 2010-02-21 16:46:10 UTC
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.
Comment 10 H.J. Lu 2010-02-21 16:47:49 UTC
[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]$ 
Comment 11 Manuel López-Ibáñez 2010-02-21 17:25:23 UTC
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?
Comment 12 jsm-csl@polyomino.org.uk 2010-02-21 17:43:34 UTC
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.

Comment 13 Manuel López-Ibáñez 2010-02-21 17:57:27 UTC
(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.

Comment 14 H.J. Lu 2010-02-21 18:00:46 UTC
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.
Comment 15 jsm-csl@polyomino.org.uk 2010-02-21 18:15:56 UTC
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.

Comment 16 Manuel López-Ibáñez 2010-02-21 18:25:24 UTC
(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?
Comment 17 jsm-csl@polyomino.org.uk 2010-02-21 18:32:06 UTC
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.

Comment 18 Manuel López-Ibáñez 2010-02-22 23:56:20 UTC
(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.


> 

Comment 19 jsm-csl@polyomino.org.uk 2010-02-23 00:29:51 UTC
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).

Comment 20 Manuel López-Ibáñez 2010-02-23 08:39:06 UTC
(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?

Comment 21 Manuel López-Ibáñez 2010-02-23 10:23:25 UTC
(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.
Comment 22 jsm-csl@polyomino.org.uk 2010-02-23 16:32:11 UTC
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.

Comment 23 jsm-csl@polyomino.org.uk 2010-02-23 16:38:43 UTC
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.

Comment 24 Manuel López-Ibáñez 2010-02-23 17:33:50 UTC
OK, I get it. Thanks for the explanation. Testing a patch.
Comment 25 Manuel López-Ibáñez 2010-02-24 10:39:43 UTC
PATCH: http://gcc.gnu.org/ml/gcc-patches/2010-02/msg00969.html
Comment 26 Manuel López-Ibáñez 2010-02-24 13:09:51 UTC
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

Comment 27 Manuel López-Ibáñez 2010-02-24 13:10:37 UTC
It should be FIXED for GCC 4.5