| Message ID | alpine.DEB.2.10.1310301121490.4464@stedding.saclay.inria.fr |
|---|---|
| State | New |
| Headers | show |
On Wed, Oct 30, 2013 at 1:43 PM, Marc Glisse <marc.glisse@inria.fr> wrote: > On Wed, 30 Oct 2013, Richard Biener wrote: > >> On Wed, Oct 30, 2013 at 1:09 AM, Marc Glisse <marc.glisse@inria.fr> wrote: >>> >>> On Tue, 29 Oct 2013, Richard Biener wrote: >>> >>>> For the POINTER_PLUS_EXPR offset argument you should use >>>> int_cst_value () to access it (it will unconditionally sign-extend) >>>> and use host_integerp (..., 0). That leaves the overflow possibility >>>> in place (and you should multiply by BITS_PER_UNIT) which we >>>> ignore in enough other places similar to this to ignore ... >>> >>> >>> >>> Like this? (passes bootstrap+testsuite on x86_64-linux-gnu) >>> >>> 2013-10-30 Marc Glisse <marc.glisse@inria.fr> >>> >>> >>> gcc/ >>> * tree-ssa-alias.c (ao_ref_init_from_ptr_and_size): Look for a >>> POINTER_PLUS_EXPR in the defining statement. >>> >>> gcc/testsuite/ >>> * gcc.dg/tree-ssa/alias-24.c: New file. >>> >>> >>> -- >>> Marc Glisse >>> >>> Index: gcc/testsuite/gcc.dg/tree-ssa/alias-24.c >>> =================================================================== >>> --- gcc/testsuite/gcc.dg/tree-ssa/alias-24.c (revision 0) >>> +++ gcc/testsuite/gcc.dg/tree-ssa/alias-24.c (working copy) >>> @@ -0,0 +1,22 @@ >>> +/* { dg-do compile } */ >>> +/* { dg-options "-O2 -fdump-tree-optimized" } */ >>> + >>> +void f (const char *c, int *i) >>> +{ >>> + *i = 42; >>> + __builtin_memcpy (i + 1, c, sizeof (int)); >>> + if (*i != 42) __builtin_abort(); >>> +} >>> + >>> +extern void keepit (); >>> +void g (const char *c, int *i) >>> +{ >>> + *i = 33; >>> + __builtin_memcpy (i - 1, c, 3 * sizeof (int)); >>> + if (*i != 33) keepit(); >>> +} >>> + >>> +/* { dg-final { scan-tree-dump-not "abort" "optimized" } } */ >>> +/* { dg-final { scan-tree-dump "keepit" "optimized" } } */ >>> +/* { dg-final { cleanup-tree-dump "optimized" } } */ >>> + >>> >>> Property changes on: gcc/testsuite/gcc.dg/tree-ssa/alias-24.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 204188) >>> +++ gcc/tree-ssa-alias.c (working copy) >>> @@ -567,20 +567,29 @@ void >>> ao_ref_init_from_ptr_and_size (ao_ref *ref, tree ptr, tree size) >>> { >>> HOST_WIDE_INT t1, t2; >>> ref->ref = NULL_TREE; >>> if (TREE_CODE (ptr) == SSA_NAME) >>> { >>> gimple stmt = SSA_NAME_DEF_STMT (ptr); >>> if (gimple_assign_single_p (stmt) >>> && gimple_assign_rhs_code (stmt) == ADDR_EXPR) >>> ptr = gimple_assign_rhs1 (stmt); >>> + else if (is_gimple_assign (stmt) >>> + && gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR >>> + && host_integerp (gimple_assign_rhs2 (stmt), 0) >>> + && (t1 = int_cst_value (gimple_assign_rhs2 (stmt))) >= 0) >> >> >> No need to restrict this to positive offsets I think. > > > Actually there is. The function documents that it only handles nonnegative > offsets, and if we ignore that, the "keepit" part of the testcase fails > because ranges_overlap_p works with unsigned offsets. I can try to change > ranges_overlap_p and remove this restriction from > ao_ref_init_from_ptr_and_size, but that's more risky so I'd prefer to do > that as a separate follow-up patch if that's ok. > > >>> + { >>> + ao_ref_init_from_ptr_and_size (ref, gimple_assign_rhs1 (stmt), >>> size); >> >> >> Please don't recurse - forwprop combines series of POINTER_PLUS_EXPRs >> and &MEM[ptr, offset] so it shouldn't be necessary. > > > Done. (note that if it isn't necessary, then it doesn't hurt either ;-) > > >>> + ref->offset += 8 * t1; >> >> >> BITS_PER_UNIT instead of 8. I'd say just have a 0-initialized >> additional_offset var that you unconditionally add ... > > > I also changed a few other 8s. > > >>> + return; >>> + } >>> } >>> >>> if (TREE_CODE (ptr) == ADDR_EXPR) >>> ref->base = get_ref_base_and_extent (TREE_OPERAND (ptr, 0), >>> &ref->offset, &t1, &t2); >>> else >>> { >>> ref->base = build2 (MEM_REF, char_type_node, >>> ptr, null_pointer_node); >>> ref->offset = 0; >> >> >> ... here at the end. > > > Here is a new version, same testing, same ChangeLog. Ok. Thanks, Richard. > -- > Marc Glisse > Index: gcc/testsuite/gcc.dg/tree-ssa/alias-24.c > =================================================================== > --- gcc/testsuite/gcc.dg/tree-ssa/alias-24.c (revision 0) > +++ gcc/testsuite/gcc.dg/tree-ssa/alias-24.c (working copy) > @@ -0,0 +1,22 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fdump-tree-optimized" } */ > + > +void f (const char *c, int *i) > +{ > + *i = 42; > + __builtin_memcpy (i + 1, c, sizeof (int)); > + if (*i != 42) __builtin_abort(); > +} > + > +extern void keepit (); > +void g (const char *c, int *i) > +{ > + *i = 33; > + __builtin_memcpy (i - 1, c, 3 * sizeof (int)); > + if (*i != 33) keepit(); > +} > + > +/* { dg-final { scan-tree-dump-not "abort" "optimized" } } */ > +/* { dg-final { scan-tree-dump "keepit" "optimized" } } */ > +/* { dg-final { cleanup-tree-dump "optimized" } } */ > + > > Property changes on: gcc/testsuite/gcc.dg/tree-ssa/alias-24.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 204200) > +++ gcc/tree-ssa-alias.c (working copy) > @@ -559,43 +559,53 @@ ao_ref_alias_set (ao_ref *ref) > } > > /* Init an alias-oracle reference representation from a gimple pointer > PTR and a gimple size SIZE in bytes. If SIZE is NULL_TREE the the > size is assumed to be unknown. The access is assumed to be only > to or after of the pointer target, not before it. */ > > void > ao_ref_init_from_ptr_and_size (ao_ref *ref, tree ptr, tree size) > { > - HOST_WIDE_INT t1, t2; > + HOST_WIDE_INT t1, t2, extra_offset = 0; > ref->ref = NULL_TREE; > if (TREE_CODE (ptr) == SSA_NAME) > { > gimple stmt = SSA_NAME_DEF_STMT (ptr); > if (gimple_assign_single_p (stmt) > && gimple_assign_rhs_code (stmt) == ADDR_EXPR) > ptr = gimple_assign_rhs1 (stmt); > + else if (is_gimple_assign (stmt) > + && gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR > + && host_integerp (gimple_assign_rhs2 (stmt), 0) > + && (t1 = int_cst_value (gimple_assign_rhs2 (stmt))) >= 0) > + { > + ptr = gimple_assign_rhs1 (stmt); > + extra_offset = BITS_PER_UNIT * t1; > + } > } > > if (TREE_CODE (ptr) == ADDR_EXPR) > ref->base = get_ref_base_and_extent (TREE_OPERAND (ptr, 0), > &ref->offset, &t1, &t2); > else > { > ref->base = build2 (MEM_REF, char_type_node, > ptr, null_pointer_node); > ref->offset = 0; > } > + ref->offset += extra_offset; > if (size > && host_integerp (size, 0) > - && TREE_INT_CST_LOW (size) * 8 / 8 == TREE_INT_CST_LOW (size)) > - ref->max_size = ref->size = TREE_INT_CST_LOW (size) * 8; > + && TREE_INT_CST_LOW (size) * BITS_PER_UNIT / BITS_PER_UNIT > + == TREE_INT_CST_LOW (size)) > + ref->max_size = ref->size = TREE_INT_CST_LOW (size) * BITS_PER_UNIT; > else > ref->max_size = ref->size = -1; > ref->ref_alias_set = 0; > ref->base_alias_set = 0; > ref->volatile_p = false; > } > > /* Return 1 if TYPE1 and TYPE2 are to be considered equivalent for the > purpose of TBAA. Return 0 if they are distinct and -1 if we cannot > decide. */ >
Index: gcc/testsuite/gcc.dg/tree-ssa/alias-24.c =================================================================== --- gcc/testsuite/gcc.dg/tree-ssa/alias-24.c (revision 0) +++ gcc/testsuite/gcc.dg/tree-ssa/alias-24.c (working copy) @@ -0,0 +1,22 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-optimized" } */ + +void f (const char *c, int *i) +{ + *i = 42; + __builtin_memcpy (i + 1, c, sizeof (int)); + if (*i != 42) __builtin_abort(); +} + +extern void keepit (); +void g (const char *c, int *i) +{ + *i = 33; + __builtin_memcpy (i - 1, c, 3 * sizeof (int)); + if (*i != 33) keepit(); +} + +/* { dg-final { scan-tree-dump-not "abort" "optimized" } } */ +/* { dg-final { scan-tree-dump "keepit" "optimized" } } */ +/* { dg-final { cleanup-tree-dump "optimized" } } */ + Property changes on: gcc/testsuite/gcc.dg/tree-ssa/alias-24.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 204200) +++ gcc/tree-ssa-alias.c (working copy) @@ -559,43 +559,53 @@ ao_ref_alias_set (ao_ref *ref) } /* Init an alias-oracle reference representation from a gimple pointer PTR and a gimple size SIZE in bytes. If SIZE is NULL_TREE the the size is assumed to be unknown. The access is assumed to be only to or after of the pointer target, not before it. */ void ao_ref_init_from_ptr_and_size (ao_ref *ref, tree ptr, tree size) { - HOST_WIDE_INT t1, t2; + HOST_WIDE_INT t1, t2, extra_offset = 0; ref->ref = NULL_TREE; if (TREE_CODE (ptr) == SSA_NAME) { gimple stmt = SSA_NAME_DEF_STMT (ptr); if (gimple_assign_single_p (stmt) && gimple_assign_rhs_code (stmt) == ADDR_EXPR) ptr = gimple_assign_rhs1 (stmt); + else if (is_gimple_assign (stmt) + && gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR + && host_integerp (gimple_assign_rhs2 (stmt), 0) + && (t1 = int_cst_value (gimple_assign_rhs2 (stmt))) >= 0) + { + ptr = gimple_assign_rhs1 (stmt); + extra_offset = BITS_PER_UNIT * t1; + } } if (TREE_CODE (ptr) == ADDR_EXPR) ref->base = get_ref_base_and_extent (TREE_OPERAND (ptr, 0), &ref->offset, &t1, &t2); else { ref->base = build2 (MEM_REF, char_type_node, ptr, null_pointer_node); ref->offset = 0; } + ref->offset += extra_offset; if (size && host_integerp (size, 0) - && TREE_INT_CST_LOW (size) * 8 / 8 == TREE_INT_CST_LOW (size)) - ref->max_size = ref->size = TREE_INT_CST_LOW (size) * 8; + && TREE_INT_CST_LOW (size) * BITS_PER_UNIT / BITS_PER_UNIT + == TREE_INT_CST_LOW (size)) + ref->max_size = ref->size = TREE_INT_CST_LOW (size) * BITS_PER_UNIT; else ref->max_size = ref->size = -1; ref->ref_alias_set = 0; ref->base_alias_set = 0; ref->volatile_p = false; } /* Return 1 if TYPE1 and TYPE2 are to be considered equivalent for the purpose of TBAA. Return 0 if they are distinct and -1 if we cannot decide. */