Patch to allow Ada to work with tree-ssa
Richard Henderson
rth@redhat.com
Tue Jun 22 11:00:00 GMT 2004
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~
More information about the Gcc-patches
mailing list