Re: [PATCH] Fix PR42617, alias-export and restrict pointers

[Richard Guenther <rguenther at suse dot de>]

On Tue, 6 Apr 2010, Richard Guenther wrote:

> On Tue, 6 Apr 2010, Richard Guenther wrote:
> 
> > On Tue, 6 Apr 2010, Richard Guenther wrote:
> > 
> > > 
> > > I am currently re-testing the following three patches against trunk
> > > and will commit them to fix alias-export with regarding to
> > > restrict qualified pointers (well, generally for pointer accesses).
> > 
> > And I now run into a latent problem with the first patch.  IVOPTs
> > expands memory accesses to RTL to determine computation cost.
> > This ends up creating MEM_ATTRs with MEM_EXPRs that contain SSA
> > names.  Those are _permanently_ stored in the mem_attrs_htab
> > (well, unless collected), including the references to eventually
> > released SSA names.  On MEM_EXPR lookup we call operand_equal_p
> > which eventually looks at the type of its operands which, for
> > released SSA names, is NULL_TREE.  Oops.
> > 
> > Now that IVOPTs expands things is ugly, and it technically doesn't need
> > MEM_EXPRs at all.  Thus we either can avoid generating them in the
> > first place (though I see no easy way to do so apart from adding a
> > global flag), or at least avoid keeping them in the hashtable.
> > 
> > This is what the following implements - we switch the mem_attrs
> > hashtable to a temporary one during IVOPTs (carefully throwing
> > away old entries once we release SSA names, which we do once
> > for each IV optimized loop).
> > 
> > I'm not sure this completely avoids the issues (as the hashtable is
> > still global, so we might get references to released SSA names
> > across functions), as RTL alias export will cause SSA names to
> > leak into the hashtable as well (but they are not released until
> > the function is completely processed).
> > 
> > The IVOPTs problem is latent on all branches, the RTL export
> > issue only on 4.5 and onwards.
> > 
> > Any better ideas?  Why do we share MEM_ATTRs across functions
> > in the first place?
> 
> Alternative patch, harden operand_equal_p and tree_nop_conversion
> against released SSA names.  Should completely fix the issue.

Or like the following.  In addition to the aforementioned to get
cross-function handling not ICEing.  Similar to the
currently_expanding_to_rtl hack for expanding SSA names, exactly
because of IVOPTs as well.

Richard.

Index: gcc/emit-rtl.c
===================================================================
*** gcc/emit-rtl.c      (revision 157991)
--- gcc/emit-rtl.c      (working copy)
*************** along with GCC; see the file COPYING3.
*** 60,65 ****
--- 60,66 ----
  #include "df.h"
  #include "params.h"
  #include "target.h"
+ #include "tree-flow.h"
  
  /* Commonly used modes.  */
  
*************** get_mem_attrs (alias_set_type alias, tre
*** 327,337 ****
    /* If everything is the default, we can just return zero.
       This must match what the corresponding MEM_* macros return when the
       field is not present.  */
!   if (alias == 0 && expr == 0 && offset == 0 && addrspace == 0
!       && (size == 0
!         || (mode != BLKmode && GET_MODE_SIZE (mode) == INTVAL (size)))
!       && (STRICT_ALIGNMENT && mode != BLKmode
!         ? align == GET_MODE_ALIGNMENT (mode) : align == BITS_PER_UNIT))
      return 0;
  
    attrs.alias = alias;
--- 332,343 ----
    /* If everything is the default, we can just return zero.
       This must match what the corresponding MEM_* macros return when the
       field is not present.  */
!   if ((alias == 0 && expr == 0 && offset == 0 && addrspace == 0
!        && (size == 0
!          || (mode != BLKmode && GET_MODE_SIZE (mode) == INTVAL (size)))
!        && (STRICT_ALIGNMENT && mode != BLKmode
!          ? align == GET_MODE_ALIGNMENT (mode) : align == BITS_PER_UNIT))
!       || (gimple_in_ssa_p (cfun) && !currently_expanding_to_rtl))
      return 0;
  
    attrs.alias = alias;