Re: [patch] stmt.c: Speed/clean up expand_case - Part 2

Re: [patch] stmt.c: Speed/clean up expand_case - Part 2

[Richard Kenner]

    > There could be a remote possibility of a backend generating CONST_INT
    > for a tree node that is not INTEGER_CST, so I left the following
    > intact.

    I would be shocked if this were the case.  Particularly with
    gimplification.

I can't think of a case offhand, but I wouldn't be "shocked".  I'd say this
was an example of "be conservative in what you generate, but liberal in
what you accept".

Re: [patch] stmt.c: Speed/clean up expand_case - Part 2

[Kazu Hirata]

Hi Richard,

> >   gcc_assert (GET_CODE (index) != CONST_INT);
> > 
> > as a friendly reminder to a backend. :-)
> 
> Um, no, this isn't really acceptable.  Just don't bother
> special casing CONST_INT here.

OK.

Kazu Hirata

Re: [patch] stmt.c: Speed/clean up expand_case - Part 2

[Richard Henderson]

On Mon, Oct 25, 2004 at 07:17:43PM -0400, Kazu Hirata wrote:
>   gcc_assert (GET_CODE (index) != CONST_INT);
> 
> as a friendly reminder to a backend. :-)

Um, no, this isn't really acceptable.  Just don't bother
special casing CONST_INT here.


r~

Re: [patch] stmt.c: Speed/clean up expand_case - Part 2

[Kazu Hirata]

Hi Richard,

> > Technically, could a backend define a __builtin_xxx and expands it to
> > a CONST_INT?
> 
> Yes, it could.  But I think I don't care.  If optimization is on,
> it'll get cleaned up later.  And the target does have the option
> of the fold_builtin target hook to make this happen earlier.

I'll submit a new patch that has both

  gcc_assert (TREE_CODE (index_expr) != INTEGER_CST);
  gcc_assert (GET_CODE (index) != CONST_INT);

as a friendly reminder to a backend. :-)

Kazu Hirata

Re: [patch] stmt.c: Speed/clean up expand_case - Part 2

[Richard Henderson]

On Mon, Oct 25, 2004 at 06:33:23PM -0400, Kazu Hirata wrote:
> Technically, could a backend define a __builtin_xxx and expands it to
> a CONST_INT?

Yes, it could.  But I think I don't care.  If optimization is on,
it'll get cleaned up later.  And the target does have the option
of the fold_builtin target hook to make this happen earlier.


r~

Re: [patch] stmt.c: Speed/clean up expand_case - Part 2

[Daniel Jacobowitz]

On Mon, Oct 25, 2004 at 06:33:23PM -0400, Kazu Hirata wrote:
> Hi Richard,
> 
> > > There could be a remote possibility of a backend generating CONST_INT
> > > for a tree node that is not INTEGER_CST, so I left the following
> > > intact.
> > 
> > I would be shocked if this were the case.  Particularly with gimplification.
> 
> Technically, could a backend define a __builtin_xxx and expands it to
> a CONST_INT?

Presumably it could also expand it to an INTEGER_CST earlier.  And
should...

-- 
Daniel Jacobowitz

Re: [patch] stmt.c: Speed/clean up expand_case - Part 2

[Kazu Hirata]

Hi Richard,

> > There could be a remote possibility of a backend generating CONST_INT
> > for a tree node that is not INTEGER_CST, so I left the following
> > intact.
> 
> I would be shocked if this were the case.  Particularly with gimplification.

Technically, could a backend define a __builtin_xxx and expands it to
a CONST_INT?

Kazu Hirata

Re: [patch] stmt.c: Speed/clean up expand_case - Part 2

[Richard Henderson]

On Mon, Oct 25, 2004 at 06:11:01PM -0400, Kazu Hirata wrote:
> There could be a remote possibility of a backend generating CONST_INT
> for a tree node that is not INTEGER_CST, so I left the following
> intact.

I would be shocked if this were the case.  Particularly with gimplification.


r~

[patch] stmt.c: Speed/clean up expand_case - Part 2

[Kazu Hirata]

Hi,

Attached is a patch to speed/clean up expand_case.

cleanup_tree_cfg cleans up a SWITCH_EXPR with its index expression
being INTEGER_CST and replaces the SWITCH_EXPR with an unconditional
jump.  This happens regardless of the optimization level.

The patch uses the above fact to simplify expand_case.

There could be a remote possibility of a backend generating CONST_INT
for a tree node that is not INTEGER_CST, so I left the following
intact.

  if (GET_CODE (index) == CONST_INT)

Tested on i686-pc-linux-gnu.  OK to apply?

Kazu Hirata

2004-10-25  Kazu Hirata  <kazu@cs.umass.edu>

	* stmt.c (expand_case): Remove code to handle constant
	index_expr.

Index: stmt.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/stmt.c,v
retrieving revision 1.402
diff -u -p -r1.402 stmt.c
--- stmt.c	24 Oct 2004 00:46:10 -0000	1.402
+++ stmt.c	25 Oct 2004 14:45:09 -0000
@@ -2355,6 +2355,10 @@ expand_case (tree exp)
     {
       tree elt;
 
+      /* cleanup_tree_cfg removes all SWITCH_EXPR with their index
+	 expressions being INTEGER_CST.  */
+      gcc_assert (TREE_CODE (index_expr) != INTEGER_CST);
+
       /* The default case is at the end of TREE_VEC.  */
       elt = TREE_VEC_ELT (vec, TREE_VEC_LENGTH (vec) - 1);
       gcc_assert (!CASE_HIGH (elt));
@@ -2498,19 +2502,15 @@ expand_case (tree exp)
 
 	  if (MEM_P (index))
 	    index = copy_to_reg (index);
-	  if (GET_CODE (index) == CONST_INT
-	      || TREE_CODE (index_expr) == INTEGER_CST)
+	  if (GET_CODE (index) == CONST_INT)
 	    {
 	      /* Make a tree node with the proper constant value
 		 if we don't already have one.  */
-	      if (TREE_CODE (index_expr) != INTEGER_CST)
-		{
-		  index_expr
-		    = build_int_cst_wide (NULL_TREE, INTVAL (index),
-					  unsignedp || INTVAL (index) >= 0
-					  ? 0 : -1);
-		  index_expr = convert (index_type, index_expr);
-		}
+	      index_expr
+		= build_int_cst_wide (NULL_TREE, INTVAL (index),
+				      unsignedp || INTVAL (index) >= 0
+				      ? 0 : -1);
+	      index_expr = convert (index_type, index_expr);
 
 	      /* For constant index expressions we need only
 		 issue an unconditional branch to the appropriate