* [RFA][PATCH] Minor fix to aliasing machinery
@ 2013-10-29 21:58 Jeff Law
2013-10-29 23:11 ` Marc Glisse
2013-10-30 9:45 ` Richard Biener
0 siblings, 2 replies; 12+ messages in thread
From: Jeff Law @ 2013-10-29 21:58 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 519 bytes --]
Marc pointed out that the handling of various BUILT_IN_MEM* and
BUILT_IN_STR* functions in tree-ssa-alias.c probably wasn't working as
intended because the code wasn't prepared for a common return value from
ao_ref_base, particularly returns of MEM_REFs.
This patch fixes the code to handle the trivial case of returning a
MEM_REF and adds a simple testcase. There's probably a lot more that
could be done here.
Bootstrapped and regression tested on x86_64-unknown-linux-gnu. Ok for
the trunk?
Thanks,
Jeff
[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 1546 bytes --]
* tree-ssa-alias.c (stmt_kills_ref_p_1): Handle case where
ao_ref_base returns a MEM_REF.
* gcc.dg/tree-ssa/alias-26.c: New test.
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/alias-26.c b/gcc/testsuite/gcc.dg/tree-ssa/alias-26.c
new file mode 100644
index 0000000..b5625b8
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/alias-26.c
@@ -0,0 +1,12 @@
+/* { 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" } } */
+
diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
index 4db83bd..5120e72 100644
--- a/gcc/tree-ssa-alias.c
+++ b/gcc/tree-ssa-alias.c
@@ -2079,6 +2079,7 @@ stmt_kills_ref_p_1 (gimple stmt, ao_ref *ref)
tree dest = gimple_call_arg (stmt, 0);
tree len = gimple_call_arg (stmt, 2);
tree base = NULL_TREE;
+ tree ref_base;
HOST_WIDE_INT offset = 0;
if (!host_integerp (len, 0))
return false;
@@ -2087,8 +2088,11 @@ stmt_kills_ref_p_1 (gimple stmt, ao_ref *ref)
&offset);
else if (TREE_CODE (dest) == SSA_NAME)
base = dest;
+ ref_base = ao_ref_base (ref);
if (base
- && base == ao_ref_base (ref))
+ && ((TREE_CODE (ref_base) == MEM_REF
+ && base == TREE_OPERAND (ref_base, 0))
+ || ref_base == base))
{
HOST_WIDE_INT size = TREE_INT_CST_LOW (len);
if (offset <= ref->offset / BITS_PER_UNIT
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFA][PATCH] Minor fix to aliasing machinery
2013-10-29 21:58 [RFA][PATCH] Minor fix to aliasing machinery Jeff Law
@ 2013-10-29 23:11 ` Marc Glisse
2013-10-30 9:46 ` Richard Biener
2013-10-30 9:45 ` Richard Biener
1 sibling, 1 reply; 12+ messages in thread
From: Marc Glisse @ 2013-10-29 23:11 UTC (permalink / raw)
To: Jeff Law; +Cc: gcc-patches
On Tue, 29 Oct 2013, Jeff Law wrote:
> Marc pointed out that the handling of various BUILT_IN_MEM* and BUILT_IN_STR*
> functions in tree-ssa-alias.c probably wasn't working as intended because the
> code wasn't prepared for a common return value from ao_ref_base, particularly
> returns of MEM_REFs.
Hmm, ao_ref_base is never a pointer, so I'd say the issue is really with
trying to use the SSA_NAME directly.
> This patch fixes the code to handle the trivial case of returning a MEM_REF
> and adds a simple testcase. There's probably a lot more that could be done
> here.
Thanks.
I am not sure we want to keep the variable "base" that is either a
decl/ref (from get_addr_base_and_unit_offset) or a pointer (dest). We know
which case is which, but then forget it by storing both into base. Maybe
something like this would be more "type-safe".
bool same = false;
if (TREE_CODE (dest) == ADDR_EXPR)
same = (ref_base == get_addr_base_and_unit_offset
(TREE_OPERAND (dest, 0), &offset));
else if (TREE_CODE (dest) == SSA_NAME
&& TREE_CODE (ref_base) == MEM_REF)
same = (TREE_OPERAND (ref_base, 0) == dest);
if (same)
...
By the way, I think the patch is fine as is, I am only discussing possible
follow-ups.
(see http://gcc.gnu.org/ml/gcc-patches/2013-10/txto0PQEYpiuz.txt for
another approach using ao_ref_init_from_ptr_and_size)
--
Marc Glisse
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFA][PATCH] Minor fix to aliasing machinery
2013-10-29 21:58 [RFA][PATCH] Minor fix to aliasing machinery Jeff Law
2013-10-29 23:11 ` Marc Glisse
@ 2013-10-30 9:45 ` Richard Biener
2013-10-30 16:11 ` Jeff Law
1 sibling, 1 reply; 12+ messages in thread
From: Richard Biener @ 2013-10-30 9:45 UTC (permalink / raw)
To: Jeff Law; +Cc: gcc-patches
On Tue, Oct 29, 2013 at 10:36 PM, Jeff Law <law@redhat.com> wrote:
>
> Marc pointed out that the handling of various BUILT_IN_MEM* and
> BUILT_IN_STR* functions in tree-ssa-alias.c probably wasn't working as
> intended because the code wasn't prepared for a common return value from
> ao_ref_base, particularly returns of MEM_REFs.
>
> This patch fixes the code to handle the trivial case of returning a MEM_REF
> and adds a simple testcase. There's probably a lot more that could be done
> here.
>
> Bootstrapped and regression tested on x86_64-unknown-linux-gnu. Ok for the
> trunk?
>
> Thanks,
> Jeff
>
> * tree-ssa-alias.c (stmt_kills_ref_p_1): Handle case where
> ao_ref_base returns a MEM_REF.
>
> * gcc.dg/tree-ssa/alias-26.c: New test.
>
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/alias-26.c
> b/gcc/testsuite/gcc.dg/tree-ssa/alias-26.c
> new file mode 100644
> index 0000000..b5625b8
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/alias-26.c
> @@ -0,0 +1,12 @@
> +/* { 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" } } */
> +
> diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
> index 4db83bd..5120e72 100644
> --- a/gcc/tree-ssa-alias.c
> +++ b/gcc/tree-ssa-alias.c
> @@ -2079,6 +2079,7 @@ stmt_kills_ref_p_1 (gimple stmt, ao_ref *ref)
> tree dest = gimple_call_arg (stmt, 0);
> tree len = gimple_call_arg (stmt, 2);
> tree base = NULL_TREE;
> + tree ref_base;
> HOST_WIDE_INT offset = 0;
> if (!host_integerp (len, 0))
> return false;
> @@ -2087,8 +2088,11 @@ stmt_kills_ref_p_1 (gimple stmt, ao_ref *ref)
> &offset);
> else if (TREE_CODE (dest) == SSA_NAME)
> base = dest;
> + ref_base = ao_ref_base (ref);
> if (base
> - && base == ao_ref_base (ref))
> + && ((TREE_CODE (ref_base) == MEM_REF
> + && base == TREE_OPERAND (ref_base, 0))
That's not sufficient - ref_base may have an offset, so for correctness
you have to check that integer_zerop (TREE_OPERAND (ref_base, 0)).
But this now looks convoluted and somewhat backward, and still
does not catch all cases (including the def-stmt lookup recently
added to ao_ref_from_ptr_and_size).
Richard.
> + || ref_base == base))
> {
> HOST_WIDE_INT size = TREE_INT_CST_LOW (len);
> if (offset <= ref->offset / BITS_PER_UNIT
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFA][PATCH] Minor fix to aliasing machinery
2013-10-29 23:11 ` Marc Glisse
@ 2013-10-30 9:46 ` Richard Biener
2013-10-30 16:19 ` Marc Glisse
0 siblings, 1 reply; 12+ messages in thread
From: Richard Biener @ 2013-10-30 9:46 UTC (permalink / raw)
To: Marc Glisse; +Cc: Jeff Law, gcc-patches
On Tue, Oct 29, 2013 at 11:23 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> On Tue, 29 Oct 2013, Jeff Law wrote:
>
>> Marc pointed out that the handling of various BUILT_IN_MEM* and
>> BUILT_IN_STR* functions in tree-ssa-alias.c probably wasn't working as
>> intended because the code wasn't prepared for a common return value from
>> ao_ref_base, particularly returns of MEM_REFs.
>
>
> Hmm, ao_ref_base is never a pointer, so I'd say the issue is really with
> trying to use the SSA_NAME directly.
Yes. Note that the code tries to relate two pointers but one is a
memory-reference (ao_ref_base) and one is either a SSA name pointer
or the result of get_addr_base_and_unit_offset (a memory reference as well).
>> This patch fixes the code to handle the trivial case of returning a
>> MEM_REF and adds a simple testcase. There's probably a lot more that could
>> be done here.
>
>
> Thanks.
>
> I am not sure we want to keep the variable "base" that is either a decl/ref
> (from get_addr_base_and_unit_offset) or a pointer (dest). We know which case
> is which, but then forget it by storing both into base. Maybe something like
> this would be more "type-safe".
>
> bool same = false;
> if (TREE_CODE (dest) == ADDR_EXPR)
> same = (ref_base == get_addr_base_and_unit_offset
> (TREE_OPERAND (dest, 0), &offset));
> else if (TREE_CODE (dest) == SSA_NAME
> && TREE_CODE (ref_base) == MEM_REF)
&& integer_zerop (TREE_OPERAND (ref_base, 1))
> same = (TREE_OPERAND (ref_base, 0) == dest);
> if (same)
> ...
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.
> By the way, I think the patch is fine as is, I am only discussing possible
> follow-ups.
Only slightly wrong-codish ;)
Richard.
> (see http://gcc.gnu.org/ml/gcc-patches/2013-10/txto0PQEYpiuz.txt for another
> approach using ao_ref_init_from_ptr_and_size)
>
> --
> Marc Glisse
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFA][PATCH] Minor fix to aliasing machinery
2013-10-30 9:45 ` Richard Biener
@ 2013-10-30 16:11 ` Jeff Law
0 siblings, 0 replies; 12+ messages in thread
From: Jeff Law @ 2013-10-30 16:11 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
On 10/30/13 03:34, Richard Biener wrote:
>>
>> * tree-ssa-alias.c (stmt_kills_ref_p_1): Handle case where
>> ao_ref_base returns a MEM_REF.
>>
>> * gcc.dg/tree-ssa/alias-26.c: New test.
>>
>> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/alias-26.c
>> b/gcc/testsuite/gcc.dg/tree-ssa/alias-26.c
>> new file mode 100644
>> index 0000000..b5625b8
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/tree-ssa/alias-26.c
>> @@ -0,0 +1,12 @@
>> +/* { 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" } } */
>> +
>> diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
>> index 4db83bd..5120e72 100644
>> --- a/gcc/tree-ssa-alias.c
>> +++ b/gcc/tree-ssa-alias.c
>> @@ -2079,6 +2079,7 @@ stmt_kills_ref_p_1 (gimple stmt, ao_ref *ref)
>> tree dest = gimple_call_arg (stmt, 0);
>> tree len = gimple_call_arg (stmt, 2);
>> tree base = NULL_TREE;
>> + tree ref_base;
>> HOST_WIDE_INT offset = 0;
>> if (!host_integerp (len, 0))
>> return false;
>> @@ -2087,8 +2088,11 @@ stmt_kills_ref_p_1 (gimple stmt, ao_ref *ref)
>> &offset);
>> else if (TREE_CODE (dest) == SSA_NAME)
>> base = dest;
>> + ref_base = ao_ref_base (ref);
>> if (base
>> - && base == ao_ref_base (ref))
>> + && ((TREE_CODE (ref_base) == MEM_REF
>> + && base == TREE_OPERAND (ref_base, 0))
>
> That's not sufficient - ref_base may have an offset, so for correctness
> you have to check that integer_zerop (TREE_OPERAND (ref_base, 0)).
> But this now looks convoluted and somewhat backward, and still
> does not catch all cases (including the def-stmt lookup recently
> added to ao_ref_from_ptr_and_size).
So how do you want to proceed? I'm not really up for burning through
this code right now and trying to sort out how it ought to work.
Perhaps checkin the test (xfailed) and wait for someone with the
interest and time to push this through to completion?
jeff
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFA][PATCH] Minor fix to aliasing machinery
2013-10-30 9:46 ` Richard Biener
@ 2013-10-30 16:19 ` Marc Glisse
2013-11-06 12:41 ` Marc Glisse
0 siblings, 1 reply; 12+ messages in thread
From: Marc Glisse @ 2013-10-30 16:19 UTC (permalink / raw)
To: Richard Biener; +Cc: Jeff Law, gcc-patches
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1170 bytes --]
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):
! { dg-do run }
! { dg-options "-fbackslash" }
implicit none
character(kind=1,len=3) :: s1(3)
s1 = [ "abc", "def", "ghi" ]
if (any (cshift (s1, -1) /= [ s1(3), s1(1:2) ])) call abort
end
we end up with a double array_ref, one of variable index, and
get_ref_base_and_extent signals that by returning as size the size of the
whole declaration (the double-array). However,
ao_ref_init_from_ptr_and_size happily ignores the last two parameters of
get_ref_base_and_extent and continues as if nothing was wrong (I don't
know how to easily check that something went wrong either). Whether we
think this is a good/bad patch for memcpy, we have a latent bug that the
recent patches are making more visible.
--
Marc Glisse
[-- Attachment #2: Type: TEXT/PLAIN, Size: 4035 bytes --]
Index: testsuite/gcc.dg/tree-ssa/alias-26.c
===================================================================
--- testsuite/gcc.dg/tree-ssa/alias-26.c (revision 0)
+++ testsuite/gcc.dg/tree-ssa/alias-26.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: testsuite/gcc.dg/tree-ssa/alias-26.c
___________________________________________________________________
Added: svn:eol-style
## -0,0 +1 ##
+native
\ No newline at end of property
Added: svn:keywords
## -0,0 +1 ##
+Author Date Id Revision URL
\ No newline at end of property
Index: tree-ssa-alias.c
===================================================================
--- tree-ssa-alias.c (revision 204199)
+++ tree-ssa-alias.c (working copy)
@@ -1982,23 +1982,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. */
@@ -2071,37 +2072,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);
+ tree base = ao_ref_base (&dref);
+ HOST_WIDE_INT offset = dref.offset;
+ if (!base)
+ return false;
+ if (TREE_CODE (base) == MEM_REF)
+ {
+ if (TREE_CODE (rbase) != MEM_REF)
+ return false;
+ // Compare pointers.
+ offset += BITS_PER_UNIT
+ * TREE_INT_CST_LOW (TREE_OPERAND (base, 1));
+ roffset += BITS_PER_UNIT
+ * TREE_INT_CST_LOW (TREE_OPERAND (rbase, 1));
+ base = TREE_OPERAND (base, 0);
+ rbase = TREE_OPERAND (rbase, 0);
+ }
+ if (base == rbase)
{
- HOST_WIDE_INT size = TREE_INT_CST_LOW (len);
- if (offset <= ref->offset / BITS_PER_UNIT
- && (offset + size
- >= ((ref->offset + ref->max_size + BITS_PER_UNIT - 1)
- / BITS_PER_UNIT)))
+ HOST_WIDE_INT size = BITS_PER_UNIT * TREE_INT_CST_LOW (len);
+ if (offset <= roffset
+ && offset + size >= (roffset + ref->max_size))
return true;
}
break;
}
case BUILT_IN_VA_END:
{
tree ptr = gimple_call_arg (stmt, 0);
if (TREE_CODE (ptr) == ADDR_EXPR)
{
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFA][PATCH] Minor fix to aliasing machinery
2013-10-30 16:19 ` Marc Glisse
@ 2013-11-06 12:41 ` Marc Glisse
2013-11-06 14:04 ` Richard Biener
0 siblings, 1 reply; 12+ messages in thread
From: Marc Glisse @ 2013-11-06 12:41 UTC (permalink / raw)
To: Richard Biener; +Cc: Jeff Law, gcc-patches
[-- Attachment #1: Type: TEXT/PLAIN, Size: 978 bytes --]
[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.glisse@inria.fr>
Jeff Law <law@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
[-- Attachment #2: Type: TEXT/PLAIN, Size: 4082 bytes --]
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);
+ 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)
+ return false;
+ // Compare pointers.
+ offset += BITS_PER_UNIT
+ * TREE_INT_CST_LOW (TREE_OPERAND (base, 1));
+ roffset += BITS_PER_UNIT
+ * TREE_INT_CST_LOW (TREE_OPERAND (rbase, 1));
+ base = TREE_OPERAND (base, 0);
+ rbase = TREE_OPERAND (rbase, 0);
+ }
+ if (base == rbase)
{
- HOST_WIDE_INT size = TREE_INT_CST_LOW (len);
- if (offset <= ref->offset / BITS_PER_UNIT
- && (offset + size
- >= ((ref->offset + ref->max_size + BITS_PER_UNIT - 1)
- / BITS_PER_UNIT)))
+ HOST_WIDE_INT size = BITS_PER_UNIT * TREE_INT_CST_LOW (len);
+ if (offset <= roffset
+ && offset + size >= (roffset + ref->max_size))
return true;
}
break;
}
case BUILT_IN_VA_END:
{
tree ptr = gimple_call_arg (stmt, 0);
if (TREE_CODE (ptr) == ADDR_EXPR)
{
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFA][PATCH] Minor fix to aliasing machinery
2013-11-06 12:41 ` Marc Glisse
@ 2013-11-06 14:04 ` Richard Biener
2013-11-06 14:13 ` Marc Glisse
0 siblings, 1 reply; 12+ messages in thread
From: Richard Biener @ 2013-11-06 14:04 UTC (permalink / raw)
To: Marc Glisse; +Cc: Jeff Law, gcc-patches
On Wed, Nov 6, 2013 at 1:19 PM, Marc Glisse <marc.glisse@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.glisse@inria.fr>
> Jeff Law <law@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.
> + return false;
> + // Compare pointers.
> + offset += BITS_PER_UNIT
> + * TREE_INT_CST_LOW (TREE_OPERAND (base, 1));
Use mem_ref_offset (base).
> + 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);
Thanks,
Richard.
> + }
> + if (base == rbase)
> {
> - HOST_WIDE_INT size = TREE_INT_CST_LOW (len);
> - if (offset <= ref->offset / BITS_PER_UNIT
> - && (offset + size
> - >= ((ref->offset + ref->max_size + BITS_PER_UNIT -
> 1)
> - / BITS_PER_UNIT)))
> + HOST_WIDE_INT size = BITS_PER_UNIT * TREE_INT_CST_LOW
> (len);
> + if (offset <= roffset
> + && offset + size >= (roffset + ref->max_size))
> return true;
> }
> break;
> }
>
> case BUILT_IN_VA_END:
> {
> tree ptr = gimple_call_arg (stmt, 0);
> if (TREE_CODE (ptr) == ADDR_EXPR)
> {
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFA][PATCH] Minor fix to aliasing machinery
2013-11-06 14:04 ` Richard Biener
@ 2013-11-06 14:13 ` Marc Glisse
2013-11-06 14:42 ` Richard Biener
0 siblings, 1 reply; 12+ messages in thread
From: Marc Glisse @ 2013-11-06 14:13 UTC (permalink / raw)
To: Richard Biener; +Cc: Jeff Law, gcc-patches
On Wed, 6 Nov 2013, Richard Biener wrote:
> On Wed, Nov 6, 2013 at 1:19 PM, Marc Glisse <marc.glisse@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.glisse@inria.fr>
>> Jeff Law <law@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.
>> + 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();
or did you mean the computations should use double_int?
>> + 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?
--
Marc Glisse
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFA][PATCH] Minor fix to aliasing machinery
2013-11-06 14:13 ` Marc Glisse
@ 2013-11-06 14:42 ` Richard Biener
2013-11-09 10:00 ` Marc Glisse
0 siblings, 1 reply; 12+ messages in thread
From: Richard Biener @ 2013-11-06 14:42 UTC (permalink / raw)
To: Marc Glisse; +Cc: Jeff Law, gcc-patches
On Wed, Nov 6, 2013 at 3:08 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> On Wed, 6 Nov 2013, Richard Biener wrote:
>
>> On Wed, Nov 6, 2013 at 1:19 PM, Marc Glisse <marc.glisse@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.glisse@inria.fr>
>>> Jeff Law <law@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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFA][PATCH] Minor fix to aliasing machinery
2013-11-06 14:42 ` Richard Biener
@ 2013-11-09 10:00 ` Marc Glisse
2013-11-11 11:58 ` Richard Biener
0 siblings, 1 reply; 12+ messages in thread
From: Marc Glisse @ 2013-11-09 10:00 UTC (permalink / raw)
To: Richard Biener; +Cc: Jeff Law, gcc-patches
[-- Attachment #1: Type: TEXT/PLAIN, Size: 683 bytes --]
On Wed, 6 Nov 2013, Richard Biener wrote:
> 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.
Here it is (bootstrap+testsuite on x86_64-unknown-linux-gnu). It feels
rather artificial to do this small bit of computation with double_int
when so much else assumes HWI is enough, but why not...
2013-11-09 Marc Glisse <marc.glisse@inria.fr>
Jeff Law <law@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
[-- Attachment #2: Type: TEXT/PLAIN, Size: 4161 bytes --]
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 204608)
+++ 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;
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))
+ tree rbase = ref->base;
+ double_int roffset = double_int::from_shwi (ref->offset);
+ ao_ref dref;
+ ao_ref_init_from_ptr_and_size (&dref, dest, len);
+ tree base = ao_ref_base (&dref);
+ double_int offset = double_int::from_shwi (dref.offset);
+ double_int bpu = double_int::from_uhwi (BITS_PER_UNIT);
+ if (!base || dref.size == -1)
+ return false;
+ if (TREE_CODE (base) == MEM_REF)
+ {
+ if (TREE_CODE (rbase) != MEM_REF)
+ return false;
+ // Compare pointers.
+ offset += bpu * mem_ref_offset (base);
+ roffset += bpu * mem_ref_offset (rbase);
+ base = TREE_OPERAND (base, 0);
+ rbase = TREE_OPERAND (rbase, 0);
+ }
+ if (base == rbase)
{
- HOST_WIDE_INT size = TREE_INT_CST_LOW (len);
- if (offset <= ref->offset / BITS_PER_UNIT
- && (offset + size
- >= ((ref->offset + ref->max_size + BITS_PER_UNIT - 1)
- / BITS_PER_UNIT)))
+ double_int size = bpu * tree_to_double_int (len);
+ double_int rsize = double_int::from_uhwi (ref->max_size);
+ if (offset.sle (roffset)
+ && (roffset + rsize).sle (offset + size))
return true;
}
break;
}
case BUILT_IN_VA_END:
{
tree ptr = gimple_call_arg (stmt, 0);
if (TREE_CODE (ptr) == ADDR_EXPR)
{
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFA][PATCH] Minor fix to aliasing machinery
2013-11-09 10:00 ` Marc Glisse
@ 2013-11-11 11:58 ` Richard Biener
0 siblings, 0 replies; 12+ messages in thread
From: Richard Biener @ 2013-11-11 11:58 UTC (permalink / raw)
To: Marc Glisse; +Cc: Jeff Law, gcc-patches
On Sat, Nov 9, 2013 at 12:18 AM, Marc Glisse <marc.glisse@inria.fr> wrote:
> On Wed, 6 Nov 2013, Richard Biener wrote:
>
>> 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.
>
>
> Here it is (bootstrap+testsuite on x86_64-unknown-linux-gnu). It feels
> rather artificial to do this small bit of computation with double_int
> when so much else assumes HWI is enough, but why not...
Ok.
Thanks,
Richard.
> 2013-11-09 Marc Glisse <marc.glisse@inria.fr>
>
> Jeff Law <law@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 204608)
> +++ 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;
> 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))
> + tree rbase = ref->base;
> + double_int roffset = double_int::from_shwi (ref->offset);
> + ao_ref dref;
> + ao_ref_init_from_ptr_and_size (&dref, dest, len);
> + tree base = ao_ref_base (&dref);
> + double_int offset = double_int::from_shwi (dref.offset);
> + double_int bpu = double_int::from_uhwi (BITS_PER_UNIT);
> + if (!base || dref.size == -1)
> + return false;
> + if (TREE_CODE (base) == MEM_REF)
> + {
> + if (TREE_CODE (rbase) != MEM_REF)
> + return false;
> + // Compare pointers.
> + offset += bpu * mem_ref_offset (base);
> + roffset += bpu * mem_ref_offset (rbase);
> + base = TREE_OPERAND (base, 0);
> + rbase = TREE_OPERAND (rbase, 0);
> + }
> + if (base == rbase)
> {
> - HOST_WIDE_INT size = TREE_INT_CST_LOW (len);
> - if (offset <= ref->offset / BITS_PER_UNIT
> - && (offset + size
> - >= ((ref->offset + ref->max_size + BITS_PER_UNIT -
> 1)
> - / BITS_PER_UNIT)))
> + double_int size = bpu * tree_to_double_int (len);
> + double_int rsize = double_int::from_uhwi (ref->max_size);
> + if (offset.sle (roffset)
> + && (roffset + rsize).sle (offset + size))
> return true;
> }
> break;
> }
>
> case BUILT_IN_VA_END:
> {
> tree ptr = gimple_call_arg (stmt, 0);
> if (TREE_CODE (ptr) == ADDR_EXPR)
> {
>
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2013-11-11 11:43 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-29 21:58 [RFA][PATCH] Minor fix to aliasing machinery Jeff Law
2013-10-29 23:11 ` Marc Glisse
2013-10-30 9:46 ` Richard Biener
2013-10-30 16:19 ` Marc Glisse
2013-11-06 12:41 ` Marc Glisse
2013-11-06 14:04 ` Richard Biener
2013-11-06 14:13 ` Marc Glisse
2013-11-06 14:42 ` Richard Biener
2013-11-09 10:00 ` Marc Glisse
2013-11-11 11:58 ` Richard Biener
2013-10-30 9:45 ` Richard Biener
2013-10-30 16:11 ` Jeff Law
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).