[Bug c/41577] Broken code with -O2

[Bug c/41577] Broken code with -O2

[magnus dot christensson at acm dot org]

------- Comment #1 from magnus dot christensson at acm dot org  2009-10-05 11:31 -------
Created an attachment (id=18707)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=18707&action=view)
-save-temps output


-- 


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

[Bug c/41577] New: Broken code with -O2

[magnus dot christensson at acm dot org]

I have narrowed down a problem in my application to the testcase which I will
soon attach.

The testcase should output:

Out r8: 0x82ce3350
Out cr: 0x8

Version of GCC: 4.3.4.

Command line which generates broken output: gcc -O2 compiler_issue.c
Command line which generates ok output: gcc -O2 -fno-tree-vrp compiler_issue.c


-- 
           Summary: Broken code with -O2
           Product: gcc
           Version: 4.3.4
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: magnus dot christensson at acm dot org
 GCC build triplet: x86_64-unknown-linux-gnu
  GCC host triplet: x86_64-unknown-linux-gnu
GCC target triplet: x86_64-unknown-linux-gnu


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

[Bug c/41577] Broken code with -O2

[magnus dot christensson at acm dot org]

------- Comment #2 from magnus dot christensson at acm dot org  2009-10-05 11:32 -------
Created an attachment (id=18708)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=18708&action=view)
C source code


-- 


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

[Bug c/41577] Broken code with -O2

[magnus dot christensson at acm dot org]

------- Comment #3 from magnus dot christensson at acm dot org  2009-10-05 11:34 -------
I suspect that the problem is that an optimization path does not properly
distinguish between signed and unsigned comparisons. You get the same behavior
if you remove the "int32 int_res = res;" line and let the following code
operate on "res".


-- 


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

[Bug tree-optimization/41577] Broken code with -O -ftree-vrp

[rguenth at gcc dot gnu dot org]

------- Comment #4 from rguenth at gcc dot gnu dot org  2009-10-05 11:59 -------
Confirmed.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
          Component|c                           |tree-optimization
     Ever Confirmed|0                           |1
           Keywords|                            |wrong-code
      Known to fail|                            |4.2.4 4.3.4
      Known to work|                            |4.1.2 4.4.1 4.5.0
   Last reconfirmed|0000-00-00 00:00:00         |2009-10-05 11:59:36
               date|                            |
            Summary|Broken code with -O2        |Broken code with -O -ftree-
                   |                            |vrp
   Target Milestone|---                         |4.3.5


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

[Bug tree-optimization/41577] Broken code with -O -ftree-vrp

[rguenth at gcc dot gnu dot org]

------- Comment #5 from rguenth at gcc dot gnu dot org  2009-10-05 11:59 -------
A workaround is to provide -fno-tree-vrp.


-- 


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

[Bug tree-optimization/41577] Broken code with -O -ftree-vrp

[rguenth at gcc dot gnu dot org]

------- Comment #6 from rguenth at gcc dot gnu dot org  2009-10-05 12:02 -------
Actually the testcase is invalid.

        uint32 res = ((uint16)(cpu->gprs[12]  >> 16) * (uint16)(cpu->gprs[16]
>> 16));

performs a signed multiplication which invokes undefined behavior if it
overflows.  Thus the compiler rightfully can assume that the result can
be converted to a positive signed value.


-- 

rguenth at gcc dot gnu dot org changed:

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


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

[Bug tree-optimization/41577] Broken code with -O -ftree-vrp

[rguenth at gcc dot gnu dot org]

------- Comment #7 from rguenth at gcc dot gnu dot org  2009-10-05 12:03 -------
A fix is to write

uint32 res = ((uint32)(uint16)(cpu->gprs[12]  >> 16)
              * (uint32)(uint16)(cpu->gprs[16] >> 16));

instead to force the multiplication in an unsigned type.


-- 


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

[Bug tree-optimization/41577] Broken code with -O -ftree-vrp

[magnus dot christensson at acm dot org]

------- Comment #8 from magnus dot christensson at acm dot org  2009-10-05 12:36 -------
(In reply to comment #7)
> A fix is to write
> 
> uint32 res = ((uint32)(uint16)(cpu->gprs[12]  >> 16)
>               * (uint32)(uint16)(cpu->gprs[16] >> 16));
> 
> instead to force the multiplication in an unsigned type.
> 

Thanks. It wasn't obvious to me that a multiply on two unsigned variables would
be promoted to signed. The uint32 trick will work here, but won't work if your
target machine has 64-bit integers.


-- 


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

[Bug tree-optimization/41577] Broken code with -O -ftree-vrp

[rguenth at gcc dot gnu dot org]

------- Comment #9 from rguenth at gcc dot gnu dot org  2009-10-05 13:09 -------
with 64bit int the multiplication will never overflow, so it would work there
as well.


-- 


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