public inbox for gcc@gcc.gnu.org
 help / color / mirror / Atom feed
* Question about tree-switch-conversion.c
@ 2010-08-10 22:58 Ian Bolton
  0 siblings, 0 replies; 2+ messages in thread
From: Ian Bolton @ 2010-08-10 22:58 UTC (permalink / raw)
  To: gcc

[-- Attachment #1: Type: text/plain, Size: 1697 bytes --]

I am in the process of fixing PR44328
(http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44328)

The problem is that gen_inbound_check in tree-switch-conversion.c subtracts
info.range_min from info.index_expr, which can cause the MIN and MAX values
for info.index_expr to become invalid.

For example:


typedef enum {
  FIRST = 0,
  SECOND,
  THIRD,
  FOURTH
} ExampleEnum;

int dummy (const ExampleEnum e)
{
  int mode = 0;
  switch (e)
  {
    case SECOND: mode = 20; break;
    case THIRD: mode = 30; break;
    case FOURTH: mode = 40; break;
  }
  return mode;
}


tree-switch-conversion would like to create a lookup table for this, so
that SECOND maps to entry 0, THIRD maps to entry 1 and FOURTH maps to
entry 2.  It achieves this by subtracting SECOND from index_expr.  The
problem is that after the subtraction, the type of the result can have a
value outside the range 0-3.

Later, when tree-vrp.c sees the inbound check as being <= 2 with a possible
range for the type as 0-3, it converts the <=2 into a != 3, which is
totally wrong.  If e==FIRST, then we can end up looking for entry 255 in
the lookup table!

I think the solution is to update the type of the result of the subtraction
to show that it is no longer in the range 0-3, but I have had trouble
implementing this.  The attached patch (based off 4.5 branch) shows my
current approach, but I ran into LTO issues:

lto1: internal compiler error: in lto_get_pickled_tree, at lto-streamer-in.c

I am guessing this is because the debug info for the type does not match
the new range I have set for it.

Is there a *right* way to update the range such that LTO doesn't get
unhappy?  (Maybe a cast with fold_convert_loc would be right?)

[-- Attachment #2: pr44328.gcc4.5.fix.patch --]
[-- Type: application/octet-stream, Size: 1184 bytes --]

Index: gcc/tree-switch-conversion.c
===================================================================
--- gcc/tree-switch-conversion.c	(revision 162573)
+++ gcc/tree-switch-conversion.c	(working copy)
@@ -721,6 +721,21 @@
 
   bound = fold_convert_loc (loc, utype, info.range_size);
   cond_stmt = gimple_build_cond (LE_EXPR, tmp_u_2, bound, NULL_TREE, NULL_TREE);
+
+  /* Check that the subtraction does not cause underflow - if it does then we
+     must fix up the max value for the tmp_u expression used in the cond_stmt,
+     so that simplify_cond_using_ranges() in tree-vrp.c has the correct value
+     ranges to play with.  */
+  if ((TREE_INT_CST_LOW(TYPE_MIN_VALUE (TREE_TYPE(tmp_u_2)))
+       - TREE_INT_CST_LOW(ulb))
+     > TREE_INT_CST_LOW(TYPE_MIN_VALUE (TREE_TYPE(tmp_u_2))))
+    {
+      TYPE_MAX_VALUE (TREE_TYPE (gimple_cond_lhs(cond_stmt)))
+        = build_int_cst (TREE_TYPE(gimple_cond_lhs(cond_stmt)),
+                         TREE_INT_CST_LOW(TYPE_MIN_VALUE (TREE_TYPE(tmp_u_2)))
+                         - TREE_INT_CST_LOW(ulb)); 
+    }
+
   gsi_insert_before (&gsi, cond_stmt, GSI_SAME_STMT);
   update_stmt (cond_stmt);
 

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: Question about tree-switch-conversion.c
       [not found] <8940219999767457518@unknownmsgid>
@ 2010-08-11 13:49 ` Richard Guenther
  0 siblings, 0 replies; 2+ messages in thread
From: Richard Guenther @ 2010-08-11 13:49 UTC (permalink / raw)
  To: Ian Bolton; +Cc: gcc

On Tue, Aug 10, 2010 at 6:48 PM, Ian Bolton <Ian.Bolton@arm.com> wrote:
> I am in the process of fixing PR44328
> (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44328)
>
> The problem is that gen_inbound_check in tree-switch-conversion.c subtracts
> info.range_min from info.index_expr, which can cause the MIN and MAX values
> for info.index_expr to become invalid.
>
> For example:
>
>
> typedef enum {
>  FIRST = 0,
>  SECOND,
>  THIRD,
>  FOURTH
> } ExampleEnum;
>
> int dummy (const ExampleEnum e)
> {
>  int mode = 0;
>  switch (e)
>  {
>    case SECOND: mode = 20; break;
>    case THIRD: mode = 30; break;
>    case FOURTH: mode = 40; break;
>  }
>  return mode;
> }
>
>
> tree-switch-conversion would like to create a lookup table for this, so
> that SECOND maps to entry 0, THIRD maps to entry 1 and FOURTH maps to
> entry 2.  It achieves this by subtracting SECOND from index_expr.  The
> problem is that after the subtraction, the type of the result can have a
> value outside the range 0-3.
>
> Later, when tree-vrp.c sees the inbound check as being <= 2 with a possible
> range for the type as 0-3, it converts the <=2 into a != 3, which is
> totally wrong.  If e==FIRST, then we can end up looking for entry 255 in
> the lookup table!
>
> I think the solution is to update the type of the result of the subtraction
> to show that it is no longer in the range 0-3, but I have had trouble
> implementing this.  The attached patch (based off 4.5 branch) shows my
> current approach, but I ran into LTO issues:
>
> lto1: internal compiler error: in lto_get_pickled_tree, at lto-streamer-in.c
>
> I am guessing this is because the debug info for the type does not match
> the new range I have set for it.
>
> Is there a *right* way to update the range such that LTO doesn't get
> unhappy?  (Maybe a cast with fold_convert_loc would be right?)

The fix is to always use a standard unsigned integer type for the new index
value.  lang_hooks.types.type_for_mode (TYPE_MODE (old-idx-type), 1)
would give you one.

Richard.

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2010-08-11  8:50 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-08-10 22:58 Question about tree-switch-conversion.c Ian Bolton
     [not found] <8940219999767457518@unknownmsgid>
2010-08-11 13:49 ` Richard Guenther

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).