Re: PR26197 (new_type_alias) fix

[Ian Lance Taylor <iant at google dot com>]

Dorit Nuzman <DORIT@il.ibm.com> writes:

> Index: tree-ssa-alias.c
> ===================================================================
> *** tree-ssa-alias.c	(revision 115685)
> --- tree-ssa-alias.c	(working copy)
> *************** static void create_global_var (void);
> *** 114,119 ****
> --- 114,120 ----
>   static void maybe_create_global_var (struct alias_info *ai);
>   static void group_aliases (struct alias_info *);
>   static void set_pt_anything (tree ptr);
> + static tree add_may_alias_for_new_tag (tree tag, tree var);
>   
>   /* Global declarations.  */

The current gcc convention is to define static functions before their
use, where possible, and to not bother with an explicit declaration.
That is, you should omit this declaration, and move the definition of
add_may_alias_for_new_tag before new_type_alias.


> +   ref = get_ref_base_and_extent (expr, &offset, &size, &maxsize);
> +   if (ref == NULL_TREE)
> +     ref = var;

Here you allow for the possibility that EXPR does not have a base and
extent which can be determined.

>         for (sv = svars; sv; sv = sv->next)
>   	{
> !           bool exact;
>   
> !           if (overlap_subvar (offset, maxsize, sv->var, &exact))
> !             VEC_safe_push (tree, heap, overlaps, sv->var);
> !         }

Here you seem to assume that OFFSET and MAXSIZE will always be set
correctly.  It seems to me that in the case where
get_ref_base_and_extent returns NULL_TREE, then OFFSET and MAXSIZE
will be such that overlap_subvar will always return false.  That seems
like the opposite of what you want to occur in that case.  It seems to
me you should fix that in the obvious way.  Please let me know if you
disagree.

> ! /* The following is based on code in add_stmt_operand to ensure that the
> !    same defs/uses/vdefs/vuses will be found after replacing a reference
> !    to var (or ARRAY_REF to var) with an INDIRECT_REF to ptr whose value
> !    is the address of var.  */

This comment needs to explain the return value of
add_may_alias_for_new_tag.  Also, I believe the reference should be to
add_virtual_operand, not to add_stmt_operand.


Patch is approved with those changes.  Since I'm suggesting a
functional change, please run another bootstrap, and please confirm
that the PR is still fixed.  It wasn't clear to me whether you were
ever able to recreate the problem; since this is an ICE, you should be
able to recreate it with a cross-compiler, and you should be able to
confirm that your patch fixes the problem.  Let us know if you need
any help with that.

Also, please add the test case to the testsuite.

Thanks.

:REVIEWMAIL:

Ian