This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Aliasing: look through pointer's def stmt
- From: Richard Biener <richard dot guenther at gmail dot com>
- To: Marc Glisse <marc dot glisse at inria dot fr>
- Cc: Jeff Law <law at redhat dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Wed, 30 Oct 2013 15:23:25 +0100
- Subject: Re: Aliasing: look through pointer's def stmt
- Authentication-results: sourceware.org; auth=none
- References: <alpine dot DEB dot 2 dot 02 dot 1310250707430 dot 14734 at stedding dot saclay dot inria dot fr> <526A0269 dot 8030407 at redhat dot com> <alpine dot DEB dot 2 dot 10 dot 1310250809590 dot 4165 at laptop-mg dot saclay dot inria dot fr> <CAFiYyc27=sN1itn+N09rpmm8tWGt-Khu7oASLd0ODsdZh=ofzw at mail dot gmail dot com> <CAFiYyc2EtayY0tiW1pz4t-zf0uVC24UcF5s7yaUrY3f7_jxoVg at mail dot gmail dot com> <alpine dot DEB dot 2 dot 10 dot 1310251123350 dot 8620 at laptop-mg dot saclay dot inria dot fr> <CAFiYyc2WmH4WNZ7h8-8iC_6mSO50PdLNLxNzQOYZrYEvn1oQkQ at mail dot gmail dot com> <alpine dot DEB dot 2 dot 02 dot 1310251852480 dot 9786 at stedding dot saclay dot inria dot fr> <alpine dot DEB dot 2 dot 10 dot 1310261838280 dot 15626 at laptop-mg dot saclay dot inria dot fr> <CAFiYyc3dZr1DrQV7X+6kN=eFFx7CKNhbHOdBgqt_bWLp0xe07w at mail dot gmail dot com> <alpine dot DEB dot 2 dot 02 dot 1310300102500 dot 21750 at stedding dot saclay dot inria dot fr> <CAFiYyc0=UP2RTdfoR9pDTSo4RnaLnXOQS8O0QP51gfKz4UmnvQ at mail dot gmail dot com> <alpine dot DEB dot 2 dot 10 dot 1310301121490 dot 4464 at stedding dot saclay dot inria dot fr>
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. */
>