Re: [RFC] [alias, middle-end] Alias and data dependency export

[Richard Guenther <rguenther at suse dot de>]

On Wed, 3 Jun 2009, Andrey Belevantsev wrote:

> Richard Guenther wrote:
> > I think we should instead benchmark getting rid of TMR_ORIGINAL
> > completely.  We should be able to initialize mem_attrs from the
> > other fields of the TMR.  But that can be done as a followup if
> > the special casing is not too ugly for now (was there supposed to
> > be a patch attached?).
> The special casing is in the lines of
> 
> *************** ptr_deref_may_alias_decl_p (tree ptr, tr
> *** 175,189 ****
>         || TREE_CODE (ptr) == INTEGER_CST)
>       return true;
> 
> !   gcc_assert (TREE_CODE (ptr) == SSA_NAME
> ! 	      && (TREE_CODE (decl) == VAR_DECL
> ! 		  || TREE_CODE (decl) == PARM_DECL
> ! 		  || TREE_CODE (decl) == RESULT_DECL));
> 
>     /* Non-aliased variables can not be pointed to.  */
>     if (!may_be_aliased (decl))
>       return false;
> 
>     /* If we do not have useful points-to information for this pointer
>        we cannot disambiguate anything else.  */
>     pi = SSA_NAME_PTR_INFO (ptr);
> --- 175,194 ----
>         || TREE_CODE (ptr) == INTEGER_CST)
>       return true;
> 
> !   gcc_assert (TREE_CODE (decl) == VAR_DECL
> !               || TREE_CODE (decl) == PARM_DECL
> !               || TREE_CODE (decl) == RESULT_DECL);
> 
>     /* Non-aliased variables can not be pointed to.  */
>     if (!may_be_aliased (decl))
>       return false;
> 
> +   if (TREE_CODE (ptr) != SSA_NAME)
> +     {
> +       gcc_assert (current_ir_type () != IR_GIMPLE);
> +       return true;
> +     }
> +
>     /* If we do not have useful points-to information for this pointer
>        we cannot disambiguate anything else.  */
>     pi = SSA_NAME_PTR_INFO (ptr);
> 
> and similarly for ptr_derefs_may_alias_p.

This could be avoided by not generating MEM_ORIG_EXPR from TMR_ORIGINAL
at all if they have INDIRECT_REF_P as base (with the same effect, aliasing
everything).

> The current patch I have is attached.  This is the cut-down version without
> ddg export (I have just edited the diff though to ease review, so the full
> compilable patch with ddg export is also attached for reference.  Also, the
> patch/changelog doesn't have code I would submit separately).
> 
> The partitioning is handled there as the last thing in partiotion_stack_vars,
> see update_alias_info_with_stack_vars.  The bases are rewritten during expand,
> seems like the problems I had were because of wrong ordering of the call to my
> code.  Other stuff you mentioned is fixed as well.
> 
> Some unsolved problems that may be done as a followup:
> 
> 1.  The patch of not adding stack vars for SSA_NAMEs breaks a stack protector
> test, ssp-2.c.  What happens that in the following function:
> 
> void
> overflow()
> {
>   int i = 0;
>   char foo[30];
> 
>   /* Overflow buffer.  */
>   for (i = 0; i < 50; i++)
>       foo[i] = 42;
> }
> 
> with the patch the loop counter (i) is placed on stack after the foo array,
> instead of before it, so it gets overwritten by the loop and it never
> terminates.  The stack protector marker stays in the same place though, I have
> no idea whether this is correct or not, and what should be changed in stack
> protector to make the patch work.  Meanwhile, I just check for SSA_NAMEs in
> update_alias_info_with_stack_vars.

I don't think the stack-protector is supposed to prevent the loop to
clobber i, but it is supposed to prevent unknown tampering with the
return location on the stack before returning from overflow().  That
the overflow now causes the testcase to not terminate is unfortunate,
but the testcase invokes undefined behavior.

I suggest you change the testcase to use a global variable i instead.

> 2. MEM_EXPR/MEM_ORIG_EXPR problem.  As we determined that the former is used
> for debugging, it seems that both should stay, but the usage of MEM_EXPR
> should be audited, and its application for debugging and for aliasing should
> be separated: debugging-related code goes to MEM_EXPR/MEM_OFFSET, while
> aliasing-related stuff (including nonoverlapping_memrefs_p, which probably can
> be simplified given the tree oracle) goes to MEM_ORIG_EXPR.  Then we can
> rename the fields to whatever we want to reflect this.

I will review the patch later.

Thanks,
Richard.