This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix PR42617, alias-export and restrict pointers
On Tue, 6 Apr 2010, Steven Bosscher wrote:
> On Tue, Apr 6, 2010 at 5:31 PM, Richard Guenther <rguenther@suse.de> wrote:
> >> > 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.
>
> Still this is probably the least ugly of the options you have.
Indeed.
> > 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"
>
> Heh.
> Maybe move gimple_in_ssa_form_p to another header file?
I guess it would fit function.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;
>
> Or move this check to another file? But that means figuring out which
> of the many set_mem_* functions get called, and from where.
I considered this, but then touching a single place looked simpler ;)
The above plus the operand_equal_p / tree_nop_conversion hardening
bootstrapped and tested ok.
> Actually, are you sure this patch doesn't break named address spaces?
> I see calls to set_mem_addr_space in tree-ssa-loop-ivopts.c itself.
> That will no longer work after your patch, IIUC.
Indeed, but I doubt any target produces meaningful costs for them.
In theory the operand_equal_p / tree_nop_conversion hardening alone
fixes the issue. But of course we still leave IVOPTs crap in the
hashtable. Maybe it's the simplest solution though (and also the
least risky for the branch in case the issue arises there).
Richard.