This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: free is a killer


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.glisse@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

Attachment: p23
Description: memcpy patch

Attachment: p24
Description: malloc hack


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]