[Bug c/50315] New: Regreesion on Atom after fix #49958

[Bug c/50315] New: Regreesion on Atom after fix #49958

[vbyakovl23 at gmail dot com]

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

             Bug #: 50315
           Summary: Regreesion on Atom after fix #49958
    Classification: Unclassified
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: vbyakovl23@gmail.com


Created attachment 25215
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=25215
Test case

After fix #49958 was lost reassociation that caused regression on Atom.

I attached a test case (test.c) and dumps for good (test.c.003t.original.g) and
bad (test.c.003t.original.b) cases. 

Regression is on expression at lines 16-47.

[Bug c/50315] Regreesion on Atom after fix #49958

[vbyakovl23 at gmail dot com]

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

--- Comment #1 from Vladimir Yakovlev <vbyakovl23 at gmail dot com> 2011-09-07 09:32:11 UTC ---
Created attachment 25216
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=25216
Dump before fix

[Bug c/50315] Regreesion on Atom after fix #49958

[vbyakovl23 at gmail dot com]

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

--- Comment #2 from Vladimir Yakovlev <vbyakovl23 at gmail dot com> 2011-09-07 09:33:20 UTC ---
Created attachment 25217
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=25217
Dump after fix

[Bug c/50315] Regression on Atom after fix #49958

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2011-09-07
     Ever Confirmed|0                           |1

--- Comment #3 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-09-07 10:34:43 UTC ---
Not obvious where (+ (+ (* a b) c) (* d e)) -> (+ (+ (* a b) (* d e)) c)
would have happened from looking at the testcase, but it looks like after
the patch the expressions are left-associative while before they are
randomly paired.  After the patch we match what is present in the source.

There is no way to recover from that, the transform is invalid.  Re-association
is not possible for integer types with undefined overflow unless
1) the no-undefined-overflow branch materializes, or 2) you re-associate by
transforming the arithmetic to twos-complement (unsigned).

Well, "confirmed".  Sort-of.

[Bug c/50315] Regression on Atom after fix #49958

[sergos.gnu at gmail dot com]

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

Sergey Ostanevich <sergos.gnu at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |sergos.gnu at gmail dot com

--- Comment #4 from Sergey Ostanevich <sergos.gnu at gmail dot com> 2011-09-07 13:56:30 UTC ---
Richard,

Will it be a good idea to have a twos-complement architecture hook? In case of
x86 we can reassociate since the architecture itself always behave as
twos-complement. So introducing such a flag can help with this particular
reassociation and another one that Ilya Enkovich implemented recently.

What's your opinion?

[Bug c/50315] Regression on Atom after fix #49958

[joseph at codesourcery dot com]

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

--- Comment #5 from joseph at codesourcery dot com <joseph at codesourcery dot com> 2011-09-07 14:40:07 UTC ---
On Wed, 7 Sep 2011, sergos.gnu at gmail dot com wrote:

> Will it be a good idea to have a twos-complement architecture hook? In case of

All architectures supported by GCC are twos-complement.

> x86 we can reassociate since the architecture itself always behave as
> twos-complement. So introducing such a flag can help with this particular
> reassociation and another one that Ilya Enkovich implemented recently.

The ability to reassociate does not depend on architecture features.

C is a high-level language whose defined semantics are that signed integer 
overflow is undefined, independent of any target architecture.

GENERIC and GIMPLE are internal representations in GCC that follow the 
same semantics for signed integer overflow as C does, independent of 
target architecture.

RTL is an internal representation in GCC where signedness is a property of 
operations rather than operands and overflow generally has modulo 
semantics (division by 0, signed division and modulo of INT_MAX by -1 are 
still undefined on RTL).  So you can reassociate on RTL but not GENERIC 
and GIMPLE.

[Bug c/50315] Regression on Atom after fix #49958

[rguenth at gcc dot gnu.org]

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

--- Comment #6 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-09-07 15:09:47 UTC ---
(In reply to comment #5)
> On Wed, 7 Sep 2011, sergos.gnu at gmail dot com wrote:
> 
> > Will it be a good idea to have a twos-complement architecture hook? In case of
> 
> All architectures supported by GCC are twos-complement.
> 
> > x86 we can reassociate since the architecture itself always behave as
> > twos-complement. So introducing such a flag can help with this particular
> > reassociation and another one that Ilya Enkovich implemented recently.
> 
> The ability to reassociate does not depend on architecture features.
> 
> C is a high-level language whose defined semantics are that signed integer 
> overflow is undefined, independent of any target architecture.
> 
> GENERIC and GIMPLE are internal representations in GCC that follow the 
> same semantics for signed integer overflow as C does, independent of 
> target architecture.

On the no-undefined-overflow branch there are tree codes for arithmetic
where we know overflow does not occur (for both signed and unsigned types).
So you can go from (a +no b) +no c to a + (b + c), dropping overflow
knowledge on re-association.

[Bug middle-end/50315] Regression on Atom after fix #49958

[sergos.gnu at gmail dot com]

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

--- Comment #7 from Sergey Ostanevich <sergos.gnu at gmail dot com> 2011-09-15 11:24:27 UTC ---
Richard, I believe your test should be reading as 

> So you can go from (a +no b) +no c to a + no (b + c), dropping overflow
knowledge on re-association.

And let me re-phrase what's Joseph said (just to be sure I got the idea):
we have to preserve the overflow semantics at GIMPLE level to avoid possible
problems during translation into RTL. 

Consider we have situation without overflow in 32-bit with particular
calculation order and can use either 32-bit or 64-bit operations to perform
that. But after reassociation in GIMPLE we can introduce overflow for 32-bit,
that will lead to wrong result in case we use 64-bit operations. 

Being aware of such situation during traslation we can evade error, but it
requires too much effort (or even impossible) to provide this data to the
translator. 

Is it right?