[Bug c/101254] New: gcc head does not comply fully to -fwrapv

[Bug c/101254] New: gcc head does not comply fully to -fwrapv

[bugzilla.gnu at coelho dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101254

            Bug ID: 101254
           Summary: gcc head does not comply fully to -fwrapv
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: bugzilla.gnu at coelho dot net
  Target Milestone: ---

Created attachment 51079
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51079&action=edit
source file for above test

-fwrapv does not work as expected on gcc head:

Previous expected behavior :

  sh> gcc --version
  gcc (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0

  sh> gcc -O2 -fwrapv gcc_overflow.c 
  sh> ./a.out 
  1: 1
  0: 0
  0: 0

Behavior with head:

  sh> gcc --version
  gcc (GCC) 12.0.0 20210627 (461f937b)

  sh> gcc -O2 -fwrapv gcc_overflow.c 
  sh> ./a.out
  1: 1
  0: 0
  0: 1

But with the same :

  sh> gcc -O1 -fwrapv gcc_overflow.c 
  sh> ./a.out
  1: 1
  0: 0
  0: 0

Not that it seems that it was working fine with bc046a60 on 2021-06-18, so the
regression may have been introduced after this date.

[Bug tree-optimization/101254] [12 Regression] gcc head does not comply fully to -fwrapv since r12-1723

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101254

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|gcc head does not comply    |[12 Regression] gcc head
                   |fully to -fwrapv            |does not comply fully to
                   |                            |-fwrapv since r12-1723
                 CC|                            |aldyh at gcc dot gnu.org,
                   |                            |amacleod at redhat dot com,
                   |                            |jakub at gcc dot gnu.org
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2021-06-29
          Component|c                           |tree-optimization
   Target Milestone|---                         |12.0
     Ever confirmed|0                           |1

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Started with r12-1723-gae6b830f31a47aca7ca24c4fea245c29214eef3a
Adjusted testcase for testsuite:
/* PR tree-optimization/101254 */
/* { dg-do run } */
/* { dg-options "-O2 -fwrapv" } */

int
foo (long long imin, long long imax)
{
  if (imin > imax)
    return 0;
  else if (imax - imin < 0 || (imax - imin) + 1 < 0)
    return 0;
  return 1;
}

int
main ()
{
  long long imax = __LONG_LONG_MAX__;
  long long imin = -imax - 1; 
  if (!foo (-10, 10))
    __builtin_abort ();
  if (foo (-10, imax))
    __builtin_abort ();
  if (foo (imin, imax))
    __builtin_abort ();
  return 0;
}

[Bug tree-optimization/101254] [12 Regression] gcc head does not comply fully to -fwrapv since r12-1723

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101254

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
           Priority|P3                          |P1

[Bug tree-optimization/101254] [12 Regression] gcc head does not comply fully to -fwrapv since r12-1723

[amacleod at redhat dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101254

--- Comment #2 from Andrew Macleod <amacleod at redhat dot com> ---
Created attachment 51082
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51082&action=edit
patch

I *think* this is correct.   but wrapv and signed stuff sometimes confuses me
:-)
when -fwrapv is on,  +INf - -INF is -1 ?  is this correct?
in which case, the relations that were being added for minus were not quite
correct in this case.

given    lhs = op1 - op2
if op1 > op2, we were producing a range of [1, +INF].  which is fine for
unsigned.   but for signed, that edge condition for -INF means the range should
be [0, +INF].
likewise, if op1 >= op2, then result range would be [0, +INF]

Is this is correct?   I hope that  -INF - -INF still equals 0 tho?  Thats the
only other case that might need consideration.

The attached patch implements the above and appears to fix the test. I just
want to be sure I have it right.

[Bug tree-optimization/101254] [12 Regression] gcc head does not comply fully to -fwrapv since r12-1723

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101254

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
INT_MAX - INT_MIN with -fwrapv is -1, indeed, and generally if
x > y then x - y < 0 iff x > y + INT_MAX.
Say
x = 1073741839
y = -1073741887
x > y && x - y (== -2147483570) < 0

[Bug tree-optimization/101254] [12 Regression] gcc head does not comply fully to -fwrapv since r12-1723

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101254

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Anyway, I think you just can't assume anything for the SIGNED &&
TYPE_OVERFLOW_WRAPS case, the result can be anything (VARYING) both for the
GT_EXPR and GE_EXPR.

[Bug tree-optimization/101254] [12 Regression] gcc head does not comply fully to -fwrapv since r12-1723

[amacleod at redhat dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101254

Andrew Macleod <amacleod at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #51082|0                           |1
        is obsolete|                            |

--- Comment #5 from Andrew Macleod <amacleod at redhat dot com> ---
Created attachment 51083
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51083&action=edit
new patch

Ah right.  so op1 == op2 is the only time we can conclude anything.  Like so:

[Bug tree-optimization/101254] [12 Regression] gcc head does not comply fully to -fwrapv since r12-1723

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101254

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Well, from x > y signed -fwrapv you can assume x - y to be ~[0, 0] (from x >= y
nothing).
It is similar to unsigned, though in that case there are no negative values and
so for x >= y x - y [0, max] is the actually VARYING and for x > y x - y [1,
max]  is the same thing as ~[0, 0].

[Bug tree-optimization/101254] [12 Regression] gcc head does not comply fully to -fwrapv since r12-1723

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101254

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
BTW, rel of NE_EXPR, LE_EXPR or LT_EXPR never appear?  I guess one can always
swap the two operands and thus transform LE_EXPR into GE_EXPR and LT_EXPR into
GT_EXPR, but for EQ_EXPR vs. NE_EXPR that doesn't work.

[Bug tree-optimization/101254] [12 Regression] gcc head does not comply fully to -fwrapv since r12-1723

[amacleod at redhat dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101254

--- Comment #8 from Andrew Macleod <amacleod at redhat dot com> ---
I just never added them... I guess we could fully flesh out the combinations
and results.   Note this is also the only non-relational operand that is even
implemented so far..  Haven't gotten to any of the others yet.

[Bug tree-optimization/101254] [12 Regression] gcc head does not comply fully to -fwrapv since r12-1723

[amacleod at redhat dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101254

Andrew Macleod <amacleod at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #51083|0                           |1
        is obsolete|                            |

--- Comment #9 from Andrew Macleod <amacleod at redhat dot com> ---
Created attachment 51084
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51084&action=edit
another patch

OK, fleshed it out.  Your observations about [0,max] for unsigned are true, but
it was harmless.  however, when I broke it into 3 cases, unsigned, wrapping
signed and nornmal signed, it looks like I can simply treat it as wrapping and
non-wrapping.. since ~[0,0]for unsigned is the same as [1, max]
anyway I added the LT, LE, and NE cases as well.

I think this is now right and complete?  or have I missed something else..
certainly possible,I'm developing a headache thinking about it.

[Bug tree-optimization/101254] [12 Regression] gcc head does not comply fully to -fwrapv since r12-1723

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101254

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Comment on attachment 51084
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51084
another patch

Except for some consistency (max is in comments lowercase, but MIN is
uppercase), it looks good to me.
Slightly OT, wonder if something tries to handle even the swapped arguments,
i.e. relation op1 < op2 and subtraction op2 - op1.  But that can be done
incrementally if not done yet...

[Bug tree-optimization/101254] [12 Regression] gcc head does not comply fully to -fwrapv since r12-1723

[amacleod at redhat dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101254

--- Comment #11 from Andrew Macleod <amacleod at redhat dot com> ---
(In reply to Jakub Jelinek from comment #10)
> Comment on attachment 51084 [details]
> another patch
> 
> Except for some consistency (max is in comments lowercase, but MIN is
> uppercase), it looks good to me.

latest version i had already switched those to +INF and -INF terminology. 
Running thru testing now. 

> Slightly OT, wonder if something tries to handle even the swapped arguments,
> i.e. relation op1 < op2 and subtraction op2 - op1.  But that can be done
> incrementally if not done yet...

Not sure I follow.  Why would we be interested in swapping operands of a minus?

[Bug tree-optimization/101254] [12 Regression] gcc head does not comply fully to -fwrapv since r12-1723

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101254

--- Comment #12 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Andrew Macleod <amacleod@gcc.gnu.org>:

https://gcc.gnu.org/g:a96d8d67d0073a7031c0712bc3fb7759417b2125

commit r12-1916-ga96d8d67d0073a7031c0712bc3fb7759417b2125
Author: Andrew MacLeod <amacleod@redhat.com>
Date:   Tue Jun 29 10:52:58 2021 -0400

    Fix MINUS_EXPR relations.

    Flesh out and correct relations for both wrapping and non-wrapping values.

            gcc/
            PR tree-optimization/101254
            * range-op.cc (operator_minus::op1_op2_relation_effect): Check for
            wrapping/non-wrapping when setting the result range.

            gcc/testsuite
            * gcc.dg/pr101254.c: New.

[Bug tree-optimization/101254] [12 Regression] gcc head does not comply fully to -fwrapv since r12-1723

[amacleod at redhat dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101254

Andrew Macleod <amacleod at redhat dot com> changed:

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

--- Comment #13 from Andrew Macleod <amacleod at redhat dot com> ---
Fix should be checked in.