| Message ID | 20110926144651.GF2687@tyan-ft48-01.lab.bos.redhat.com |
|---|---|
| State | New |
| Headers | show |
On Mon, 26 Sep 2011, Jakub Jelinek wrote: > Hi! > > As discussed in the PR, we currently don't vectorize > #include <valarray> > > std::valarray<int> > f1 (std::valarray<int> a, std::valarray<int> b, std::valarray<int> c, int z) > { > int i; > for (i = 0; i < z; i++) > { > a[i] = b[i] + c[i]; > a[i] += b[i] * c[i]; > } > return a; > } > > void > f2 (std::valarray<int> &__restrict a, std::valarray<int> &__restrict b, std::valarray<int> &__restrict c, int z) > { > int i; > for (i = 0; i < z; i++) > { > a[i] = b[i] + c[i]; > a[i] += b[i] * c[i]; > } > } > > because while the ptr loaded from _M_data is TYPE_RESTRICT, the > reference computed as that pointer plus i * 4 that results from inling > is not TYPE_RESTRICT. > > Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, > ok for trunk? Let's see what kind of fallout we get ;) For example, if the following is valid C code I expect we will vectorize the second loop (disambiguating p[i] and q[i]) bogously: void foo (int *p) { int * __restrict p1 = p; int * __restrict p2 = p + 32; int *q; int i; for (i = 0; i < 32; ++i) p1[i] = p2[i]; p = p1; q = p2 - 31; for (i = 0; i < 32; ++i) p[i] = q[i]; } because p and q base on different restrict qualified pointers (p1 and p2 respective). At the moment we are safe from this because of the TYPE_RESTRICT checks. Any opinion on the above? Is it valid to base non-restrict pointers on restrict ones? It would be sort-of weird at least, but at least I don't think the first loop use is bogus (even though the pointed-to objects are the same). Thanks, Richard. > 2011-09-26 Jakub Jelinek <jakub@redhat.com> > > PR tree-optimization/50522 > * tree-ssa-alias.c (ptr_deref_may_alias_decl_p): Don't test > TYPE_RESTRICT. > (ptr_derefs_may_alias_p): Call pt_solutions_same_restrict_base > unconditionally. > > --- gcc/tree-ssa-alias.c.jj 2011-09-15 12:18:37.000000000 +0200 > +++ gcc/tree-ssa-alias.c 2011-09-26 13:49:24.000000000 +0200 > @@ -223,7 +223,6 @@ ptr_deref_may_alias_decl_p (tree ptr, tr > pointer and that pointers points-to set doesn't contain this decl > then they can't alias. */ > if (DECL_RESTRICTED_P (decl) > - && TYPE_RESTRICT (TREE_TYPE (ptr)) > && pi->pt.vars_contains_restrict) > return bitmap_bit_p (pi->pt.vars, DECL_PT_UID (decl)); > > @@ -319,9 +318,7 @@ ptr_derefs_may_alias_p (tree ptr1, tree > > /* If both pointers are restrict-qualified try to disambiguate > with restrict information. */ > - if (TYPE_RESTRICT (TREE_TYPE (ptr1)) > - && TYPE_RESTRICT (TREE_TYPE (ptr2)) > - && !pt_solutions_same_restrict_base (&pi1->pt, &pi2->pt)) > + if (!pt_solutions_same_restrict_base (&pi1->pt, &pi2->pt)) > return false; > > /* ??? This does not use TBAA to prune decls from the intersection > > Jakub > >
Hi! Adding Joseph and Jason to CC. On Mon, Sep 26, 2011 at 04:56:20PM +0200, Richard Guenther wrote: > Let's see what kind of fallout we get ;) For example, if the > following is valid C code I expect we will vectorize the second > loop (disambiguating p[i] and q[i]) bogously: > > void foo (int *p) > { > int * __restrict p1 = p; > int * __restrict p2 = p + 32; > int *q; > int i; > for (i = 0; i < 32; ++i) > p1[i] = p2[i]; > p = p1; > q = p2 - 31; > for (i = 0; i < 32; ++i) > p[i] = q[i]; > } > > because p and q base on different restrict qualified pointers > (p1 and p2 respective). At the moment we are safe from this > because of the TYPE_RESTRICT checks. > > Any opinion on the above? Is it valid to base non-restrict > pointers on restrict ones? It would be sort-of weird at least, > but at least I don't think the first loop use is bogus (even > though the pointed-to objects are the same). If the last loop was for (i = 0; i < 32; i++) q[i] = p[i]; then I believe the above would be clearly invalid C99, because an object X (say incoming p[4]) would be modified in the same block using a pointer based on p1 and using a pointer not based on p1 (q), which would violate the requirements that if the object is modified through lvalue whose address is based on p1, all modifications to B in that block should be done through lvalues whose address is based on p1. In the above testcase all modifications are made through lvalues whose addresses are p1 based though, so it is less clear. Joseph? Anyway, GCC currently makes p1 and p2 use the same restrict base actually, and does vectorize both loops before the patch, after the patch as well as with -D__restrict= (the vectorization of the second loop is guarded with non-overlap check). It looks like a gimplification is wrong: int * restrict p1 = p; int * restrict p2 = p + 128; is gimplified into: p1 = (int * restrict) p; p.0 = (int * restrict) p; p2 = p.0 + 128; which IMHO is incorrect, I'd say p.0 shouldn't be int * restrict, but plain int *, only the final value should be TYPE_RESTRICT. Jakub
On Mon, 26 Sep 2011, Jakub Jelinek wrote: > Hi! > > Adding Joseph and Jason to CC. > > On Mon, Sep 26, 2011 at 04:56:20PM +0200, Richard Guenther wrote: > > Let's see what kind of fallout we get ;) For example, if the > > following is valid C code I expect we will vectorize the second > > loop (disambiguating p[i] and q[i]) bogously: > > > > void foo (int *p) > > { > > int * __restrict p1 = p; > > int * __restrict p2 = p + 32; > > int *q; > > int i; > > for (i = 0; i < 32; ++i) > > p1[i] = p2[i]; > > p = p1; > > q = p2 - 31; > > for (i = 0; i < 32; ++i) > > p[i] = q[i]; > > } > > > > because p and q base on different restrict qualified pointers > > (p1 and p2 respective). At the moment we are safe from this > > because of the TYPE_RESTRICT checks. > > > > Any opinion on the above? Is it valid to base non-restrict > > pointers on restrict ones? It would be sort-of weird at least, > > but at least I don't think the first loop use is bogus (even > > though the pointed-to objects are the same). > > If the last loop was > for (i = 0; i < 32; i++) > q[i] = p[i]; > then I believe the above would be clearly invalid C99, because > an object X (say incoming p[4]) would be modified in the same block That can be fixed by adding some {}s and using a different decl for the 2nd p/q. > using a pointer based on p1 and using a pointer not based on p1 > (q), which would violate the requirements that if the object is > modified through lvalue whose address is based on p1, all modifications > to B in that block should be done through lvalues whose address is > based on p1. In the above testcase all modifications are made through > lvalues whose addresses are p1 based though, so it is less clear. > Joseph? > > Anyway, GCC currently makes p1 and p2 use the same restrict > base actually, and does vectorize both loops before the patch, > after the patch as well as with -D__restrict= > (the vectorization of the second loop is guarded with non-overlap > check). It looks like a gimplification is wrong: > int * restrict p1 = p; > int * restrict p2 = p + 128; > is gimplified into: > p1 = (int * restrict) p; > p.0 = (int * restrict) p; > p2 = p.0 + 128; > which IMHO is incorrect, I'd say p.0 shouldn't be int * restrict, > but plain int *, only the final value should be TYPE_RESTRICT. Yeah, but maybe it's already fold that transforms (int * restrict)(p + 128) to (int * restrict)p + 128. I'm pretty sure it is. Richard.
On Mon, Sep 26, 2011 at 05:50:40PM +0200, Jakub Jelinek wrote: > On Mon, Sep 26, 2011 at 04:56:20PM +0200, Richard Guenther wrote: > > Let's see what kind of fallout we get ;) For example, if the > > following is valid C code I expect we will vectorize the second > > loop (disambiguating p[i] and q[i]) bogously: > > > > void foo (int *p) > > { > > int * __restrict p1 = p; > > int * __restrict p2 = p + 32; > > int *q; > > int i; > > for (i = 0; i < 32; ++i) > > p1[i] = p2[i]; > > p = p1; > > q = p2 - 31; > > for (i = 0; i < 32; ++i) > > p[i] = q[i]; > > } > > > > because p and q base on different restrict qualified pointers > > (p1 and p2 respective). At the moment we are safe from this > > because of the TYPE_RESTRICT checks. > > > > Any opinion on the above? Is it valid to base non-restrict > > pointers on restrict ones? It would be sort-of weird at least, > > but at least I don't think the first loop use is bogus (even > > though the pointed-to objects are the same). > > based on p1. In the above testcase all modifications are made through > lvalues whose addresses are p1 based though, so it is less clear. I think I've missed the by any means there. For int a[64]; foo (a); In the above function with this e.g. a[4] object is modified during the extecution of the foo block B, thus the 6.7.3.1/4 rules should apply. And within that block a[4] is accessed through lvalues whose address is p1 based as well as through lvalues whose address is not p1 based (is p2 based), therefore it is invalid. Jakub
On Mon, Sep 26, 2011 at 06:01:45PM +0200, Jakub Jelinek wrote: > In the above function with this e.g. a[4] object is modified during > the extecution of the foo block B, thus the 6.7.3.1/4 rules should apply. > And within that block a[4] is accessed through lvalues whose address > is p1 based as well as through lvalues whose address is not p1 based > (is p2 based), therefore it is invalid. I think better testcase would be: void foo (int *x, int y) { int * __restrict p1 = x; #ifdef WORKAROUND int *p3 = x + y; int * __restrict p2 = p3; #else int * __restrict p2 = x + y; #endif int *p; int *q; int i; for (i = 0; i < 32; ++i) p1[i] = p2[i]; p = p1; q = p2 - 31; for (i = 0; i < 32; ++i) p[i] = q[i]; } which would be invalid to call with foo (a, 32); given the above, but it isn't obvious to the compiler what value y has. With -DWORKAROUND the PT decls in (restr) look correct, without that not (supposedly because of the folding of the initializer), still, the vectorizer together with the alias oracle don't figure out they can omit the non-overlap tests before both loops. Jakub
On Mon, 26 Sep 2011, Jakub Jelinek wrote: > Hi! > > Adding Joseph and Jason to CC. > > On Mon, Sep 26, 2011 at 04:56:20PM +0200, Richard Guenther wrote: > > Let's see what kind of fallout we get ;) For example, if the > > following is valid C code I expect we will vectorize the second > > loop (disambiguating p[i] and q[i]) bogously: > > > > void foo (int *p) > > { > > int * __restrict p1 = p; > > int * __restrict p2 = p + 32; > > int *q; > > int i; > > for (i = 0; i < 32; ++i) > > p1[i] = p2[i]; > > p = p1; > > q = p2 - 31; > > for (i = 0; i < 32; ++i) > > p[i] = q[i]; > > } > > > > because p and q base on different restrict qualified pointers > > (p1 and p2 respective). At the moment we are safe from this > > because of the TYPE_RESTRICT checks. > > > > Any opinion on the above? Is it valid to base non-restrict > > pointers on restrict ones? It would be sort-of weird at least, > > but at least I don't think the first loop use is bogus (even > > though the pointed-to objects are the same). > > If the last loop was > for (i = 0; i < 32; i++) > q[i] = p[i]; > then I believe the above would be clearly invalid C99, because > an object X (say incoming p[4]) would be modified in the same block > using a pointer based on p1 and using a pointer not based on p1 > (q), which would violate the requirements that if the object is > modified through lvalue whose address is based on p1, all modifications > to B in that block should be done through lvalues whose address is > based on p1. In the above testcase all modifications are made through > lvalues whose addresses are p1 based though, so it is less clear. > Joseph? If an object that is accessed by a restricted pointer is also modified, then all accesses (not just all modifications) must be through pointers based on the restricted pointer. So in the original loop with p[i] = q[i], q[i] for i from 0 to 30 is an object that was previously modified through p1 and is now being accessed through p2. So this code appears invalid to me.
On Fri, 30 Sep 2011, Joseph S. Myers wrote: > On Mon, 26 Sep 2011, Jakub Jelinek wrote: > > > Hi! > > > > Adding Joseph and Jason to CC. > > > > On Mon, Sep 26, 2011 at 04:56:20PM +0200, Richard Guenther wrote: > > > Let's see what kind of fallout we get ;) For example, if the > > > following is valid C code I expect we will vectorize the second > > > loop (disambiguating p[i] and q[i]) bogously: > > > > > > void foo (int *p) > > > { > > > int * __restrict p1 = p; > > > int * __restrict p2 = p + 32; > > > int *q; > > > int i; > > > for (i = 0; i < 32; ++i) > > > p1[i] = p2[i]; > > > p = p1; > > > q = p2 - 31; > > > for (i = 0; i < 32; ++i) > > > p[i] = q[i]; > > > } > > > > > > because p and q base on different restrict qualified pointers > > > (p1 and p2 respective). At the moment we are safe from this > > > because of the TYPE_RESTRICT checks. > > > > > > Any opinion on the above? Is it valid to base non-restrict > > > pointers on restrict ones? It would be sort-of weird at least, > > > but at least I don't think the first loop use is bogus (even > > > though the pointed-to objects are the same). > > > > If the last loop was > > for (i = 0; i < 32; i++) > > q[i] = p[i]; > > then I believe the above would be clearly invalid C99, because > > an object X (say incoming p[4]) would be modified in the same block > > using a pointer based on p1 and using a pointer not based on p1 > > (q), which would violate the requirements that if the object is > > modified through lvalue whose address is based on p1, all modifications > > to B in that block should be done through lvalues whose address is > > based on p1. In the above testcase all modifications are made through > > lvalues whose addresses are p1 based though, so it is less clear. > > Joseph? > > If an object that is accessed by a restricted pointer is also modified, > then all accesses (not just all modifications) must be through pointers > based on the restricted pointer. So in the original loop with p[i] = > q[i], q[i] for i from 0 to 30 is an object that was previously modified > through p1 and is now being accessed through p2. So this code appears > invalid to me. In the above first loop the restrict pointers p1 and p2 access distinct object pieces. The second loop uses non-restrict qualified pointers p and q (that are based on the restrict variants p1 and p2 though) to access overlapping pieces. Is the second loop invalid because p and q are based on p1 and p2 even though they are not restrict qualified? Or is the loop ok because p and q are not restrict qualified? Thanks, Richard.
On Tue, Oct 04, 2011 at 11:01:27AM +0200, Richard Guenther wrote: > > > > void foo (int *p) > > > > { > > > > int * __restrict p1 = p; > > > > int * __restrict p2 = p + 32; > > > > int *q; > > > > int i; > > > > for (i = 0; i < 32; ++i) > > > > p1[i] = p2[i]; > > > > p = p1; > > > > q = p2 - 31; > > > > for (i = 0; i < 32; ++i) > > > > p[i] = q[i]; > > > > } > > > > > In the above first loop the restrict pointers p1 and p2 access > distinct object pieces. The second loop uses non-restrict qualified > pointers p and q (that are based on the restrict variants p1 and p2 > though) to access overlapping pieces. Is the second loop invalid > because p and q are based on p1 and p2 even though they are not > restrict qualified? IMHO yes. The standard doesn't seem to talk about the accesses being done through the restricted pointer, but about accesses that are based on the restricted pointer, and as soon as you access in the associated block (here the foo function) some object through an lvalue whose address is based on some restricted pointer and the value is modified by any means, then all accesses to that object need to be done through something based on that restricted pointer. Jakub
On Tue, 4 Oct 2011, Jakub Jelinek wrote: > On Tue, Oct 04, 2011 at 11:01:27AM +0200, Richard Guenther wrote: > > > > > void foo (int *p) > > > > > { > > > > > int * __restrict p1 = p; > > > > > int * __restrict p2 = p + 32; > > > > > int *q; > > > > > int i; > > > > > for (i = 0; i < 32; ++i) > > > > > p1[i] = p2[i]; > > > > > p = p1; > > > > > q = p2 - 31; > > > > > for (i = 0; i < 32; ++i) > > > > > p[i] = q[i]; > > > > > } > > > > > > > > In the above first loop the restrict pointers p1 and p2 access > > distinct object pieces. The second loop uses non-restrict qualified > > pointers p and q (that are based on the restrict variants p1 and p2 > > though) to access overlapping pieces. Is the second loop invalid > > because p and q are based on p1 and p2 even though they are not > > restrict qualified? > > IMHO yes. The standard doesn't seem to talk about the accesses being done > through the restricted pointer, but about accesses that are based on > the restricted pointer, and as soon as you access in the associated block > (here the foo function) some object through an lvalue whose address is > based on some restricted pointer and the value is modified by any means, > then all accesses to that object need to be done through something > based on that restricted pointer. So when I change the above to /*p = p;*/ q = (p + 32) - 31; then the code will be valid? When I obfuscate that enough I can get GCC CSEing p + 32 and thus effectively q will look like it is based on p2. Richard.
On Tue, Oct 04, 2011 at 11:55:17AM +0200, Richard Guenther wrote: > > On Tue, Oct 04, 2011 at 11:01:27AM +0200, Richard Guenther wrote: > > > > > > void foo (int *p) > > > > > > { > > > > > > int * __restrict p1 = p; > > > > > > int * __restrict p2 = p + 32; > > > > > > int *q; > > > > > > int i; > > > > > > for (i = 0; i < 32; ++i) > > > > > > p1[i] = p2[i]; > > > > > > p = p1; > > > > > > q = p2 - 31; > > > > > > for (i = 0; i < 32; ++i) > > > > > > p[i] = q[i]; > > > > > > } > > > > > > > > > > > In the above first loop the restrict pointers p1 and p2 access > > > distinct object pieces. The second loop uses non-restrict qualified > > > pointers p and q (that are based on the restrict variants p1 and p2 > > > though) to access overlapping pieces. Is the second loop invalid > > > because p and q are based on p1 and p2 even though they are not > > > restrict qualified? > > > > IMHO yes. The standard doesn't seem to talk about the accesses being done > > through the restricted pointer, but about accesses that are based on > > the restricted pointer, and as soon as you access in the associated block > > (here the foo function) some object through an lvalue whose address is > > based on some restricted pointer and the value is modified by any means, > > then all accesses to that object need to be done through something > > based on that restricted pointer. > > So when I change the above to > > /*p = p;*/ > q = (p + 32) - 31; void foo (int *p) { int * __restrict p1 = p; int * __restrict p2 = p + 32; int *q; int i; for (i = 0; i < 32; ++i) p1[i] = p2[i]; q = (p + 32) - 31; for (i = 0; i < 32; ++i) p[i] = q[i]; } > then the code will be valid? When I obfuscate that enough I > can get GCC CSEing p + 32 and thus effectively q will look > like it is based on p2. The above is still invalid. p[0] through p[31] is modified and is accessed both through lvalue whose address is based on p1 (p1[i]) and through lvalues whose address is not based on p1 (p[i] and q[i] (the latter only for p[0] through p[30])). If you take the first loop out, it would be valid though. Jakub
On Tue, 4 Oct 2011, Jakub Jelinek wrote: > On Tue, Oct 04, 2011 at 11:55:17AM +0200, Richard Guenther wrote: > > > On Tue, Oct 04, 2011 at 11:01:27AM +0200, Richard Guenther wrote: > > > > > > > void foo (int *p) > > > > > > > { > > > > > > > int * __restrict p1 = p; > > > > > > > int * __restrict p2 = p + 32; > > > > > > > int *q; > > > > > > > int i; > > > > > > > for (i = 0; i < 32; ++i) > > > > > > > p1[i] = p2[i]; > > > > > > > p = p1; > > > > > > > q = p2 - 31; > > > > > > > for (i = 0; i < 32; ++i) > > > > > > > p[i] = q[i]; > > > > > > > } > > > > > > > > > > > > > > In the above first loop the restrict pointers p1 and p2 access > > > > distinct object pieces. The second loop uses non-restrict qualified > > > > pointers p and q (that are based on the restrict variants p1 and p2 > > > > though) to access overlapping pieces. Is the second loop invalid > > > > because p and q are based on p1 and p2 even though they are not > > > > restrict qualified? > > > > > > IMHO yes. The standard doesn't seem to talk about the accesses being done > > > through the restricted pointer, but about accesses that are based on > > > the restricted pointer, and as soon as you access in the associated block > > > (here the foo function) some object through an lvalue whose address is > > > based on some restricted pointer and the value is modified by any means, > > > then all accesses to that object need to be done through something > > > based on that restricted pointer. > > > > So when I change the above to > > > > /*p = p;*/ > > q = (p + 32) - 31; > > void foo (int *p) > { > int * __restrict p1 = p; > int * __restrict p2 = p + 32; > int *q; > int i; > for (i = 0; i < 32; ++i) > p1[i] = p2[i]; > q = (p + 32) - 31; > for (i = 0; i < 32; ++i) > p[i] = q[i]; > } > > > then the code will be valid? When I obfuscate that enough I > > can get GCC CSEing p + 32 and thus effectively q will look > > like it is based on p2. > > The above is still invalid. p[0] through p[31] is modified and > is accessed both through lvalue whose address is based on p1 (p1[i]) > and through lvalues whose address is not based on p1 (p[i] and > q[i] (the latter only for p[0] through p[30])). If you take > the first loop out, it would be valid though. So int *x; > void foo (int *p) > { > int * __restrict p1 = p; > int * __restrict p2 = p + 32; > int *q; > int i; x = p2; > q = p + 32; q = q - 31; > for (i = 0; i < 32; ++i) > p[i] = q[i]; > } would be valid and we'd rely on CSE not to replace q = p + 32 with q = p2 (ignoring the fact that for a miscompile we need similar tricks for p1). It doesn't do that at the moment because we fold int * __restrict p2 = p + 32 to ((int * __restrict)p) + 32 and thus see p.0_4 = (int * restrict) p_2(D); p2_5 = p.0_4 + 128; vs. q_6 = p_2(D) + 128; but you are going to change that ;) Richard.
On Tue, Oct 04, 2011 at 12:17:30PM +0200, Richard Guenther wrote: > int *x; > > > void foo (int *p) > > { > > int * __restrict p1 = p; > > int * __restrict p2 = p + 32; > > int *q; > > int i; > x = p2; > > q = p + 32; > q = q - 31; > > for (i = 0; i < 32; ++i) > > p[i] = q[i]; > > } Yes, this is valid and so is a modified version of the earlier testcase where all accesses in the first loop are biased (bar below, assuming y > 32 or y <= -32). int *x; void foo (int *p) { int *__restrict p1 = p; int *__restrict p2 = p + 32; int *q; int i; x = p2; q = p + 32; q = q - 31; for (i = 0; i < 32; ++i) p[i] = q[i]; } void bar (int *p, int y) { int *__restrict p1 = p; int *__restrict p2 = p + 32; int *q; int i; for (i = 0; i < 32; ++i) p1[i + y] = p2[i + y]; q = (p + 32) - 31; for (i = 0; i < 32; ++i) p[i] = q[i]; } > > would be valid and we'd rely on CSE not to replace q = p + 32 > with q = p2 (ignoring the fact that for a miscompile we need > similar tricks for p1). It doesn't do that at the moment > because we fold int * __restrict p2 = p + 32 to > ((int * __restrict)p) + 32 and thus see > > p.0_4 = (int * restrict) p_2(D); > p2_5 = p.0_4 + 128; > > vs. > > q_6 = p_2(D) + 128; > > but you are going to change that ;) But even with the "Restrict fixes" patch I've just checked in and with the TYPE_RESTRICT check removal patch I don't see anything wrong in the IL, the only thing that is PT (restr) is the stmt computing p2, which is just stored into x and nothing else, and in the second function only the first loop. Jakub
On Tue, 4 Oct 2011, Jakub Jelinek wrote: > On Tue, Oct 04, 2011 at 12:17:30PM +0200, Richard Guenther wrote: > > int *x; > > > > > void foo (int *p) > > > { > > > int * __restrict p1 = p; > > > int * __restrict p2 = p + 32; > > > int *q; > > > int i; > > x = p2; > > > q = p + 32; > > q = q - 31; > > > for (i = 0; i < 32; ++i) > > > p[i] = q[i]; > > > } > > Yes, this is valid and so is a modified version of the earlier > testcase where all accesses in the first loop are biased > (bar below, assuming y > 32 or y <= -32). > > int *x; > > void > foo (int *p) > { > int *__restrict p1 = p; > int *__restrict p2 = p + 32; > int *q; > int i; > x = p2; > q = p + 32; > q = q - 31; > for (i = 0; i < 32; ++i) > p[i] = q[i]; > } > > void > bar (int *p, int y) > { > int *__restrict p1 = p; > int *__restrict p2 = p + 32; > int *q; > int i; > for (i = 0; i < 32; ++i) > p1[i + y] = p2[i + y]; > q = (p + 32) - 31; > for (i = 0; i < 32; ++i) > p[i] = q[i]; > } > > > > > would be valid and we'd rely on CSE not to replace q = p + 32 > > with q = p2 (ignoring the fact that for a miscompile we need > > similar tricks for p1). It doesn't do that at the moment > > because we fold int * __restrict p2 = p + 32 to > > ((int * __restrict)p) + 32 and thus see > > > > p.0_4 = (int * restrict) p_2(D); > > p2_5 = p.0_4 + 128; > > > > vs. > > > > q_6 = p_2(D) + 128; > > > > but you are going to change that ;) > > But even with the "Restrict fixes" patch I've just checked in > and with the TYPE_RESTRICT check removal patch I don't see anything > wrong in the IL, the only thing that is PT (restr) is the stmt > computing p2, which is just stored into x and nothing else, and > in the second function only the first loop. But that's by pure luck and not by design, no? Richard.
On Tue, 4 Oct 2011, Richard Guenther wrote: > On Tue, 4 Oct 2011, Jakub Jelinek wrote: > > > On Tue, Oct 04, 2011 at 12:17:30PM +0200, Richard Guenther wrote: > > > int *x; > > > > > > > void foo (int *p) > > > > { > > > > int * __restrict p1 = p; > > > > int * __restrict p2 = p + 32; > > > > int *q; > > > > int i; > > > x = p2; > > > > q = p + 32; > > > q = q - 31; > > > > for (i = 0; i < 32; ++i) > > > > p[i] = q[i]; > > > > } > > > > Yes, this is valid and so is a modified version of the earlier > > testcase where all accesses in the first loop are biased > > (bar below, assuming y > 32 or y <= -32). > > > > int *x; > > > > void > > foo (int *p) > > { > > int *__restrict p1 = p; > > int *__restrict p2 = p + 32; > > int *q; > > int i; > > x = p2; > > q = p + 32; > > q = q - 31; > > for (i = 0; i < 32; ++i) > > p[i] = q[i]; > > } > > > > void > > bar (int *p, int y) > > { > > int *__restrict p1 = p; > > int *__restrict p2 = p + 32; > > int *q; > > int i; > > for (i = 0; i < 32; ++i) > > p1[i + y] = p2[i + y]; > > q = (p + 32) - 31; > > for (i = 0; i < 32; ++i) > > p[i] = q[i]; > > } > > > > > > > > would be valid and we'd rely on CSE not to replace q = p + 32 > > > with q = p2 (ignoring the fact that for a miscompile we need > > > similar tricks for p1). It doesn't do that at the moment > > > because we fold int * __restrict p2 = p + 32 to > > > ((int * __restrict)p) + 32 and thus see > > > > > > p.0_4 = (int * restrict) p_2(D); > > > p2_5 = p.0_4 + 128; > > > > > > vs. > > > > > > q_6 = p_2(D) + 128; > > > > > > but you are going to change that ;) > > > > But even with the "Restrict fixes" patch I've just checked in > > and with the TYPE_RESTRICT check removal patch I don't see anything > > wrong in the IL, the only thing that is PT (restr) is the stmt > > computing p2, which is just stored into x and nothing else, and > > in the second function only the first loop. > > But that's by pure luck and not by design, no? Well, updating my tree and playing with it a bit the above case looks safe. So, I think the patch removing the TYPE_RESTRICT checks is ok. We can revert it again when issues show up. Thanks, Richard.
--- gcc/tree-ssa-alias.c.jj 2011-09-15 12:18:37.000000000 +0200 +++ gcc/tree-ssa-alias.c 2011-09-26 13:49:24.000000000 +0200 @@ -223,7 +223,6 @@ ptr_deref_may_alias_decl_p (tree ptr, tr pointer and that pointers points-to set doesn't contain this decl then they can't alias. */ if (DECL_RESTRICTED_P (decl) - && TYPE_RESTRICT (TREE_TYPE (ptr)) && pi->pt.vars_contains_restrict) return bitmap_bit_p (pi->pt.vars, DECL_PT_UID (decl)); @@ -319,9 +318,7 @@ ptr_derefs_may_alias_p (tree ptr1, tree /* If both pointers are restrict-qualified try to disambiguate with restrict information. */ - if (TYPE_RESTRICT (TREE_TYPE (ptr1)) - && TYPE_RESTRICT (TREE_TYPE (ptr2)) - && !pt_solutions_same_restrict_base (&pi1->pt, &pi2->pt)) + if (!pt_solutions_same_restrict_base (&pi1->pt, &pi2->pt)) return false; /* ??? This does not use TBAA to prune decls from the intersection
Hi! As discussed in the PR, we currently don't vectorize #include <valarray> std::valarray<int> f1 (std::valarray<int> a, std::valarray<int> b, std::valarray<int> c, int z) { int i; for (i = 0; i < z; i++) { a[i] = b[i] + c[i]; a[i] += b[i] * c[i]; } return a; } void f2 (std::valarray<int> &__restrict a, std::valarray<int> &__restrict b, std::valarray<int> &__restrict c, int z) { int i; for (i = 0; i < z; i++) { a[i] = b[i] + c[i]; a[i] += b[i] * c[i]; } } because while the ptr loaded from _M_data is TYPE_RESTRICT, the reference computed as that pointer plus i * 4 that results from inling is not TYPE_RESTRICT. Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2011-09-26 Jakub Jelinek <jakub@redhat.com> PR tree-optimization/50522 * tree-ssa-alias.c (ptr_deref_may_alias_decl_p): Don't test TYPE_RESTRICT. (ptr_derefs_may_alias_p): Call pt_solutions_same_restrict_base unconditionally. Jakub