On Wed, Nov 6, 2013 at 3:08 PM, Marc Glisse <marc.gli...@inria.fr> wrote: > On Wed, 6 Nov 2013, Richard Biener wrote: > >> On Wed, Nov 6, 2013 at 1:19 PM, Marc Glisse <marc.gli...@inria.fr> wrote: >>> >>> [Discussion started in >>> http://gcc.gnu.org/ml/gcc-patches/2013-10/msg02472.html ] >>> >>> >>> On Wed, 30 Oct 2013, Marc Glisse wrote: >>> >>>> On Wed, 30 Oct 2013, Richard Biener wrote: >>>> >>>>> Btw, get_addr_base_and_unit_offset may also return an offsetted >>>>> MEM_REF (from &MEM [p_3, 17] for example). As we are interested in >>>>> pointers this could be handled by not requiring a memory reference >>>>> but extracting the base address and offset, covering more cases. >>>> >>>> >>>> >>>> I tried the attached patch, and it almost worked, except for one fortran >>>> testcase (widechar_intrinsics_10.f90): >>> >>> >>> >>> Now that ao_ref_init_from_ptr_and_size has been fixed, the following >>> patch >>> passes bootstrap+testsuite (default languages) on >>> x86_64-unknown-linux-gnu. >>> >>> 2013-11-06 Marc Glisse <marc.gli...@inria.fr> >>> Jeff Law <l...@redhat.com> >>> >>> gcc/ >>> * tree-ssa-alias.c (stmt_kills_ref_p_1): Use >>> ao_ref_init_from_ptr_and_size for builtins. >>> >>> gcc/testsuite/ >>> * gcc.dg/tree-ssa/alias-27.c: New testcase. >>> >>> -- >>> Marc Glisse >>> Index: gcc/testsuite/gcc.dg/tree-ssa/alias-27.c >>> =================================================================== >>> --- gcc/testsuite/gcc.dg/tree-ssa/alias-27.c (revision 0) >>> +++ gcc/testsuite/gcc.dg/tree-ssa/alias-27.c (working copy) >>> @@ -0,0 +1,11 @@ >>> +/* { dg-do compile } */ >>> +/* { dg-options "-O1 -fdump-tree-optimized" } */ >>> + >>> +void f (long *p) { >>> + *p = 42; >>> + p[4] = 42; >>> + __builtin_memset (p, 0, 100); >>> +} >>> + >>> +/* { dg-final { scan-tree-dump-not "= 42" "optimized" } } */ >>> +/* { dg-final { cleanup-tree-dump "optimized" } } */ >>> >>> Property changes on: gcc/testsuite/gcc.dg/tree-ssa/alias-27.c >>> ___________________________________________________________________ >>> Added: svn:keywords >>> ## -0,0 +1 ## >>> +Author Date Id Revision URL >>> \ No newline at end of property >>> Added: svn:eol-style >>> ## -0,0 +1 ## >>> +native >>> \ No newline at end of property >>> Index: gcc/tree-ssa-alias.c >>> =================================================================== >>> --- gcc/tree-ssa-alias.c (revision 204453) >>> +++ gcc/tree-ssa-alias.c (working copy) >>> @@ -2001,23 +2001,24 @@ stmt_may_clobber_ref_p (gimple stmt, tre >>> return stmt_may_clobber_ref_p_1 (stmt, &r); >>> } >>> >>> /* If STMT kills the memory reference REF return true, otherwise >>> return false. */ >>> >>> static bool >>> stmt_kills_ref_p_1 (gimple stmt, ao_ref *ref) >>> { >>> /* For a must-alias check we need to be able to constrain >>> - the access properly. */ >>> - ao_ref_base (ref); >>> - if (ref->max_size == -1) >>> + the access properly. >>> + FIXME: except for BUILTIN_FREE. */ >>> + if (!ao_ref_base (ref) >>> + || ref->max_size == -1) >>> return false; >>> >>> if (gimple_has_lhs (stmt) >>> && TREE_CODE (gimple_get_lhs (stmt)) != SSA_NAME >>> /* The assignment is not necessarily carried out if it can throw >>> and we can catch it in the current function where we could >>> inspect >>> the previous value. >>> ??? We only need to care about the RHS throwing. For aggregate >>> assignments or similar calls and non-call exceptions the LHS >>> might throw as well. */ >>> @@ -2090,37 +2091,47 @@ stmt_kills_ref_p_1 (gimple stmt, ao_ref >>> case BUILT_IN_MEMPCPY: >>> case BUILT_IN_MEMMOVE: >>> case BUILT_IN_MEMSET: >>> case BUILT_IN_MEMCPY_CHK: >>> case BUILT_IN_MEMPCPY_CHK: >>> case BUILT_IN_MEMMOVE_CHK: >>> case BUILT_IN_MEMSET_CHK: >>> { >>> tree dest = gimple_call_arg (stmt, 0); >>> tree len = gimple_call_arg (stmt, 2); >>> - tree base = NULL_TREE; >>> - HOST_WIDE_INT offset = 0; >>> + tree rbase = ref->base; >>> + HOST_WIDE_INT roffset = ref->offset; >>> if (!host_integerp (len, 0)) >>> return false; >>> - if (TREE_CODE (dest) == ADDR_EXPR) >>> - base = get_addr_base_and_unit_offset (TREE_OPERAND (dest, >>> 0), >>> - &offset); >>> - else if (TREE_CODE (dest) == SSA_NAME) >>> - base = dest; >>> - if (base >>> - && base == ao_ref_base (ref)) >>> + ao_ref dref; >>> + ao_ref_init_from_ptr_and_size (&dref, dest, len); >> >> >> What I dislike about this is that it can end up building a new tree node. >> Not sure if that should block the patch though as it clearly allows to >> simplify the code ... >> >>> + tree base = ao_ref_base (&dref); >>> + HOST_WIDE_INT offset = dref.offset; >>> + if (!base || dref.size == -1) >>> + return false; >>> + if (TREE_CODE (base) == MEM_REF) >>> + { >>> + if (TREE_CODE (rbase) != MEM_REF) >> >> >> why's that? I think that just processing both bases separately >> would work as well. > > > If they differ, there is no point going on, we might as well break early. > And this way we maintain the property that either base and rbase are both > refs, or they are both pointers, not some weird mix.
One can be plain 'b' while one MEM[&b, 5], so yes, they differ, but only in offset. >>> + return false; >>> + // Compare pointers. >>> + offset += BITS_PER_UNIT >>> + * TREE_INT_CST_LOW (TREE_OPERAND (base, 1)); >> >> >> Use mem_ref_offset (base). > > > offset += BITS_PER_UNIT * mem_ref_offset(base).to_shwi(); Yes. > or did you mean the computations should use double_int? That would certainly be more correct ... (with a test at the end whether the result fits_shwi ()). >>> + roffset += BITS_PER_UNIT >>> + * TREE_INT_CST_LOW (TREE_OPERAND (rbase, >>> 1)); >> >> >> Likewise. >> >>> + base = TREE_OPERAND (base, 0); >>> + rbase = TREE_OPERAND (rbase, 0); >> >> >> Both could be &decl here, so you want >> >> if (TREE_CODE (base) == ADDR_EXPR) >> base = TREE_OPERAND (base, 0); > > > I rely on the ao_ref* functions to set base to decl and not MEM_REF[&decl], > is that a wrong assumption? Ah, I missed that, yes, that's a correct assumption which also makes my first comment moot. It's been some time since I wrote all this code ... ;) So the only thing that remains is the mem_ref_offset thing and yes, I guess I'd prefer to use double-ints because we deal with bit offsets in the end. Thanks, Richard. > -- > Marc Glisse
