Re: PR other/51165: add new adress_escapes predicate
On Tue, Jan 17, 2012 at 5:41 PM, Patrick Marlier
patrick.marl...@gmail.com wrote:
On 01/17/2012 08:20 AM, Aldy Hernandez wrote:
On 01/17/12 03:09, Richard Guenther wrote:
On Mon, Jan 16, 2012 at 4:58 PM, Aldy Hernandezal...@redhat.com wrote:
Not really - you handle both ptr and *ptr in the same predicate and
call both address escaped. What I suggested was sth like
I think I confused myself and you by asking the wrong question in the
first
place.
Actually, what I want is to handle VAR_DECL correctly, and your original
suggestion of using may_be_aliased() fits the bill. The other calls to
ptr_deref_may_alias_global_p() were fine because we have an SSA_NAME.
We're now down to a one-liner :).
How about this? All TM memory optimization tests fixed, and no
regressions.
Ok. Note that may_be_aliased (x) is also true if x is an automatic
variable
that has its address taken. You can use is_global_var (x) instead if you
only want to test for global memory.
Richard.
is_global_var is fine. Thanks.
Committed.
I disagree. is_global_var is already tested just above:
if (is_global_var (x))
return !TREE_READONLY (x);
Which make sense since we don't want to log if it is read-only.
So you can do probably the following:
Index: trans-mem.c
===
--- trans-mem.c (revision 183253)
+++ trans-mem.c (working copy)
@@ -1497,8 +1497,6 @@ requires_barrier (basic_block entry_block, tree x,
to needs_to_live_in_memory until we eliminate
lower_sequence_tm altogether. */
needs_to_live_in_memory (x)
- /* X escapes. */
- || is_global_var (x))
True, because needs_to_live_in_memory is true for all global vars already
(and includes all address-taken automatic vars as well, thus may_be_aliased (x),
too).
Richard.
return true;
else
{
Thanks.
Patrick Marlier.
Re: PR other/51165: add new adress_escapes predicate
So you can do probably the following:
Index: trans-mem.c
===
--- trans-mem.c (revision 183253)
+++ trans-mem.c (working copy)
@@ -1497,8 +1497,6 @@ requires_barrier (basic_block entry_block, tree x,
to needs_to_live_in_memory until we eliminate
lower_sequence_tm altogether. */
needs_to_live_in_memory (x)
- /* X escapes. */
- || is_global_var (x))
True, because needs_to_live_in_memory is true for all global vars already
(and includes all address-taken automatic vars as well, thus may_be_aliased (x),
too).
Sweet. Thanks guys.
Richard, I assume you are OK with this?
Tested on x86-64 Linux.
* trans-mem.c (requires_barrier): Remove call to is_global_var.
Index: trans-mem.c
===
--- trans-mem.c (revision 183272)
+++ trans-mem.c (working copy)
@@ -1496,9 +1496,7 @@ requires_barrier (basic_block entry_bloc
during lower_sequence_tm/gimplification, leave the call
to needs_to_live_in_memory until we eliminate
lower_sequence_tm altogether. */
- needs_to_live_in_memory (x)
- /* X escapes. */
- || is_global_var (x))
+ needs_to_live_in_memory (x))
return true;
else
{
Re: PR other/51165: add new adress_escapes predicate
On Wed, Jan 18, 2012 at 3:00 PM, Aldy Hernandez al...@redhat.com wrote: So you can do probably the following: Index: trans-mem.c === --- trans-mem.c (revision 183253) +++ trans-mem.c (working copy) @@ -1497,8 +1497,6 @@ requires_barrier (basic_block entry_block, tree x, to needs_to_live_in_memory until we eliminate lower_sequence_tm altogether. */ needs_to_live_in_memory (x) - /* X escapes. */ - || is_global_var (x)) True, because needs_to_live_in_memory is true for all global vars already (and includes all address-taken automatic vars as well, thus may_be_aliased (x), too). Sweet. Thanks guys. Richard, I assume you are OK with this? Yes. Richard. Tested on x86-64 Linux.
Re: PR other/51165: add new adress_escapes predicate
On Mon, Jan 16, 2012 at 4:58 PM, Aldy Hernandez al...@redhat.com wrote: Not really - you handle both ptr and *ptr in the same predicate and call both address escaped. What I suggested was sth like I think I confused myself and you by asking the wrong question in the first place. Actually, what I want is to handle VAR_DECL correctly, and your original suggestion of using may_be_aliased() fits the bill. The other calls to ptr_deref_may_alias_global_p() were fine because we have an SSA_NAME. We're now down to a one-liner :). How about this? All TM memory optimization tests fixed, and no regressions. Ok. Note that may_be_aliased (x) is also true if x is an automatic variable that has its address taken. You can use is_global_var (x) instead if you only want to test for global memory. Richard.
Re: PR other/51165: add new adress_escapes predicate
On 01/17/12 03:09, Richard Guenther wrote: On Mon, Jan 16, 2012 at 4:58 PM, Aldy Hernandezal...@redhat.com wrote: Not really - you handle both ptr and *ptr in the same predicate and call both address escaped. What I suggested was sth like I think I confused myself and you by asking the wrong question in the first place. Actually, what I want is to handle VAR_DECL correctly, and your original suggestion of using may_be_aliased() fits the bill. The other calls to ptr_deref_may_alias_global_p() were fine because we have an SSA_NAME. We're now down to a one-liner :). How about this? All TM memory optimization tests fixed, and no regressions. Ok. Note that may_be_aliased (x) is also true if x is an automatic variable that has its address taken. You can use is_global_var (x) instead if you only want to test for global memory. Richard. is_global_var is fine. Thanks. Committed.
Re: PR other/51165: add new adress_escapes predicate
On 01/17/2012 08:20 AM, Aldy Hernandez wrote:
On 01/17/12 03:09, Richard Guenther wrote:
On Mon, Jan 16, 2012 at 4:58 PM, Aldy Hernandezal...@redhat.com wrote:
Not really - you handle both ptr and *ptr in the same predicate and
call both address escaped. What I suggested was sth like
I think I confused myself and you by asking the wrong question in the
first
place.
Actually, what I want is to handle VAR_DECL correctly, and your original
suggestion of using may_be_aliased() fits the bill. The other calls to
ptr_deref_may_alias_global_p() were fine because we have an SSA_NAME.
We're now down to a one-liner :).
How about this? All TM memory optimization tests fixed, and no
regressions.
Ok. Note that may_be_aliased (x) is also true if x is an automatic
variable
that has its address taken. You can use is_global_var (x) instead if you
only want to test for global memory.
Richard.
is_global_var is fine. Thanks.
Committed.
I disagree. is_global_var is already tested just above:
if (is_global_var (x))
return !TREE_READONLY (x);
Which make sense since we don't want to log if it is read-only.
So you can do probably the following:
Index: trans-mem.c
===
--- trans-mem.c (revision 183253)
+++ trans-mem.c (working copy)
@@ -1497,8 +1497,6 @@ requires_barrier (basic_block entry_block, tree x,
to needs_to_live_in_memory until we eliminate
lower_sequence_tm altogether. */
needs_to_live_in_memory (x)
- /* X escapes. */
- || is_global_var (x))
return true;
else
{
Thanks.
Patrick Marlier.
PR other/51165: add new adress_escapes predicate
As discussed in the PR, the problem here is that we are using
ptr_deref_may_alias_global_p() to determine if a dereferenced address
escapes, whereas we were previously using the now non existent
is_call_clobbered. The function ptr_deref_may_alias_global_p() does not
understand SSA_NAMEs, whereas is_call_clobbered did.
Richi suggested using may_be_aliased() for DECLs.
The patch below abstracts an address_escapes_p() predicate for more
generic use into the aliasing code. Using this instead of
ptr_deref_may_alias_global_p() fixes all 4 TM memory optimization
regressions. TM logging is now back in business.
Is this what you had in mind? OK for trunk?
PR other/51165
* tree-ssa-alias.h: (address_escapes_p): Declare it.
* tree-ssa-alias.c (address_escapes_p): New.
* trans-mem.c (thread_private_new_memory): Use it.
(requires_barrier): Use it.
Index: testsuite/gcc.dg/tm/memopt-3.c
===
--- testsuite/gcc.dg/tm/memopt-3.c (revision 183072)
+++ testsuite/gcc.dg/tm/memopt-3.c (working copy)
@@ -16,5 +16,5 @@ int f()
return lala.x[0];
}
-/* { dg-final { scan-tree-dump-times logging: lala.x\\\[i_1\\\] 1 tmmark {
xfail *-*-* } } } */
+/* { dg-final { scan-tree-dump-times logging: lala.x\\\[i_1\\\] 1 tmmark }
} */
/* { dg-final { cleanup-tree-dump tmmark } } */
Index: testsuite/gcc.dg/tm/memopt-5.c
===
--- testsuite/gcc.dg/tm/memopt-5.c (revision 183072)
+++ testsuite/gcc.dg/tm/memopt-5.c (working copy)
@@ -19,5 +19,5 @@ int f()
return lala.x[i];
}
-/* { dg-final { scan-tree-dump-times ITM_LU\[0-9\] \\\(lala.x\\\[55\\\] 1
tmedge { xfail *-*-* } } } */
+/* { dg-final { scan-tree-dump-times ITM_LU\[0-9\] \\\(lala.x\\\[55\\\] 1
tmedge } } */
/* { dg-final { cleanup-tree-dump tmedge } } */
Index: testsuite/gcc.dg/tm/memopt-7.c
===
--- testsuite/gcc.dg/tm/memopt-7.c (revision 183072)
+++ testsuite/gcc.dg/tm/memopt-7.c (working copy)
@@ -17,6 +17,6 @@ int f()
return lala.x[asdf];
}
-/* { dg-final { scan-tree-dump-times tm_save.\[0-9_\]+ = lala 1 tmedge {
xfail *-*-* } } } */
-/* { dg-final { scan-tree-dump-times lala = tm_save 1 tmedge { xfail *-*-*
} } } */
+/* { dg-final { scan-tree-dump-times tm_save.\[0-9_\]+ = lala 1 tmedge } }
*/
+/* { dg-final { scan-tree-dump-times lala = tm_save 1 tmedge } } */
/* { dg-final { cleanup-tree-dump tmedge } } */
Index: testsuite/gcc.dg/tm/memopt-4.c
===
--- testsuite/gcc.dg/tm/memopt-4.c (revision 183072)
+++ testsuite/gcc.dg/tm/memopt-4.c (working copy)
@@ -19,6 +19,6 @@ int f()
return lala.x[i];
}
-/* { dg-final { scan-tree-dump-times tm_save.\[0-9_\]+ = lala.x\\\[55\\\] 1
tmedge { xfail *-*-* } } } */
-/* { dg-final { scan-tree-dump-times lala.x\\\[55\\\] = tm_save 1 tmedge {
xfail *-*-* } } } */
+/* { dg-final { scan-tree-dump-times tm_save.\[0-9_\]+ = lala.x\\\[55\\\] 1
tmedge } } */
+/* { dg-final { scan-tree-dump-times lala.x\\\[55\\\] = tm_save 1 tmedge }
} */
/* { dg-final { cleanup-tree-dump tmedge } } */
Index: tree-ssa-alias.c
===
--- tree-ssa-alias.c(revision 183072)
+++ tree-ssa-alias.c(working copy)
@@ -332,6 +332,20 @@ ptr_deref_may_alias_ref_p_1 (tree ptr, a
return true;
}
+/* Returns true if an address escapes the current function. */
+bool
+address_escapes_p (tree x)
+{
+ x = get_base_address (x);
+ if (TREE_CODE (x) == SSA_NAME)
+return ptr_deref_may_alias_global_p (x);
+ if (TREE_CODE (x) == MEM_REF)
+return ptr_deref_may_alias_global_p (TREE_OPERAND (x, 0));
+ if (DECL_P (x))
+return may_be_aliased (x);
+ return false;
+}
+
/* Dump alias information on FILE. */
Index: tree-ssa-alias.h
===
--- tree-ssa-alias.h(revision 183072)
+++ tree-ssa-alias.h(working copy)
@@ -99,6 +99,7 @@ extern tree ao_ref_base (ao_ref *);
extern alias_set_type ao_ref_alias_set (ao_ref *);
extern bool ptr_deref_may_alias_global_p (tree);
extern bool ptr_derefs_may_alias_p (tree, tree);
+extern bool address_escapes_p (tree);
extern bool refs_may_alias_p (tree, tree);
extern bool refs_may_alias_p_1 (ao_ref *, ao_ref *, bool);
extern bool refs_anti_dependent_p (tree, tree);
Index: trans-mem.c
===
--- trans-mem.c (revision 183072)
+++ trans-mem.c (working copy)
@@ -1347,7 +1347,7 @@ thread_private_new_memory (basic_block e
/* Search DEF chain to find the original definition of this address. */
do
{
- if (ptr_deref_may_alias_global_p (x))
+ if (address_escapes_p (x))
{
/* Address escapes. This is not thread-private. */
Re: PR other/51165: add new adress_escapes predicate
On Mon, Jan 16, 2012 at 3:29 PM, Aldy Hernandez al...@redhat.com wrote:
As discussed in the PR, the problem here is that we are using
ptr_deref_may_alias_global_p() to determine if a dereferenced address
escapes, whereas we were previously using the now non existent
is_call_clobbered. The function ptr_deref_may_alias_global_p() does not
understand SSA_NAMEs, whereas is_call_clobbered did.
Richi suggested using may_be_aliased() for DECLs.
The patch below abstracts an address_escapes_p() predicate for more generic
use into the aliasing code. Using this instead of
ptr_deref_may_alias_global_p() fixes all 4 TM memory optimization
regressions. TM logging is now back in business.
Is this what you had in mind? OK for trunk?
Not really - you handle both ptr and *ptr in the same predicate and
call both address escaped. What I suggested was sth like
/* Return true, if the memory access X may alias with a global variable. */
bool
access_may_refer_to_global_p (tree x)
{
x = get_base_address (x);
if (DECL_P (x))
return is_global_var (x);
else if (TREE_CODE (x) == MEM_REF
|| TREE_CODE (x) == TARGET_MEM_REF)
return ptr_deref_may_alias_global_p (TREE_OPERAND (x, 0));
return true;
}
Richard.
Re: PR other/51165: add new adress_escapes predicate
Not really - you handle both ptr and *ptr in the same predicate and
call both address escaped. What I suggested was sth like
I think I confused myself and you by asking the wrong question in the
first place.
Actually, what I want is to handle VAR_DECL correctly, and your original
suggestion of using may_be_aliased() fits the bill. The other calls to
ptr_deref_may_alias_global_p() were fine because we have an SSA_NAME.
We're now down to a one-liner :).
How about this? All TM memory optimization tests fixed, and no regressions.
PR other/51165
* trans-mem.c (requires_barrier): Call may_be_aliased.
testsuite/
PR other/51165
* gcc.dg/tm/memopt-3.c: Remove xfail.
* gcc.dg/tm/memopt-4.c: Remove xfail.
* gcc.dg/tm/memopt-5.c: Remove xfail.
* gcc.dg/tm/memopt-7.c: Remove xfail.
Index: testsuite/gcc.dg/tm/memopt-3.c
===
--- testsuite/gcc.dg/tm/memopt-3.c (revision 183072)
+++ testsuite/gcc.dg/tm/memopt-3.c (working copy)
@@ -16,5 +16,5 @@ int f()
return lala.x[0];
}
-/* { dg-final { scan-tree-dump-times logging: lala.x\\\[i_1\\\] 1 tmmark {
xfail *-*-* } } } */
+/* { dg-final { scan-tree-dump-times logging: lala.x\\\[i_1\\\] 1 tmmark }
} */
/* { dg-final { cleanup-tree-dump tmmark } } */
Index: testsuite/gcc.dg/tm/memopt-5.c
===
--- testsuite/gcc.dg/tm/memopt-5.c (revision 183072)
+++ testsuite/gcc.dg/tm/memopt-5.c (working copy)
@@ -19,5 +19,5 @@ int f()
return lala.x[i];
}
-/* { dg-final { scan-tree-dump-times ITM_LU\[0-9\] \\\(lala.x\\\[55\\\] 1
tmedge { xfail *-*-* } } } */
+/* { dg-final { scan-tree-dump-times ITM_LU\[0-9\] \\\(lala.x\\\[55\\\] 1
tmedge } } */
/* { dg-final { cleanup-tree-dump tmedge } } */
Index: testsuite/gcc.dg/tm/memopt-7.c
===
--- testsuite/gcc.dg/tm/memopt-7.c (revision 183072)
+++ testsuite/gcc.dg/tm/memopt-7.c (working copy)
@@ -17,6 +17,6 @@ int f()
return lala.x[asdf];
}
-/* { dg-final { scan-tree-dump-times tm_save.\[0-9_\]+ = lala 1 tmedge {
xfail *-*-* } } } */
-/* { dg-final { scan-tree-dump-times lala = tm_save 1 tmedge { xfail *-*-*
} } } */
+/* { dg-final { scan-tree-dump-times tm_save.\[0-9_\]+ = lala 1 tmedge } }
*/
+/* { dg-final { scan-tree-dump-times lala = tm_save 1 tmedge } } */
/* { dg-final { cleanup-tree-dump tmedge } } */
Index: testsuite/gcc.dg/tm/memopt-4.c
===
--- testsuite/gcc.dg/tm/memopt-4.c (revision 183072)
+++ testsuite/gcc.dg/tm/memopt-4.c (working copy)
@@ -19,6 +19,6 @@ int f()
return lala.x[i];
}
-/* { dg-final { scan-tree-dump-times tm_save.\[0-9_\]+ = lala.x\\\[55\\\] 1
tmedge { xfail *-*-* } } } */
-/* { dg-final { scan-tree-dump-times lala.x\\\[55\\\] = tm_save 1 tmedge {
xfail *-*-* } } } */
+/* { dg-final { scan-tree-dump-times tm_save.\[0-9_\]+ = lala.x\\\[55\\\] 1
tmedge } } */
+/* { dg-final { scan-tree-dump-times lala.x\\\[55\\\] = tm_save 1 tmedge }
} */
/* { dg-final { cleanup-tree-dump tmedge } } */
Index: trans-mem.c
===
--- trans-mem.c (revision 183072)
+++ trans-mem.c (working copy)
@@ -1498,7 +1498,7 @@ requires_barrier (basic_block entry_bloc
lower_sequence_tm altogether. */
needs_to_live_in_memory (x)
/* X escapes. */
- || ptr_deref_may_alias_global_p (x))
+ || may_be_aliased (x))
return true;
else
{
