[Bug c++/46806] New: -O3 implies 0 <= 8 <= 7

[Bug c++/46806] New: -O3 implies 0 <= 8 <= 7

[cstankevitz at toyon dot com]

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

           Summary: -O3 implies 0 <= 8 <= 7
           Product: gcc
           Version: 4.5.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: cstankevitz@toyon.com
              Host: Linux seymour.toyon.corp 2.6.35.6-48.fc14.x86_64 #1
                    SMP Fri Oct 22 15:36:08 UTC 2010 x86_64 x86_64 x86_64
                    GNU/Linux
             Build: g++ (GCC) 4.5.1 20100924 (Red Hat 4.5.1-4)


Created attachment 22636
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=22636
sample program

Linux seymour.toyon.corp 2.6.35.6-48.fc14.x86_64 #1 SMP Fri Oct 22 15:36:08 UTC
2010 x86_64 x86_64 x86_64 GNU/Linux

g++ (GCC) 4.5.1 20100924 (Red Hat 4.5.1-4)

Compile the attached source with "g++ test.cpp -O3" and the executable will
report 0<=8<=7.

To make the strange result go away:

- comment out the do-nothing line gValue = EdgeType (or make a number of other
minor changes)

- compile with -O1

[Bug c++/46806] -O3 implies 0 <= 8 <= 7

[pinskia at gcc dot gnu.org]

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

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> 2010-12-05 02:26:07 UTC ---
the enum is unsigned and 8 is out of range of the enum.

[Bug c++/46806] -O3 implies 0 <= 8 <= 7

[cstankevitz at toyon dot com]

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

--- Comment #2 from Chris Stankevitz <cstankevitz at toyon dot com> 2010-12-05 02:47:51 UTC ---
> 8 is out of range of the enum.

I agree.  Unfortunately, with -O3, 8 is reported as in the range.

[Bug c++/46806] -O3 implies 0 <= 8 <= 7

[cstankevitz at toyon dot com]

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

Chris Stankevitz <cstankevitz at toyon dot com> changed:

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

--- Comment #3 from Chris Stankevitz <cstankevitz at toyon dot com> 2010-12-05 02:53:17 UTC ---
Created attachment 22637
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=22637
sample program

Updated sample to use a cast to explicitly compare signed ints.

[Bug c++/46806] -O3 implies 0 <= 8 <= 7

[redi at gcc dot gnu.org]

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

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> 2010-12-05 04:16:26 UTC ---
dup of PR 43680 - the default behaviour has changed in 4.6, -fstrict-enums
restores the old semantics

[Bug c++/46806] -O3 implies 0 <= 8 <= 7

[redi at gcc dot gnu.org]

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

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> 2010-12-05 04:18:55 UTC ---
(In reply to comment #2)
> > 8 is out of range of the enum.
> 
> I agree.  Unfortunately, with -O3, 8 is reported as in the range.

if a program has undefined behaviour then you shouldn't expect consistent
results at different optimisation levels ... but luckily for you gcc 4.6 is
more forgiving of out-of-range enums

[Bug c++/46806] -O3 implies 0 <= 8 <= 7

[rguenth at gcc dot gnu.org]

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

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> 2010-12-05 11:31:56 UTC ---
.

[Bug c++/46806] -O3 implies 0 <= 8 <= 7

[cstankevitz at toyon dot com]

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

--- Comment #7 from Chris Stankevitz <cstankevitz at toyon dot com> 2010-12-05 18:11:55 UTC ---

If I understand correctly, you are saying "The following is not a valid way to
test whether a user-supplied int is withing the bounds of an enum:"

  if (
    Eight >= static_cast<int>(eZero) &&
    Eight <= static_cast<int>(eSeven))
  {
    Value = static_cast<TEValue>(Eight);

    std::cerr << "0 <= 8 <= 7 (this is bad)\n";
  }

What is the correct, valid, portable way to test whether a user-supplied int is
in the range of an enum?

Thank you,

Chris

[Bug c++/46806] -O3 implies 0 <= 8 <= 7

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

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

--- Comment #8 from Jonathan Wakely <redi at gcc dot gnu.org> 2010-12-05 19:27:39 UTC ---
Hmm, I might have been too hasty declaring this invalid.

The comparisons are ok, with or without casts, I think the problem is this
line:
    Value = static_cast<TEValue>(Eight);

The optimiser thinks that because you do that cast, that implies that Eight
must be in the range [0,7] always, not just when you've checked that it is
actually in range.

Changing that line to this prevents the condition always being true:
    Value = static_cast<TEValue>(Eight%8);

The condition before that line should ensure that's not needed, because you've
already ensured the value is in range, so maybe there is a bug here, which is
hidden in 4.6 by -fstrict-enums being off by default.

I'm re-opening this again, could a front-end or middle-end maintainer comment
on whether this really is invalid?

[Bug c++/46806] -O3 implies 0 <= 8 <= 7

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2010.12.05 22:13:20
         AssignedTo|unassigned at gcc dot       |rguenth at gcc dot gnu.org
                   |gnu.org                     |
     Ever Confirmed|0                           |1

--- Comment #9 from Richard Guenther <rguenth at gcc dot gnu.org> 2010-12-05 22:13:20 UTC ---
Heh, you are seeing a dup of https://bugzilla.novell.com/show_bug.cgi?id=623386
(sorry, not public).

Which is a copyrename issue and hard to trigger (I couldn't trigger it
with anything other than 4.3).  4.3 testcase:

typedef enum { zero = 0, one = 1, two = 2, ENUM_MAX = 3 } my_enum;
my_enum e;
extern "C" void abort (void);
int __attribute__((noinline)) foo() { return 10; }
int main()
{
  int r;
  r = foo();
  if ((r < 0) || (r >= ENUM_MAX))
    return 0;
  e = (my_enum)r;
  abort ();
}

And I have a patch (maybe I even posted it just didn't apply it),
the problem is we are coalescing a variable of enum type with one
of integer type.

Index: gcc/tree-ssa-copyrename.c
===================================================================
*** gcc/tree-ssa-copyrename.c   (revision 163549)
--- gcc/tree-ssa-copyrename.c   (working copy)
*************** copy_rename_partition_coalesce (var_map
*** 239,245 ****
      }

    /* Don't coalesce if the two variables aren't type compatible.  */
!   if (!types_compatible_p (TREE_TYPE (root1), TREE_TYPE (root2)))
      {
        if (debug)
        fprintf (debug, " : Incompatible types.  No coalesce.\n");
--- 239,246 ----
      }

    /* Don't coalesce if the two variables aren't type compatible.  */
!   if (TYPE_MAIN_VARIANT (TREE_TYPE (root1))
!       != TYPE_MAIN_VARIANT (TREE_TYPE (root2)))
      {
        if (debug)
        fprintf (debug, " : Incompatible types.  No coalesce.\n");


Seems I did post an earlier version for comments:
http://gcc.gnu.org/ml/gcc-patches/2010-08/msg01873.html

[Bug c++/46806] -O3 implies 0 <= 8 <= 7

[cstankevitz at toyon dot com]

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

Chris Stankevitz <cstankevitz at toyon dot com> changed:

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

--- Comment #10 from Chris Stankevitz <cstankevitz at toyon dot com> 2010-12-05 23:47:19 UTC ---
> The comparisons are ok, with or without casts, I think the problem
> is this line:
>     Value = static_cast<TEValue>(Eight);

I disagree because the problem exists even if that line is this more reasonable
statement:

  Value = static_cast<TEValue>(eZero);

[Bug c++/46806] -O3 implies 0 <= 8 <= 7

[paolo.carlini at oracle dot com]

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

Paolo Carlini <paolo.carlini at oracle dot com> changed:

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

[Bug c++/46806] -O3 implies 0 <= 8 <= 7

[rguenth at gcc dot gnu.org]

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

--- Comment #11 from Richard Guenther <rguenth at gcc dot gnu.org> 2010-12-06 12:53:41 UTC ---
Author: rguenth
Date: Mon Dec  6 12:53:38 2010
New Revision: 167490

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=167490
Log:
2010-12-06  Richard Guenther  <rguenther@suse.de>

    PR tree-optimization/46806
    * tree-ssa-copyrename.c (copy_rename_partition_coalesce): Do not
    coalesce different types.

    * g++.dg/torture/20100825.C: New testcase.

Added:
    trunk/gcc/testsuite/g++.dg/torture/20100825.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-copyrename.c

[Bug c++/46806] -O3 implies 0 <= 8 <= 7

[rguenth at gcc dot gnu.org]

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

--- Comment #12 from Richard Guenther <rguenth at gcc dot gnu.org> 2010-12-06 13:07:15 UTC ---
Author: rguenth
Date: Mon Dec  6 13:07:12 2010
New Revision: 167492

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=167492
Log:
2010-12-06  Richard Guenther  <rguenther@suse.de>

    PR tree-optimization/46806
    * tree-ssa-copyrename.c (copy_rename_partition_coalesce): Do not
    coalesce different types.

    * g++.dg/torture/20100825.C: New testcase.

Added:
    branches/gcc-4_5-branch/gcc/testsuite/g++.dg/torture/20100825.C
Modified:
    branches/gcc-4_5-branch/gcc/ChangeLog
    branches/gcc-4_5-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_5-branch/gcc/tree-ssa-copyrename.c

[Bug tree-optimization/46806] -O3 implies 0 <= 8 <= 7

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
          Component|c++                         |tree-optimization
      Known to work|                            |4.5.2, 4.6.0
         Resolution|                            |FIXED
   Target Milestone|---                         |4.5.2
      Known to fail|                            |4.5.1

--- Comment #13 from Richard Guenther <rguenth at gcc dot gnu.org> 2010-12-06 13:08:07 UTC ---
Fixed for 4.5.2.