Re: [patch] TARGET_MEM_REF (resent 2)

[Richard Henderson <rth at redhat dot com>]

On Mon, Apr 04, 2005 at 10:25:04PM +0200, Zdenek Dvorak wrote:
> *************** gimplify_addr_expr (tree *expr_p, tree *
> *** 3270,3275 ****
> --- 3270,3280 ----
>         ret = GS_OK;
>         break;
>   
> +     case TARGET_MEM_REF:
> +       *expr_p = tree_mem_ref_addr (TREE_TYPE (expr), op0);
> +       ret = GS_OK;
> +       break;

Presumably this is here to deal with force_gimple_operands?

> *************** tree_could_trap_p (tree expr)
> *** 1739,1744 ****
> --- 1739,1751 ----
>    restart:
>     switch (code)
>       {
> +     case TARGET_MEM_REF:
> +       /* For TARGET_MEM_REFs use the information based on the original
> + 	 reference.  */
> +       expr = tmr_ann (expr)->original;
> +       code = TREE_CODE (expr);

So tmr annotations are non-optional?  Why, then, are they annotations
instead of direct operands?

> +       sym = gen_rtx_SYMBOL_REF (Pmode, ggc_strdup ("test_symbol"));

Problems: There are at least 5 different kinds of symbols that
you can run up against:

  (1) binds_local_p, small data area.
  (2) binds_local_p, eg local statics
  (3) !binds_local_p, eg global variables
  (4) thread local, local_exec
  (5) thread local, !local_exec

Now, (1) won't appear often in an array context, but it certainly can.
All you have to do is set -GN high enough, or explicitly mark any
random object __attribute__((section (".sdata"))).

All of these affect whether or not a symbol is in fact a valid address.
The only one you've tested here is (3).  And that result may very well
be incorrect for (4) or (5).

Fortunately, an incorrect result here doesn't appear to yield incorrect
results out the back end, because it would seem that the expr.c expander
would wind up validizing the address anyway.

But it is something you ought to be thinking about if you're intending
to be accurately modeling target addresses.

> +   if (symbol)
> +     {
> +       act_elem = symbol;
> +       if (offset)
> + 	{
> + 	  act_elem = gen_rtx_fmt_e (CONST, Pmode,
> + 				    gen_rtx_fmt_ee (PLUS, Pmode,
> + 						    act_elem, offset));
> + 	  if (offset_p)
> + 	    *offset_p = &XEXP (XEXP (act_elem, 0), 1);
> + 	}
> + 
> +       if (*addr)
> + 	*addr = gen_rtx_fmt_ee (PLUS, Pmode, *addr, act_elem);
> +       else
> + 	*addr = act_elem;
> +     }

What is the purpose of having symbol+offset be different fields,
when they are both simply addends?  And apparently constant ones
at that?  We are talking about "sym+off(base, index, step)" type
of x86 addressing modes, are we not?

> + create_mem_ref (block_stmt_iterator *bsi, tree type, tree addr)
> + {
> +   tree mem_ref, tmp;
> +   tree addr_type = build_pointer_type (type);
> +   struct mem_address parts;
> + 
> +   addr_to_parts (addr, &parts);

You're being exceedingly heroic decomposing these arbitrarily complex
addresses into something intelligable.  Is there any way you could avoid
building up these complex expressions only to have to decompose them
again?  Further...

> +   /* The expression is too complicated.  Try making it simpler.  */

And then still more work simplifying the expression until its valid.  It
would be a Good Thing if you remembered what kinds of addresses are not
valid, and don't try to generate them any more.

Combining this with the previous step, you might halt construction of
the complex address much earlier and thus do less work.

The poster child here is going to be ia64, in which *all* of this work
is pointless.  The question becomes how soon do you figure out to stop.

> ! /* Extract the alias analysis info for the memory reference REF.  There are
> !    several ways how this information may be stored and what precisely is
> !    its semantics depending on the type of the reference, but there always is
> !    somewhere hidden one _DECL node that is used to determine the set of
> !    virtual operands for the reference.  The code below deciphers this jungle
> !    and extracts this single useful piece of information.  */
>   
> ! static tree
> ! get_ref_tag (tree ref)

I have no clue whether or not this is correct.  You'll have to get a
ruling from Diego on this.



r~