Patch to allow Ada to work with tree-ssa

[Richard Henderson]

On Mon, Jun 21, 2004 at 11:17:27PM -0400, Richard Kenner wrote:
> *************** gimplify_expr_stmt (tree *stmt_p)
> *** 244,248 ****
>   
>     if (stmt == NULL_TREE)
> -     stmt = build_empty_stmt ();
> +     stmt = alloc_stmt_list ();

Huh?

> ! 	  gimplify_stmt (&alloc_stmt);
> ! 	  append_to_statement_list(alloc_stmt, stmt_p);

gimplify_and_add.

> --- 148,151 ----
> *** c-typeck.c	21 Jun 2004 09:15:20 -0000	1.323
> --- c-typeck.c	21 Jun 2004 22:49:52 -0000
> *************** default_function_array_conversion (tree 
> *** 1227,1234 ****
>         if (TREE_CODE (exp) == VAR_DECL)
>   	{
> ! 	  /* ??? This is not really quite correct
> ! 	     in that the type of the operand of ADDR_EXPR
> ! 	     is not the target type of the type of the ADDR_EXPR itself.
> ! 	     Question is, can this lossage be avoided?  */
>   	  adr = build1 (ADDR_EXPR, ptrtype, exp);
>   	  if (!c_mark_addressable (exp))
> --- 1227,1234 ----
>         if (TREE_CODE (exp) == VAR_DECL)
>   	{
> ! 	  /* We are making an ADDR_EXPR of ptrtype.  This is a valid
> ! 	     ADDR_EXPR because it's the best way of representing what
> ! 	     happens in C when we take the address of an array and place
> ! 	     it in a pointer to the element type.  */

Like I said, probably &array[0] would be a better choice.

> *************** get_inner_reference (tree exp, HOST_WIDE
> *** 5677,5680 ****
> --- 5669,5736 ----
>   }
>   
> + /* Return a tree of sizetype representing the size, in bytes, of the element
> +    of EXP, an ARRAY_REF.  */
> + 
> + tree
> + array_ref_element_size (tree exp)

I wonder if "stride" might be a better term than "size"?

> +   /* If a size was specified in the ARRAY_REF, it's the size measured
> +      in alignment units of the element type.  So multiply by that value.  */
> +   if (aligned_size)
> +     return size_binop (MULT_EXPR, aligned_size,
> + 		       size_int (TYPE_ALIGN (elmt_type) / BITS_PER_UNIT));

Is that multiplication really healthy?  Given that we've done all of
the heavy lifting optimization that we're going to do, I'd think we'd
get a division and a multiplication, and not be able to merge them.

Anyway, what are you hoping to gain with not exposing the multiplication
to the tree optimizers?

> +   /* Otherwise, return a zero of the appropriate type.  */
> +   return fold_convert (TREE_TYPE (TREE_OPERAND (exp, 1)), integer_zero_node);

Are you assuming that we've canonicalized the index operand to the
domain type?  I was not aware that was happening at present...

> + /* Forward declarations.  */
> + static hashval_t gimple_tree_hash (const void *);

Please don't add unnecessary forward declarations.

> + /* Unshare all the trees in BODY_P, a pointer to the body of FNDECL, and the
> +    bodies of any nested functions.  */

Why would you need to do this?  What's getting shared between outer
and inner functions?  If it's related to TYPE_SIZE_UNIT and friends,
it seems easier to me to just always copy those expressions when we
pull them out.  Indeed, that's what I was already doing for the 
places that we did handle variable sized types before.

>   /* If a NOP conversion is changing a pointer to array of foo to a pointer
> !    to foo, embed that change in the ADDR_EXPR by converting 
> !       T array[U];
> !       (T *)&array
>      ==>
> !       &array[L]
> !    where L is the lower bound.  Only do this for constant lower bound since
> !    we have no place to put any statements made during gimplification of
> !    the lower bound.  */

Well, we certainly *can* give gimplify_conversion the pre-queue from
gimplify_expr, but it seems unlikely that we'll necessarily see that
non-constant lower-bound and generate (symbol_ref array).  I.e. 
avoid all arithmetic when we get to the rtl level.

> +   /* The lower bound and element sizes must be constant.  */
> +   if (TREE_CODE (TYPE_SIZE_UNIT (dctype)) != INTEGER_CST
> +       || !TYPE_DOMAIN (datype) || !TYPE_MIN_VALUE (TYPE_DOMAIN (datype))

Doesn't null domain imply zero lower bound?

> + build_addr_expr_with_type (tree t, tree ptrtype)
...
> + build_addr_expr (tree t)

What's wrong with the build_fold functions in fold-const.c?

> !   /* We can either handle one REALPART_EXPR or IMAGEPART_EXPR or
> !      nest of handled components.  */
> !   if (TREE_CODE (*expr_p) == REALPART_EXPR
> !       || TREE_CODE (*expr_p) == IMAGPART_EXPR)
> !     p = &TREE_OPERAND (*expr_p, 0);
> !   else
> !     for (p = expr_p; handled_component_p (*p); p = &TREE_OPERAND (*p, 0))
>         VARRAY_PUSH_TREE (stack, *p);

I see no reason to separate out the complex accessors here.  Why?

> *************** gimplify_addr_expr (tree *expr_p, tree *
> !     case VIEW_CONVERT_EXPR:
> !       /* Take the address of our operand and then convert it to the type of
> ! 	 this ADDR_EXPR.  */
> !       *expr_p = fold_convert (TREE_TYPE (expr),
> ! 			      build_addr_expr (TREE_OPERAND (op0, 0)));

What are you doing to prevent this from breaking aliasing?
This looks just like "(int *)&some_float" to me...

I guess you just hope that the source and destination type
alias sets are compatible?  Perhaps we should be validating?

> ! 	  /* This is probably a _REF that contains something nested that
> ! 	     has side effects.  Recurse through the operands to find it.  */
> ! 	  enum tree_code code = TREE_CODE (*expr_p);
> ! 
> ! 	  if (code == COMPONENT_REF
> ! 	      || code == REALPART_EXPR || code == IMAGPART_EXPR)
> ! 	    gimplify_expr (&TREE_OPERAND (*expr_p, 0), pre_p, post_p,
> ! 			   gimple_test_f, fallback);
> ! 	  else if (code == ARRAY_REF || code == ARRAY_RANGE_REF)
> ! 	    {
> ! 	      gimplify_expr (&TREE_OPERAND (*expr_p, 0), pre_p, post_p,
> ! 			     gimple_test_f, fallback);
> ! 	      gimplify_expr (&TREE_OPERAND (*expr_p, 1), pre_p, post_p,
> ! 			   gimple_test_f, fallback);

I don't understand what you're doing here.  In what way could you 
have not already gimplified these references?

> + tree
> + gimplify_type_sizes (tree type)

This would seem a bit better with a list_p argument.

> +   gimplify_expr (expr_p, &pre, &post, is_gimple_val, fb_rvalue);
> + 
> +   if (pre)
> +     append_to_statement_list (pre, stmt_p);
> +   if (post)
> +     append_to_statement_list (post, stmt_p);

It is incorrect to give gimplify_expr a post-queue here, because
*expr_p is going to be used after *stmt_p.  Either gimplify_expr
will find a workaround for the missing post-queue, or we'll abort.
As it is we'll generate incorrect code.

> *************** get_base_address (tree t)
> !       else if (handled_component_p (t))
> ! 	t = get_base_address (TREE_OPERAND (t, 0));

Loop + recursion?  Pick one.



r~