* [PATCH] Handle &__restrict parameters in tree-ssa-structalias.c like DECL_BY_REFERENCE parameters @ 2011-09-23 19:20 Jakub Jelinek 2011-09-24 16:45 ` Richard Guenther 0 siblings, 1 reply; 5+ messages in thread From: Jakub Jelinek @ 2011-09-23 19:20 UTC (permalink / raw) To: Richard Guenther; +Cc: gcc-patches, Jason Merrill Hi! This simple patch improves the f3 function in the testcase below, a parameter with TYPE_RESTRICT REFERENCE_TYPE IMHO can be safely treated like the DECL_BY_REFERENCE case where the source actually didn't contain a reference, but compiler created it anyway. If source contains &__restrict parameter, the parameter again points to a chunk of (for the function global) restrict memory that nothing inside of the function should access otherwise than through this parameter or pointers derived from it. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2011-09-23 Jakub Jelinek <jakub@redhat.com> * tree-ssa-structalias.c (intra_create_variable_infos): Treat TYPE_RESTRICT REFERENCE_TYPE parameters like restricted DECL_BY_REFERENCE parameters. * g++.dg/tree-ssa/restrict2.C: New test. --- gcc/tree-ssa-structalias.c.jj 2011-09-15 12:18:37.000000000 +0200 +++ gcc/tree-ssa-structalias.c 2011-09-23 15:36:23.000000000 +0200 @@ -5589,10 +5589,11 @@ intra_create_variable_infos (void) varinfo_t p; /* For restrict qualified pointers to objects passed by - reference build a real representative for the pointed-to object. */ - if (DECL_BY_REFERENCE (t) - && POINTER_TYPE_P (TREE_TYPE (t)) - && TYPE_RESTRICT (TREE_TYPE (t))) + reference build a real representative for the pointed-to object. + Treat restrict qualified references the same. */ + if (TYPE_RESTRICT (TREE_TYPE (t)) + && ((DECL_BY_REFERENCE (t) && POINTER_TYPE_P (TREE_TYPE (t))) + || TREE_CODE (TREE_TYPE (t)) == REFERENCE_TYPE)) { struct constraint_expr lhsc, rhsc; varinfo_t vi; --- gcc/testsuite/g++.dg/tree-ssa/restrict2.C.jj 2011-09-23 16:11:27.000000000 +0200 +++ gcc/testsuite/g++.dg/tree-ssa/restrict2.C 2011-09-23 16:10:28.000000000 +0200 @@ -0,0 +1,62 @@ +// { dg-do compile } +// { dg-options "-O2 -fdump-tree-optimized" } + +struct S { int *__restrict p; int q; }; +S s; + +int +f1 (S x, S y) +{ + x.p[0] = 1; + y.p[0] = 0; +// { dg-final { scan-tree-dump-times "return 1" 1 "optimized" } } + return x.p[0]; +} + +int +f2 (S x) +{ + x.p[0] = 2; + s.p[0] = 0; +// { dg-final { scan-tree-dump-times "return 2" 1 "optimized" } } + return x.p[0]; +} + +int +f3 (S &__restrict x, S &__restrict y) +{ + x.p[0] = 3; + y.p[0] = 0; +// { dg-final { scan-tree-dump-times "return 3" 1 "optimized" } } + return x.p[0]; +} + +int +f4 (S &x, S &y) +{ + x.p[0] = 4; + y.p[0] = 0; +// { dg-final { scan-tree-dump-times "return 4" 0 "optimized" } } + return x.p[0]; +} + +int +f5 (S *__restrict x, S *__restrict y) +{ + x->p[0] = 5; + y->p[0] = 0; +// We might handle this some day +// { dg-final { scan-tree-dump-times "return 5" 0 "optimized" } } + return x->p[0]; +} + +int +f6 (S *x, S *y) +{ + x->p[0] = 6; + y->p[0] = 0; +// { dg-final { scan-tree-dump-times "return 6" 0 "optimized" } } + return x->p[0]; +} + +// { dg-final { cleanup-tree-dump "optimized" } } Jakub ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Handle &__restrict parameters in tree-ssa-structalias.c like DECL_BY_REFERENCE parameters 2011-09-23 19:20 [PATCH] Handle &__restrict parameters in tree-ssa-structalias.c like DECL_BY_REFERENCE parameters Jakub Jelinek @ 2011-09-24 16:45 ` Richard Guenther 2011-09-24 18:12 ` Jakub Jelinek 2011-09-24 21:15 ` Jason Merrill 0 siblings, 2 replies; 5+ messages in thread From: Richard Guenther @ 2011-09-24 16:45 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Richard Guenther, gcc-patches, Jason Merrill On Fri, Sep 23, 2011 at 7:06 PM, Jakub Jelinek <jakub@redhat.com> wrote: > Hi! > > This simple patch improves the f3 function in the testcase below, > a parameter with TYPE_RESTRICT REFERENCE_TYPE IMHO can be safely treated > like the DECL_BY_REFERENCE case where the source actually didn't contain > a reference, but compiler created it anyway. If source contains &__restrict > parameter, the parameter again points to a chunk of (for the function > global) restrict memory that nothing inside of the function should access > otherwise than through this parameter or pointers derived from it. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > 2011-09-23 Jakub Jelinek <jakub@redhat.com> > > * tree-ssa-structalias.c (intra_create_variable_infos): Treat > TYPE_RESTRICT REFERENCE_TYPE parameters like restricted > DECL_BY_REFERENCE parameters. > > * g++.dg/tree-ssa/restrict2.C: New test. > > --- gcc/tree-ssa-structalias.c.jj 2011-09-15 12:18:37.000000000 +0200 > +++ gcc/tree-ssa-structalias.c 2011-09-23 15:36:23.000000000 +0200 > @@ -5589,10 +5589,11 @@ intra_create_variable_infos (void) > varinfo_t p; > > /* For restrict qualified pointers to objects passed by > - reference build a real representative for the pointed-to object. */ > - if (DECL_BY_REFERENCE (t) > - && POINTER_TYPE_P (TREE_TYPE (t)) > - && TYPE_RESTRICT (TREE_TYPE (t))) > + reference build a real representative for the pointed-to object. > + Treat restrict qualified references the same. */ > + if (TYPE_RESTRICT (TREE_TYPE (t)) > + && ((DECL_BY_REFERENCE (t) && POINTER_TYPE_P (TREE_TYPE (t))) > + || TREE_CODE (TREE_TYPE (t)) == REFERENCE_TYPE)) > { > struct constraint_expr lhsc, rhsc; > varinfo_t vi; I don't see why f4 (s, s) would be invalid. But you would miscompile it. (I'm not sure that a restrict qualified component is properly defined by the C standard - we're just making this extension in a very constrained case to allow Fortran array descriptors to work). Richard. > + > +int > +f2 (S x) > +{ > + x.p[0] = 2; > + s.p[0] = 0; > +// { dg-final { scan-tree-dump-times "return 2" 1 "optimized" } } > + return x.p[0]; > +} > + > +int > +f3 (S &__restrict x, S &__restrict y) > +{ > + x.p[0] = 3; > + y.p[0] = 0; > +// { dg-final { scan-tree-dump-times "return 3" 1 "optimized" } } > + return x.p[0]; > +} > + > +int > +f4 (S &x, S &y) > +{ > + x.p[0] = 4; > + y.p[0] = 0; > +// { dg-final { scan-tree-dump-times "return 4" 0 "optimized" } } > + return x.p[0]; > +} > + > +int > +f5 (S *__restrict x, S *__restrict y) > +{ > + x->p[0] = 5; > + y->p[0] = 0; > +// We might handle this some day > +// { dg-final { scan-tree-dump-times "return 5" 0 "optimized" } } > + return x->p[0]; > +} > + > +int > +f6 (S *x, S *y) > +{ > + x->p[0] = 6; > + y->p[0] = 0; > +// { dg-final { scan-tree-dump-times "return 6" 0 "optimized" } } > + return x->p[0]; > +} > + > +// { dg-final { cleanup-tree-dump "optimized" } } > > Jakub > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Handle &__restrict parameters in tree-ssa-structalias.c like DECL_BY_REFERENCE parameters 2011-09-24 16:45 ` Richard Guenther @ 2011-09-24 18:12 ` Jakub Jelinek 2011-09-25 10:45 ` Richard Guenther 2011-09-24 21:15 ` Jason Merrill 1 sibling, 1 reply; 5+ messages in thread From: Jakub Jelinek @ 2011-09-24 18:12 UTC (permalink / raw) To: Richard Guenther Cc: Richard Guenther, gcc-patches, Jason Merrill, Benjamin Kosnik On Sat, Sep 24, 2011 at 01:26:36PM +0200, Richard Guenther wrote: > > +int > > +f3 (S &__restrict x, S &__restrict y) > > +{ > > + Â x.p[0] = 3; > > + Â y.p[0] = 0; > > +// { dg-final { scan-tree-dump-times "return 3" 1 "optimized" } } > > + Â return x.p[0]; > > +} > > + > > +int > > +f4 (S &x, S &y) > > +{ > > + Â x.p[0] = 4; > > + Â y.p[0] = 0; > > +// { dg-final { scan-tree-dump-times "return 4" 0 "optimized" } } > > + Â return x.p[0]; > > +} > I don't see why > > f4 (s, s) > > would be invalid. But you would miscompile it. f3 (s, s) you mean? I believe it is invalid. For f4 it would be valid and not optimized out. > (I'm not sure that a restrict qualified component is properly defined > by the C standard - we're just making this extension in a very constrained > case to allow Fortran array descriptors to work). Well, C standard doesn't have references, and C++ doesn't have restrict. So it is all about extensions. But what else would be & __restrict for than similar to *__restrict to say that the pointed (resp. referenced) object must not be accessed through other means than the reference or references/pointers derived from it, in the spirit of ISO C99 6.7.3.1. So, before jumping to __restrict fields, consider int a[10], b[10]; int * f8 (S &__restrict x, S &__restrict y) { x.p = a; y.p = b; return x.p; } which we already optimize even before the patch. It is certainly invalid to call f8 (s, s). And the restricted fields, it is a straightforward extension to the restrict definition of ISO C99. We don't use it just for Fortran descriptors, but e.g. std::valarray uses __restrict fields too and has that backed up by the C++ standard requirements. Two different std::valarray objects will have different pointers inside of the structure. My intent currently is to be able to vectorize: #include <valarray> std::valarray<int> f9 (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 f10 (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]; } } In f9 we currently handle it differently than in f10, while IMHO it should be the same thing, a is guaranteed in both cases not to alias b nor c and b is guaranteed not to alias c, furthermore, a._M_data[0] through a._M_data[z-1] is guaranteed not to alias b._M_data[0] through b._M_data[z-1] and c._M_data[0] through c._M_data[z-1] and similarly for b vs. c. The __restrict on the _M_data field in std::valarray is a hint that different std::valarray objects will have different pointers. In f9 we have: size_tD.1850 D.53593; intD.9 * restrict D.53592; intD.9 & D.53591; ... D.53592_7 = MEM[(struct valarrayD.50086 *)aD.50087_6(D) + 8B]; D.53593_42 = D.53456_5 * 4; # PT = nonlocal escaped { D.53660 } (restr) D.53591_43 = D.53592_7 + D.53593_42; ... *D.53591_43 = D.53462_19; and while PTA computes the restricted property here, we unfortunately still don't use it, because D.53591 (which comes from all the inlined wrappers) isn't TYPE_RESTRICT. Shouldn't we propagate that property to either SSA_NAMEs initialized from restricted pointers resp. POINTER_PLUS_EXPRs, or if it is common to all VAR_DECLs underlying such SSA_NAMEs, to the VAR_DECLs? But in f10 we don't get even that far, the a._M_data (which is actually a->_M_data, since a is a (restricted) reference) load is already itself not considered as restricted by PTA. It is nice that we optimize Fortran arrays well, but it would be nice if we did the same for C++ too. Jakub ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Handle &__restrict parameters in tree-ssa-structalias.c like DECL_BY_REFERENCE parameters 2011-09-24 18:12 ` Jakub Jelinek @ 2011-09-25 10:45 ` Richard Guenther 0 siblings, 0 replies; 5+ messages in thread From: Richard Guenther @ 2011-09-25 10:45 UTC (permalink / raw) To: Jakub Jelinek Cc: Richard Guenther, gcc-patches, Jason Merrill, Benjamin Kosnik On Sat, Sep 24, 2011 at 2:15 PM, Jakub Jelinek <jakub@redhat.com> wrote: > On Sat, Sep 24, 2011 at 01:26:36PM +0200, Richard Guenther wrote: >> > +int >> > +f3 (S &__restrict x, S &__restrict y) >> > +{ >> > + x.p[0] = 3; >> > + y.p[0] = 0; >> > +// { dg-final { scan-tree-dump-times "return 3" 1 "optimized" } } >> > + return x.p[0]; >> > +} >> > + >> > +int >> > +f4 (S &x, S &y) >> > +{ >> > + x.p[0] = 4; >> > + y.p[0] = 0; >> > +// { dg-final { scan-tree-dump-times "return 4" 0 "optimized" } } >> > + return x.p[0]; >> > +} > >> I don't see why >> >> f4 (s, s) >> >> would be invalid. But you would miscompile it. > > f3 (s, s) you mean? I believe it is invalid. For f4 it would be valid > and not optimized out. Ah, I misread the dump-test. >> (I'm not sure that a restrict qualified component is properly defined >> by the C standard - we're just making this extension in a very constrained >> case to allow Fortran array descriptors to work). > > Well, C standard doesn't have references, and C++ doesn't have restrict. > So it is all about extensions. > But what else would be & __restrict for than similar to *__restrict > to say that the pointed (resp. referenced) object must not be accessed > through other means than the reference or references/pointers derived from > it, in the spirit of ISO C99 6.7.3.1. > So, before jumping to __restrict fields, consider > int a[10], b[10]; > int * > f8 (S &__restrict x, S &__restrict y) > { > x.p = a; > y.p = b; > return x.p; > } > which we already optimize even before the patch. > It is certainly invalid to call f8 (s, s). > > And the restricted fields, it is a straightforward extension to the restrict > definition of ISO C99. We don't use it just for Fortran descriptors, but > e.g. std::valarray uses __restrict fields too and has that backed up by the > C++ standard requirements. Two different std::valarray objects will have > different pointers inside of the structure. > > My intent currently is to be able to vectorize: > #include <valarray> > > std::valarray<int> > f9 (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 > f10 (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]; > } > } > > In f9 we currently handle it differently than in f10, while IMHO it should > be the same thing, a is guaranteed in both cases not to alias b nor c and b > is guaranteed not to alias c, furthermore, a._M_data[0] through a._M_data[z-1] > is guaranteed not to alias b._M_data[0] through b._M_data[z-1] and c._M_data[0] > through c._M_data[z-1] and similarly for b vs. c. The __restrict on the > _M_data field in std::valarray is a hint that different std::valarray > objects will have different pointers. Ok, I'm just worried that people get bitten by this (given the two existing wrong-code issues we still have with restrict tracking and inlining). But yes, your patch looks safe to me. Maybe we can document how GCC treats restrict qualification of structure members? > > In f9 we have: > size_tD.1850 D.53593; > intD.9 * restrict D.53592; > intD.9 & D.53591; > ... > D.53592_7 = MEM[(struct valarrayD.50086 *)aD.50087_6(D) + 8B]; > D.53593_42 = D.53456_5 * 4; > # PT = nonlocal escaped { D.53660 } (restr) > D.53591_43 = D.53592_7 + D.53593_42; > ... > *D.53591_43 = D.53462_19; > and while PTA computes the restricted property here, we unfortunately still > don't use it, because D.53591 (which comes from all the inlined wrappers) > isn't TYPE_RESTRICT. Yeah, that's extra safety checks because we ignore nonlocal/escaped when applying the restrict tag match. Otherwise the restrict tags are prone to leak into other variables solutions. Maybe finally fixing that two wrong-code restrict bugs will solve this issue though. > Shouldn't we propagate that property to either > SSA_NAMEs initialized from restricted pointers resp. POINTER_PLUS_EXPRs, > or if it is common to all VAR_DECLs underlying such SSA_NAMEs, to the > VAR_DECLs? I'm not sure where to best do that. In principle we shouldn't need to check TYPE_RESTRICT at all, but that requires some thoughts. > But in f10 we don't get even that far, the a._M_data (which is actually > a->_M_data, since a is a (restricted) reference) load is already itself > not considered as restricted by PTA. > > It is nice that we optimize Fortran arrays well, but it would be nice if we > did the same for C++ too. Yeah, I agree. Your patch is ok. Thanks, Richard. > Jakub > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Handle &__restrict parameters in tree-ssa-structalias.c like DECL_BY_REFERENCE parameters 2011-09-24 16:45 ` Richard Guenther 2011-09-24 18:12 ` Jakub Jelinek @ 2011-09-24 21:15 ` Jason Merrill 1 sibling, 0 replies; 5+ messages in thread From: Jason Merrill @ 2011-09-24 21:15 UTC (permalink / raw) To: Richard Guenther; +Cc: Jakub Jelinek, Richard Guenther, gcc-patches On 09/24/2011 07:26 AM, Richard Guenther wrote: > I don't see why > > f4 (s, s) > > would be invalid. But you would miscompile it. >> +int >> +f4 (S&x, S&y) >> +{ >> + x.p[0] = 4; >> + y.p[0] = 0; // { dg-final { scan-tree-dump-times "return 4" 0 "optimized" } } >> + return x.p[0]; >> +} It looks to me like the testcase is testing that we *don't* optimize f4, which I think is the correct result. > +// We might handle this some day > +// { dg-final { scan-tree-dump-times "return 5" 0 "optimized" } } But we could optimize f5, so I don't think we want to test for not optimizing. Better would be to test for the optimization, but mark it as xfail. Jason ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-09-25 8:41 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-09-23 19:20 [PATCH] Handle &__restrict parameters in tree-ssa-structalias.c like DECL_BY_REFERENCE parameters Jakub Jelinek 2011-09-24 16:45 ` Richard Guenther 2011-09-24 18:12 ` Jakub Jelinek 2011-09-25 10:45 ` Richard Guenther 2011-09-24 21:15 ` Jason Merrill
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).