[Bug middle-end/16631] New: Problems inlining with enums

[Bug middle-end/16631] New: Problems inlining with enums

[roger at eyesopen dot com]

 

-- 
           Summary: Problems inlining with enums
           Product: gcc
           Version: 3.5.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: middle-end
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: roger at eyesopen dot com
                CC: gcc-bugs at gcc dot gnu dot org


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

[Bug middle-end/16631] Problems inlining with enums

[roger at eyesopen dot com]

------- Additional Comments From roger at eyesopen dot com  2004-07-19 17:32 -------
Created an attachment (id=6782)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=6782&action=view)
Failing test case

Doh!  I hit the wrong key.

There appears to be a problem with function inlining corrupting the range of
C's enumerated types.  This currently causes a bootstrap failure on Tru64
alphaev67-dec-osf5.1.  The attached testcase is reduced from GCC's mips-tdump.c
that demonstrates the problem on other platforms, including i686-pc-linux-gnu.

When compiled -O3, we currently generate the error messages:
foo3.i: In function `print_symbol':
foo3.i:413: warning: case label value 32 exceeds maximum value for type
foo3.i:450: warning: case label value 64 exceeds maximum value for type

This problem does not occur compiling the same file at -O2.   The original
from which this testcase is derived declared print_symbol and sc_to_string
as static which triggered the problem at -O2.


-- 


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

[Bug middle-end/16631] Problems inlining with enums

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-07-19 17:37 -------
*** Bug 16547 has been marked as a duplicate of this bug. ***

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ro at techfak dot uni-
                   |                            |bielefeld dot de


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

[Bug middle-end/16631] Problems inlining with enums

[roger at eyesopen dot com]

------- Additional Comments From roger at eyesopen dot com  2004-07-19 17:44 -------
It's just not my day.  Whilst reducing the attached testcase, I inadvertantly
introduced two spurious warnings that are completely unrelated to this failure.
These "assignment makes pointer from integer without a cast" warnings can be
silenced by adding "extern void* xmalloc(unsigned long);" to the top of foo3.i.
Sorry for the inconvenince.


-- 


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

[Bug middle-end/16631] Problems inlining with enums

[bangerth at dealii dot org]

------- Additional Comments From bangerth at dealii dot org  2004-07-19 17:46 -------
Confirmed. I'll try to come up with something a little smaller. 
 
W. 

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|                            |1
   Last reconfirmed|0000-00-00 00:00:00         |2004-07-19 17:46:13
               date|                            |


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

[Bug middle-end/16631] Problems inlining with enums

[bangerth at dealii dot org]

------- Additional Comments From bangerth at dealii dot org  2004-07-19 18:03 -------
Take this: 
------------------------- 
void dummy (int); 
 
typedef struct { 
  unsigned int i : 2; 
} uint_2; 
 
typedef enum E { 
  e_min = 0, 
  e_max = 4 
} E; 
 
int bar (E e) { 
  switch(e) { 
      case e_max: return 0; 
    } 
  return 1; 
} 
 
void foo (uint_2 *p) { 
  E e = (E) p->i; 
  dummy (bar (e)); 
} 
---------------------------- 
 
g/x> /home/bangerth/bin/gcc-3.5-pre/bin/gcc -O3 -c x.c 
x.c: In function `foo': 
x.c:13: warning: case label value 4 exceeds maximum value for type 
 
Clearly, the value 4 is within the range of possible values for type 'E'. 
I get the impression that gcc tries to propagate possible value ranges 
from the original variable 'p->i' because of the struct is defined as 
  typedef struct { 
    unsigned int i : 3; 
  } uint_2; 
(and should then be named uint_3), then the bug goes away. Thinking about 
it, the warning even makes some sense: in a two-bit integer, even if 
unsigned, the only values accessible are 0...3, so given the way we 
call bar(), the case will never be triggered. How much sense it makes 
to print a warning based on the fact that we can determine value ranges 
for a particular call is a separate matter, though. (It parallels the 
question whether we should print warnings for instantiations of C++ 
templates that are triggered for a particular value of the template 
argument.) 
 
W. 

-- 


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

[Bug middle-end/16631] Problems inlining with enums

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-07-19 18:50 -------
Since the warning is correct closing as invalid and then reopening the bootstrap bug.

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


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

[Bug middle-end/16631] Problems inlining with enums

[bangerth at dealii dot org]

------- Additional Comments From bangerth at dealii dot org  2004-07-19 19:04 -------
No, the warning is _not_ correct. You don't want warnings that only 
happen during inlining if the compiler can prove, based on knowing 
argument values or value ranges, that certain parts of an inlined 
function are dead. That's a completely common case, we don't want 
to see warnings then. 
 
This is the same as if you had code like 
  void foo (int i) { 
    if (i>256) 
      do_something(); 
  } 
 
  void bar (char c) { 
    foo((int)c); 
  } 
Since the argument to foo is certainly smaller than 256, the if-branch 
is never taken and the call to do_something is dead. I can't believe we 
want a warning for that. 
 
W. 

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


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

[Bug middle-end/16631] Problems inlining with enums

[roger at eyesopen dot com]

------- Additional Comments From roger at eyesopen dot com  2004-07-19 20:09 -------
Indeed.  This warning should only be emitted w.r.t. the original type as written
by the programmer, preferably by the language front-end.  The problem here is
that tree-ssa or some other middle-end optimization is eliminating a NOP_EXPR,
possibly that it shouldn't, but certainly it shouldn't complain that it has.

If the middle-end can prove that the value is a limited subset of the case's
range, that's good and can be used prune the jump table, but it shouldn't
automatically be diagnosed as a warning:

switch (x & 7)
{
case 8: foo();
case 3: bar();
}

Getting rid of case labels because they are unreachable is clever, but not
necessarily an error.  Especially, if this restricted range is purely an
artifact of tree inlining.  If inlining the code above results in "x" always
having the value 2 in the inlined instance, then obviously "case 3" shouldn't
then become a warning.

*IF* we're sure that this isn't an incorrect misuse of "useless type conversion"
(which isn't yet clear), then perhaps this should become a "c" component PR,
requiring the warning to be moved from the middle-end into each of the langauge
front-ends.


-- 


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

[Bug middle-end/16631] Problems inlining with enums

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-07-19 20:43 -------
And a patch was posted here: <http://gcc.gnu.org/ml/gcc-patches/2004-07/msg01859.html>.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |diagnostic, patch


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

[Bug middle-end/16631] Problems inlining with enums

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-07-19 21:15 -------
*** Bug 16547 has been marked as a duplicate of this bug. ***

-- 


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

[Bug middle-end/16631] [3.5 Regression] Problems inlining with enums

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-07-19 21:16 -------
Confirmed.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |NEW
            Summary|Problems inlining with enums|[3.5 Regression] Problems
                   |                            |inlining with enums
   Target Milestone|---                         |3.5.0


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

[Bug middle-end/16631] [3.5 Regression] Problems inlining with enums

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-07-20 14:32 -------
Fixed by:
        * c-common.h (check_case_value): Remove prototype.
        (c_add_case_label): Adjust prototype.
        * c-common.c (check_case_value): Make static.
        (check_case_bounds): New function.
        (c_add_case_label): Use it.  Take new argument orig_type.
        * c-typeck.c (struct c_switch): New orig_type field.
        (c_start_case): Set it.
        (do_case): Pass it to c_add_case_label.
        * expr.c (expand_expr_real_1): Don't warn for out-of-bounds
        cases from here.  Add the labels in reverse order.
        * stmt.c (struct case_node): Adjust comment.  Remove balance field.
        (add_case_node): Return nothing, don't check for duplicate cases.
        Insert new case nodes in a list, not in an AVL tree.
        (expand_end_case_type): Don't turn a case tree into a case list.
        (case_tree2list): Remove.
        * tree.h (add_case_node): Adjust prototype.
        
        cp/
        * cp-tree.h (struct lang_decl_flags): Unify the template_info and
        thunk_alias, and the access and virtual_offset fields.
        (THUNK_VIRTUAL_OFFSET, THUNK_ALIAS): Adjust.
        * decl.c (finish_case_label): Update c_add_case_node call.
        
        testsuite/
        * testsuite/gcc.dg/switch-warn-1.c: New test.
        * testsuite/gcc.dg/switch-warn-2.c: New test.
        * gcc.c-torture/compile/pr14730.c: Update

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


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