On Mon, Oct 28, 2013 at 11:05 PM, Marc Glisse <marc.gli...@inria.fr> wrote: > 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.
The VA_END case looks ok (though == equality testing is a bit too conservative, the SSA_NAME case of the builtins handling looks wrong indeed. > 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)) No, the type of 'ptr' are not in any way relevant. Richard. > 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