Re: [ast-optimizer-branch]: Simplify STMT_EXPR's

[Daniel Berlin <dberlin at dberlin dot org>]

On Thu, 2 May 2002, Diego Novillo wrote:

> On Thu, 02 May 2002, Daniel Berlin wrote:
> 
> > 
> > Transforms stmt expressions like
> > 
> > a
> > ({<....>})
> > b
> > 
> > into
> > 
> > a
> > {<....>}
> > b
> > 
> > 
> > Trying to transform those that return values doesn't always work.
> > 
> > 
> > We generate what looks like correct code, i.e.:
> > 
> > 
> > a
> > b = ({<.....> <value>})
> > 
> > becomes
> > 
> > a
> > {<....> <new temporary> = <value>}
> > b = <new temporary>
> > 
> > 
> > but during rtl expansion, it aborts failing the following test in 
> > varasm.c:
> > 
> >   /* Check that we are not being given an automatic variable.  */
> >   /* A weak alias has TREE_PUBLIC set but not the other bits.  */
> >   if (TREE_CODE (decl) == PARM_DECL
> >       || TREE_CODE (decl) == RESULT_DECL
> >       || (TREE_CODE (decl) == VAR_DECL
> >           && !TREE_STATIC (decl)
> >           && !TREE_PUBLIC (decl)
> >           && !DECL_EXTERNAL (decl)
> >           && !DECL_REGISTER (decl)))
> >     abort ();
> > 
> > 
> > 
> > So, until I figure out what to do, it only transforms those that do not 
> > return values.
> > 
> > Index: c-simplify.c
> > ===================================================================
> > RCS file: /cvs/gcc/gcc/gcc/Attic/c-simplify.c,v
> > retrieving revision 1.1.2.14
> > diff -c -3 -p -w -B -b -r1.1.2.14 c-simplify.c
> > *** c-simplify.c	1 May 2002 00:30:40 -0000	1.1.2.14
> > --- c-simplify.c	2 May 2002 14:30:54 -0000
> > *************** simplify_decl_stmt (t, post_p)
> > *** 879,885 ****
> >   	            post_p);
> >   	  DECL_INITIAL (decl) = NULL_TREE;
> >   	}
> > ! 
> >         prev = t;
> >         t = TREE_CHAIN (t);
> >       }
> > --- 879,889 ----
> >   	            post_p);
> >   	  DECL_INITIAL (decl) = NULL_TREE;
> >   	}
> > !       else if (DECL_INITIAL (decl) 
> > ! 	      && TREE_CODE_CLASS (TREE_CODE (DECL_INITIAL (decl))) == 's')
> > !         {
> > ! 	 simplify_stmt (DECL_INITIAL (decl));
> > ! 	}
> >         prev = t;
> >         t = TREE_CHAIN (t);
> >       }
> Why are you leaving the DECL_INITIAL around?
Because it's to a readonly variable.

If it wasn't, the first part of the if would have taken care of it.

However, we should still simplify the right hand sides of readonly 
variables.
>  We only leave
> DECL_INITIALs in read-only constants.
I'm aware.
>   Othwerise, we move the
> initializer into the function body and simplify it later.  If you
> leave the DECL_INITIAL here, you will have temporaries inside the
> statement expression that will be referenced before their
> DECL_STMT, which leads to the abort in varasm.c.
I haven't left it for a non-readonly variable, only simplified it if it is 
the initializer for a readonly variable (they can still be statement 
expressions).

This also isn't the cause of the abort (I tried it without, no dice).

> 
> > *************** simplify_expr (expr, pre_p, post_p, simp
> > *** 1049,1055 ****
> > --- 1053,1123 ----
> >          SIMPLE grammar.  */
> >       case STMT_EXPR:
> >         simplify_stmt (STMT_EXPR_STMT (expr));
> > + 
> > +       /* These tests determine if the STMT_EXPR returns a value. If it does, we need
> > +          to store that value somewhere.  */
> > +       if (TREE_CODE (STMT_EXPR_STMT (expr)) == COMPOUND_STMT
> > +  	  && TREE_CODE (COMPOUND_BODY (STMT_EXPR_STMT (expr))) == SCOPE_STMT 
> > + 	  && TREE_CODE (TREE_TYPE (expr)) != VOID_TYPE)
> > + 	{
> >
> Please call a separate function.  The body of simplify_expr() is
> hairy enough already.
sure.

> 
> [ ... ]
> > + 	  if (TREE_CODE_CLASS (TREE_CODE (last_expr)) == 'e')
> > + 	    {
> > + 	      temp2 = build_modify_expr (tmp, NOP_EXPR, TREE_OPERAND (last_expr, 0));
> > + 	      new_stmt = build_stmt (EXPR_STMT ,temp2);
> > + 	      TREE_CHAIN (new_stmt) = TREE_CHAIN (temp);
> > + 	      TREE_CHAIN (temp) = new_stmt;
> > + 	    }
> > + 	  else
> > + 	    {
> > + 	      temp2 = build_modify_expr (tmp, NOP_EXPR, last_expr);
> > + 	      EXPR_STMT_EXPR (temp) = temp2;
> > + 	    }
> 
> More concisely:
> 
> 	temp2 = (is_simple_modify_expr (last_expr))
> 	        ? build_modify_expr (tmp, NOP_EXPR, TREE_OPERAND (last_expr, 0))
> 		: build_modify_expr (tmp, NOP_EXPR, last_expr);
> 
> Thanks.  Diego.
>