[patch] Fix ICE during RTL expansion at -O1

[Eric Botcazou]

> Note that looking at the access path _is_ assuming TBAA constraints as
> soon as the base objects are not the same (in the above case '*p' and 'a'
> are not the same and p could alias a in a way that all f1 and f2 overlap).

Right, but here I'm assuming (and asserting) that the base objects are the 
same.

> Index: alias.c
> ===================================================================
> --- alias.c     (revision 197926)
> +++ alias.c     (working copy)
> @@ -2232,8 +2232,11 @@ nonoverlapping_component_refs_p (const_r
> 
>      found:
>        /* If we're left with accessing different fields of a structure, then
> no -        possible overlap, unless they are both bitfields.  */
> -      if (TREE_CODE (typex) == RECORD_TYPE && fieldx != fieldy)
> +        possible overlap, unless they are both bitfields.
> +        ??? Pointer inequality is too fragile in the LTO compiler.  */
> +      if (TREE_CODE (typex) == RECORD_TYPE
> +         && fieldx != fieldy
> +         && DECL_NAME (fieldx) != DECL_NAME (fieldy))
> 
> this, if at all, should go in with a separate patch and a testcase.
> And I think it should _not_ go in.

OK, I can remove the LTO related bits.

> Otoh...
> 
> +      /* ??? We cannot simply use the type of operand #0 of the refs here
> +        as the Fortran compiler smuggles type punning into COMPONENT_REFs
> +        for common blocks instead of using unions like everyone else.  */
> +      tree type1 = TYPE_MAIN_VARIANT (DECL_CONTEXT (field1));
> +      tree type2 = TYPE_MAIN_VARIANT (DECL_CONTEXT (field2));
> +
> +      if (type1 != type2 || TREE_CODE (type1) != RECORD_TYPE)
> +        goto may_overlap;
> +
> +      /* ??? Pointer inequality is too fragile in the LTO compiler.  */
> +      if (field1 != field2 && DECL_NAME (field1) != DECL_NAME (field2))
> 
> this suggests you are seeing multiple FIELD_DECLs for the same field
> in the _same_ FIELD_DECL chain ...?!  Are you sure this happens with
> GCC 4.8?  There were some fixes in that area in the LTO type merging code.

No, let's drop the LTO related bits for mainline.  But the Fortran related 
bits are necessary, it's the verification you talked about earlier: for a 
COMPONENT_REF

  TYPE_MAIN_VARIANT (TREE_TYPE (TREE_OPERAND (t, 0)))
    = TYPE_MAIN_VARIANT (DECL_CONTEXT (TREE_OPERAND (t, 1)))

is expected to be always true (but isn't checked as you pointed out).  But the 
Fortran compiler violates it (4.9, gfortran.dg/aliasing_array_result_1.f90) as 
it implements a common block by defining 2 RECORD_TYPEs with 1 field, one for 
each variables, and does an implicit type conversion of TREE_OPERAND (t, 0).

> Index: tree-streamer.c
> ===================================================================
> --- tree-streamer.c     (revision 197926)
> +++ tree-streamer.c     (working copy)
> @@ -267,10 +267,9 @@ record_common_node (struct streamer_tree
>        /* The FIELD_DECLs of structures should be shared, so that every
>          COMPONENT_REF uses the same tree node when referencing a field.
>          Pointer equality between FIELD_DECLs is used by the alias
> -        machinery to compute overlapping memory references (See
> -        nonoverlapping_component_refs_p).  */
> -      tree f;
> -      for (f = TYPE_FIELDS (node); f; f = TREE_CHAIN (f))
> +        machinery to compute overlapping component references (see
> +        nonoverlapping_component_refs_of_decl_p).  */
> +      for (tree f = TYPE_FIELDS (node); f; f = TREE_CHAIN (f))
>         record_common_node (cache, f);
>      }
>  }
> 
> without actually removing nonoverlapping_component_refs_p it still applies
> to both...

Will adjust.

> Can you port the non-quadratic algorithm to the RTL
> nonoverlapping_component_refs_p first?  The core code should exactly
> be the same ... (and shared, and only called once from the RTL oracle).

No, it cannot be ported, the non-quadratic aspect is a consequence of the same 
base object assumption.  It you don't have it, you need to be quadratic to be 
able to deal with variable offsets in this way.

> I don't understand the "reference several fields" comment.  Because I
> can clearly have aggregate copies of RECORD_TYPE.  Can you try
> do elaborate more on why the algorithm should be sufficient to catch
> all cases?

I can, but you need to keep in mind that the base objects are the same.

> You could enhance it to not require
> 
> +  /* We must have the same base DECL.  */
> +  gcc_assert (ref1 == ref2);
> 
> for MEM_REF bases under the same conditions like aliasing_component_refs_p,
> that is if the MEM_REF isn't view-converting.

The code already handles MEM_REF<ADDR_EXPR> though (and checks that the offset 
is zero).  I'm not sure that we need more (but ready to be proved wrong here).

> That said, if the bases are the same DECLs then indeed you do not
> rely on TBAA.  The RTL nonoverlapping_component_refs_p does not
> disable itself properly for pointer based accesses that might be
> view-converted / aliased accesses (a simple testcase with ref-all
> pointers properly offsetted to alias two adjacent fields may be enough to
> show that).
> 
> Also with your patch enhanced like I suggest we should be able to
> remove aliasing_component_refs_p as well, no?

My revised patch is only a safe, non-quadratic, non-TBAA based disambiguation 
for references based on the same DECL, so it's a full replacement neither for 
aliasing_component_refs_p nor for nonoverlapping_component_refs_p.

Thanks for the review.  Updated patch attached.


        * tree-ssa-alias.c (nonoverlapping_component_refs_of_decl_p): New.
        (decl_refs_may_alias_p): Add REF1 and REF2 parameters.
        Use nonoverlapping_component_refs_of_decl_p to disambiguate component
        references.
        (refs_may_alias_p_1): Adjust call to decl_refs_may_alias_p.
        * tree-streamer.c (record_common_node): Adjust reference in comment.


-- 
Eric Botcazou
-------------- next part --------------
A non-text attachment was scrubbed...
Name: p3.diff
Type: text/x-patch
Size: 5949 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20130416/4e3ead37/attachment.bin>