[Bug middle-end/49543] New: Cast move causes incorrect code and numerical results

[Bug middle-end/49543] New: Cast move causes incorrect code and numerical results

[acarmeli at mathworks dot com]

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

           Summary: Cast move causes incorrect code and numerical results
           Product: gcc
           Version: 4.6.0
            Status: UNCONFIRMED
          Severity: major
          Priority: P3
         Component: middle-end
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: acarmeli@mathworks.com


Created attachment 24606
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=24606
Folded and unfolded expressions calculate different results

Overview
--------

In the following example, "Data" is a 32-bit int, and "tmp" is a 64-bit int.

with gcc -ansi -pedantic the following line:

   tmp = (int64_T)(Data - 1) * 1805715409L; /* 32-bit subtraction */

is compiled as

   tmp = ((int64_T)Data - 1) * 1805715409L; /* 64-bit subtraction */

This changes the numerical result if Data-1 overflowed.

If the code was not folded, as in this example:

    int32_T hoist;
    hoist = Data - 1;
    tmp = (int64_T)hoist * 1805715409L;

the result in tmp is computed correctly.

The attached file cast_move_bug.c demonstrates this and has copiling
instructions.

The attached file cast_move_bug.s shows that the location of the up-cast to 64
with cltq instruction moved to *before* the subtraction (compared to the
unfolded version).

Triggering
----------
The bug was observed on all versions of gcc up to 4.6.0 on 64-bit Linux. 

When -pedantic *IS NOT* specified, the bug is triggered only with -O0. Under
-O1 and above the bug is not triggered.

When -pedantic *IS* specified, the bug is triggered under all optimization
levels: -O0 through -O3.

g++ 4.3.2 and 4.4.3 do not exhibit this bug.

Claim
-----
The up-cast to 64-bit was incorrectly moved inside the parentheses, violating
the effect of parenthesization on type sizes, and causing the subtraction to
become 64-bit.

Fix request
-----------
MathWorks requests the fix to be back-ported to 4.5 - a reasonably current, and
stable release within our customer community.

[Bug middle-end/49543] Cast move causes incorrect code and numerical results

[acarmeli at mathworks dot com]

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

--- Comment #1 from Alexander Carmeli <acarmeli at mathworks dot com> 2011-06-27 12:36:20 UTC ---
Created attachment 24607
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=24607
Assembly code demonstrating incorrect cltq instruction placement

[Bug middle-end/49543] Cast move causes incorrect code and numerical results

[pinskia at gcc dot gnu.org]

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

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> 2011-06-27 17:40:01 UTC ---
    /* Data - 1 overflows */

Yes that would mean it is undefined code.  Try -fwrapv.

[Bug middle-end/49543] Cast move causes incorrect code and numerical results

[acarmeli at mathworks dot com]

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

--- Comment #3 from Alexander Carmeli <acarmeli at mathworks dot com> 2011-06-27 18:40:25 UTC ---
Andrew,

You are correct about the standard not defining the result.

Similar behavior was fixed before (see bug 36300
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36300)

I think this is still a bug, for 3 reasons:

1. the standard also defines the subtraction operation and its result to be
32-bit. The compiler is not free to up-cast it prematurely to a bigger size for
optimization purposes. This was the same in 36300.

2. Relying on overflow behavior of 2's complement machines is extremely
popular. Having the same expression behave differently in folded and unfolded
expressions can lead to many unexpected behaviors. In addition:
2.1. Unoptimized code misbehaves, optimized code is well behaved - this is
unexpected.
2.2. The C++ compiler does generate the expected numerical behavior under all
conditions, without -fwrapv.

3. In my opinion, -pedantic should not affect compiler semantics. It should
only control warning messages (correct me if I got this wrong).


As for the result being undefined (value, not size), the standard primarily
keeps the options open for non-2's complement machines and machines with
non-traditional logic.

[Bug middle-end/49543] Cast move causes incorrect code and numerical results

[pinskia at gcc dot gnu.org]

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

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> 2011-06-27 19:25:37 UTC ---
Well C++ is not C, that is Data is a constant expression in C++ but not in C.

[Bug middle-end/49543] Cast move causes incorrect code and numerical results

[acarmeli at mathworks dot com]

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

--- Comment #5 from Alexander Carmeli <acarmeli at mathworks dot com> 2011-06-27 20:15:37 UTC ---
That's a good point. I removed the const and g++ fails as well.

Therefore, the bug is in the C++ compiler too.

Consts can be promoted as well. Why promote the non-const expression and not
promote the const expression? The const expression is constant-folded as a
32-bit subtraction. Therefore, so should the non-const expression. Otherwise,
numerical results depend on the arguments being const or not, and that's
problematic.

My claims still hold: 
- Expression folding should not change numerical results compared to
  unfolded expressions.
- It is not okay to promote to 64 bit. The value is undefined. The size is.
- If -pedantic compiles, it should generate he same results as without
  -pedantic.

[Bug middle-end/49543] Cast move causes incorrect code and numerical results

[rguenth at gcc dot gnu.org]

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

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |INVALID

--- Comment #6 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-06-28 09:31:35 UTC ---
There is undefined behavior if the overflow occurs, so the upcast is valid.