On Mon, 28 Oct 2013, Jeff Law wrote:
On 10/26/13 01:15, Marc Glisse wrote:
Hello,
this patch teaches gcc that free kills the memory its argument points
to. The equality test is probably too strict, I guess we can loosen it
later (unless you have suggestions?).
Note that the corresponding code for BUILT_IN_MEMCPY and others seems
suspicious to me, it looks like it is testing for equality between a
pointer and a mem_ref, which is unlikely to happen.
Bootstrap+testsuite on x86_64-unknown-linux-gnu.
2013-10-27 Marc Glisse <marc.gli...@inria.fr>
PR tree-optimization/19831
gcc/
* tree-ssa-alias.c (stmt_kills_ref_p_1): Handle BUILT_IN_FREE.
gcc/testsuite/
* gcc.dg/tree-ssa/alias-25.c: New file.
OK for the trunk.
Thanks.
I agree the MEM_REF* and VA_END cases look strange and I think they're wrong
as well. Did you happen to try fixing them and running the testsuite to see
if anything fails?
ISTM your testcase could be tweaked to test conclusively if they're doing the
right thing -- and regardless of what's found that test can/should make its
way into the testsuite.
I checked and it does the wrong thing (I don't have the testcase handy
anymore, but it shouldn't be hard to recreate one), I even wrote a patch
(attached) but it is related to:
http://gcc.gnu.org/ml/gcc-patches/2013-10/msg02238.html
so it can't go in. A more limited fix (still missing many cases) would be
possible. I didn't test VA_END, but it doesn't look as bad (it is the
SSA_NAME case that is wrong for memcpy).
While I am posting non-submitted patches, does the following vaguely make
sense? I couldn't find a testcase that exercised it without some local
patches, but the idea (IIRC) is that with:
struct A { struct B b; int i; }
we can easily end up with one ao_ref.base that is a MEM_REF[p] and
another one a MEM_REF[(B*)q] where p and q are of type A*, and that
prevents us from noticing that p->i and q->b don't alias. Maybe I should
instead find a way to get MEM_REF[q] as ao_ref.base, but if q actually
points to a larger structure that starts with A it is likely to fail as
well...
--- gcc/tree-ssa-alias.c (revision 204095)
+++ gcc/tree-ssa-alias.c (working copy)
@@ -1099,22 +1099,24 @@ indirect_refs_may_alias_p (tree ref1 ATT
/* If both references are through the same type, they do not alias
if the accesses do not overlap. This does extra disambiguation
for mixed/pointer accesses but requires strict aliasing. */
if ((TREE_CODE (base1) != TARGET_MEM_REF
|| (!TMR_INDEX (base1) && !TMR_INDEX2 (base1)))
&& (TREE_CODE (base2) != TARGET_MEM_REF
|| (!TMR_INDEX (base2) && !TMR_INDEX2 (base2)))
&& same_type_for_tbaa (TREE_TYPE (base1), TREE_TYPE (ptrtype1)) == 1
&& same_type_for_tbaa (TREE_TYPE (base2), TREE_TYPE (ptrtype2)) == 1
- && same_type_for_tbaa (TREE_TYPE (ptrtype1),
- TREE_TYPE (ptrtype2)) == 1)
+ && (same_type_for_tbaa (TREE_TYPE (ptrtype1),
+ TREE_TYPE (ptrtype2)) == 1
+ || same_type_for_tbaa (TREE_TYPE (TREE_TYPE (ptr1)),
+ TREE_TYPE (TREE_TYPE (ptr2))) == 1))
return ranges_overlap_p (offset1, max_size1, offset2, max_size2);
/* Do type-based disambiguation. */
if (base1_alias_set != base2_alias_set
&& !alias_sets_conflict_p (base1_alias_set, base2_alias_set))
return false;
/* Do access-path based disambiguation. */
if (ref1 && ref2
&& (handled_component_p (ref1) || handled_component_p (ref2))
In the category "ugly hack that I won't submit", let me also attach this
patch that sometimes helps notice that malloc doesn't alias other things
(I don't know if the first hunk ever fires).
--
Marc Glisse
Index: gcc/tree-ssa-alias.c
===================================================================
--- gcc/tree-ssa-alias.c (revision 204089)
+++ gcc/tree-ssa-alias.c (working copy)
@@ -1985,23 +1985,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. */
@@ -2064,37 +2065,45 @@ 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 += 8 * TREE_INT_CST_LOW (TREE_OPERAND (base, 1));
+ roffset += 8 * 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 = 8 * 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)
{
Index: gcc/tree-ssa-alias.c
===================================================================
--- gcc/tree-ssa-alias.c (revision 204095)
+++ gcc/tree-ssa-alias.c (working copy)
@@ -207,24 +207,33 @@ ptr_deref_may_alias_decl_p (tree ptr, tr
return true;
}
/* Non-aliased variables can not be pointed to. */
if (!may_be_aliased (decl))
return false;
/* If we do not have useful points-to information for this pointer
we cannot disambiguate anything else. */
pi = SSA_NAME_PTR_INFO (ptr);
- if (!pi)
- return true;
+ if (pi && !pt_solution_includes (&pi->pt, decl))
+ return false;
+
+ /* malloc does not alias declarations. */
+ if (TREE_CODE (ptr) == SSA_NAME)
+ {
+ gimple stmt = SSA_NAME_DEF_STMT (ptr);
+ if (is_gimple_call (stmt)
+ && (gimple_call_return_flags (stmt) & ERF_NOALIAS))
+ return false;
+ }
- return pt_solution_includes (&pi->pt, decl);
+ return true;
}
/* Return true if dereferenced PTR1 and PTR2 may alias.
The caller is responsible for applying TBAA to see if accesses
through PTR1 and PTR2 may conflict at all. */
bool
ptr_derefs_may_alias_p (tree ptr1, tree ptr2)
{
struct ptr_info_def *pi1, *pi2;
@@ -293,26 +302,58 @@ ptr_derefs_may_alias_p (tree ptr1, tree
/* We may end up with two empty points-to solutions for two same pointers.
In this case we still want to say both pointers alias, so shortcut
that here. */
if (ptr1 == ptr2)
return true;
/* If we do not have useful points-to information for either pointer
we cannot disambiguate anything else. */
pi1 = SSA_NAME_PTR_INFO (ptr1);
pi2 = SSA_NAME_PTR_INFO (ptr2);
- if (!pi1 || !pi2)
- return true;
-
/* ??? This does not use TBAA to prune decls from the intersection
that not both pointers may access. */
- return pt_solutions_intersect (&pi1->pt, &pi2->pt);
+ if (pi1 && pi2 && !pt_solutions_intersect (&pi1->pt, &pi2->pt))
+ return false;
+
+ /* malloc cannot alias a parameter, a preexisting variable or
+ another malloc. This is a hack, we don't follow the def-use chain,
+ and this should all be part of the points-to analysis anyway. */
+ if (in_gimple_form
+ && TREE_CODE (ptr1) == SSA_NAME && TREE_CODE (ptr2) == SSA_NAME)
+ {
+ gimple stmt1 = SSA_NAME_DEF_STMT (ptr1);
+ gimple stmt2 = SSA_NAME_DEF_STMT (ptr2);
+ int w1;
+ int w2;
+ if (is_gimple_call (stmt1)
+ && (gimple_call_return_flags (stmt1) & ERF_NOALIAS))
+ w1 = 2;
+ else
+#if 1
+ w1 = stmt_dominates_stmt_p (stmt1, stmt2);
+#else
+ w1 = gimple_nop_p (stmt1);
+#endif
+ if (is_gimple_call (stmt2)
+ && (gimple_call_return_flags (stmt2) & ERF_NOALIAS))
+ w2 = 2;
+ else
+#if 1
+ w2 = stmt_dominates_stmt_p (stmt2, stmt1);
+#else
+ w2 = gimple_nop_p (stmt2);
+#endif
+ if (w1 + w2 > 2)
+ return false;
+ }
+
+ return true;
}
/* Return true if dereferencing PTR may alias *REF.
The caller is responsible for applying TBAA to see if PTR
may access *REF at all. */
static bool
ptr_deref_may_alias_ref_p_1 (tree ptr, ao_ref *ref)
{
tree base = ao_ref_base (ref);
