| Message ID | 87o9qdwmkz.fsf@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Add a vect_get_dr_size helper function | expand |
On Thu, Sep 14, 2017 at 1:23 PM, Richard Sandiford <richard.sandiford@linaro.org> wrote: > This patch adds a helper function for getting the number of > bytes accessed by a scalar data reference, which helps when general > modes have a variable size. > > Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64le-linux-gnu. > OK to install? Can you put it into tree-data-ref.h? Ok with that change. Richard. > Richard > > > 2017-09-14 Richard Sandiford <richard.sandiford@linaro.org> > Alan Hayward <alan.hayward@arm.com> > David Sherwood <david.sherwood@arm.com> > > gcc/ > * tree-vect-data-refs.c (vect_get_dr_size): New function. > (vect_update_misalignment_for_peel): Use it. > (vect_enhance_data_refs_alignment): Likewise. > > Index: gcc/tree-vect-data-refs.c > =================================================================== > --- gcc/tree-vect-data-refs.c 2017-09-14 11:27:50.350257085 +0100 > +++ gcc/tree-vect-data-refs.c 2017-09-14 11:29:19.649870912 +0100 > @@ -950,6 +950,13 @@ vect_compute_data_ref_alignment (struct > return true; > } > > +/* Return the size of the value accessed by DR, which is always constant. */ > + > +static unsigned int > +vect_get_dr_size (struct data_reference *dr) > +{ > + return GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (DR_REF (dr)))); > +} > > /* Function vect_update_misalignment_for_peel. > Sets DR's misalignment > @@ -970,8 +977,8 @@ vect_update_misalignment_for_peel (struc > unsigned int i; > vec<dr_p> same_aligned_drs; > struct data_reference *current_dr; > - int dr_size = GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (DR_REF (dr)))); > - int dr_peel_size = GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (DR_REF (dr_peel)))); > + int dr_size = vect_get_dr_size (dr); > + int dr_peel_size = vect_get_dr_size (dr_peel); > stmt_vec_info stmt_info = vinfo_for_stmt (DR_STMT (dr)); > stmt_vec_info peel_stmt_info = vinfo_for_stmt (DR_STMT (dr_peel)); > > @@ -1659,8 +1666,7 @@ vect_enhance_data_refs_alignment (loop_v > > vectype = STMT_VINFO_VECTYPE (stmt_info); > nelements = TYPE_VECTOR_SUBPARTS (vectype); > - mis = DR_MISALIGNMENT (dr) / GET_MODE_SIZE (TYPE_MODE ( > - TREE_TYPE (DR_REF (dr)))); > + mis = DR_MISALIGNMENT (dr) / vect_get_dr_size (dr); > if (DR_MISALIGNMENT (dr) != 0) > npeel_tmp = (negative ? (mis - nelements) > : (nelements - mis)) & (nelements - 1); > @@ -1932,8 +1938,7 @@ vect_enhance_data_refs_alignment (loop_v > updating DR_MISALIGNMENT values. The peeling factor is the > vectorization factor minus the misalignment as an element > count. */ > - mis = DR_MISALIGNMENT (dr0); > - mis /= GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (DR_REF (dr0)))); > + mis = DR_MISALIGNMENT (dr0) / vect_get_dr_size (dr0); > npeel = ((negative ? mis - nelements : nelements - mis) > & (nelements - 1)); > }
Richard Biener <richard.guenther@gmail.com> writes: > On Thu, Sep 14, 2017 at 1:23 PM, Richard Sandiford > <richard.sandiford@linaro.org> wrote: >> This patch adds a helper function for getting the number of >> bytes accessed by a scalar data reference, which helps when general >> modes have a variable size. >> >> Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64le-linux-gnu. >> OK to install? > > Can you put it into tree-data-ref.h? The idea (which I forgot to say) was to collect the uses within the vectoriser into one place so that we can assert in only one place that the reference is constant-sized. A general data_reference can be variable-sized, so the guarantees wouldn't be the same elsewhere. Would putting it in tree-vectorizer.h be OK? Thanks, Richard
On Thu, Sep 14, 2017 at 4:05 PM, Richard Sandiford <richard.sandiford@linaro.org> wrote: > Richard Biener <richard.guenther@gmail.com> writes: >> On Thu, Sep 14, 2017 at 1:23 PM, Richard Sandiford >> <richard.sandiford@linaro.org> wrote: >>> This patch adds a helper function for getting the number of >>> bytes accessed by a scalar data reference, which helps when general >>> modes have a variable size. >>> >>> Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64le-linux-gnu. >>> OK to install? >> >> Can you put it into tree-data-ref.h? > > The idea (which I forgot to say) was to collect the uses within the > vectoriser into one place so that we can assert in only one place > that the reference is constant-sized. > > A general data_reference can be variable-sized, so the guarantees > wouldn't be the same elsewhere. > > Would putting it in tree-vectorizer.h be OK? Maybe name it vect_get_scalar_dr_size then? Ok with that. Richard. > Thanks, > Richard
Richard Biener <richard.guenther@gmail.com> writes: > On Thu, Sep 14, 2017 at 4:05 PM, Richard Sandiford > <richard.sandiford@linaro.org> wrote: >> Richard Biener <richard.guenther@gmail.com> writes: >>> On Thu, Sep 14, 2017 at 1:23 PM, Richard Sandiford >>> <richard.sandiford@linaro.org> wrote: >>>> This patch adds a helper function for getting the number of >>>> bytes accessed by a scalar data reference, which helps when general >>>> modes have a variable size. >>>> >>>> Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64le-linux-gnu. >>>> OK to install? >>> >>> Can you put it into tree-data-ref.h? >> >> The idea (which I forgot to say) was to collect the uses within the >> vectoriser into one place so that we can assert in only one place >> that the reference is constant-sized. >> >> A general data_reference can be variable-sized, so the guarantees >> wouldn't be the same elsewhere. >> >> Would putting it in tree-vectorizer.h be OK? > > Maybe name it vect_get_scalar_dr_size then? > > Ok with that. As discussed on IRC, I tried also using SCALAR_TYPE_MODE instead of TYPE_MODE, to go with the new function name, but it turns out that the mode can be a single-element vector instead of a "true" scalar. This version of the patch sticks with vect_get_scalar_dr_size as the name, but adds a comment to say that "scalar" includes "scalar equivalent". Since SCALAR_TYPE_MODE is too strong an assertion, I went with tree_to_uhwi instead. As also discussed on IRC, it might be possible in current sources to use SCALAR_TYPE_MODE (TREE_TYPE (STMT_VINFO_VECTYPE ...)) instead. But SVE has extending loads and truncating stores, for which the DR size will be different from the vector element size, so if we switch it now, we'd have to switch back to the DR size again later. (Sorry again for labouring such a simple change!) Tested as before. OK to install? Thanks, Richard 2017-09-22 Richard Sandiford <richard.sandiford@linaro.org> Alan Hayward <alan.hayward@arm.com> David Sherwood <david.sherwood@arm.com> gcc/ * tree-vectorizer.h (vect_get_scalar_dr_size): New function. * tree-vect-data-refs.c (vect_update_misalignment_for_peel): Use it. (vect_enhance_data_refs_alignment): Likewise. Index: gcc/tree-vectorizer.h =================================================================== --- gcc/tree-vectorizer.h 2017-09-18 16:49:51.902170781 +0100 +++ gcc/tree-vectorizer.h 2017-09-22 08:58:24.308199570 +0100 @@ -1095,6 +1095,19 @@ vect_get_num_copies (loop_vec_info loop_ / TYPE_VECTOR_SUBPARTS (vectype)); } +/* Return the size of the value accessed by unvectorized data reference DR. + This is only valid once STMT_VINFO_VECTYPE has been calculated for the + associated gimple statement, since that guarantees that DR accesses + either a scalar or a scalar equivalent. ("Scalar equivalent" here + includes things like V1SI, which can be vectorized in the same way + as a plain SI.) */ + +inline unsigned int +vect_get_scalar_dr_size (struct data_reference *dr) +{ + return tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (DR_REF (dr)))); +} + /* Source location */ extern source_location vect_location; Index: gcc/tree-vect-data-refs.c =================================================================== --- gcc/tree-vect-data-refs.c 2017-09-18 16:42:00.553203578 +0100 +++ gcc/tree-vect-data-refs.c 2017-09-22 08:58:24.308199570 +0100 @@ -955,7 +955,6 @@ vect_compute_data_ref_alignment (struct return true; } - /* Function vect_update_misalignment_for_peel. Sets DR's misalignment - to 0 if it has the same alignment as DR_PEEL, @@ -975,8 +974,8 @@ vect_update_misalignment_for_peel (struc unsigned int i; vec<dr_p> same_aligned_drs; struct data_reference *current_dr; - int dr_size = GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (DR_REF (dr)))); - int dr_peel_size = GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (DR_REF (dr_peel)))); + int dr_size = vect_get_scalar_dr_size (dr); + int dr_peel_size = vect_get_scalar_dr_size (dr_peel); stmt_vec_info stmt_info = vinfo_for_stmt (DR_STMT (dr)); stmt_vec_info peel_stmt_info = vinfo_for_stmt (DR_STMT (dr_peel)); @@ -1664,8 +1663,7 @@ vect_enhance_data_refs_alignment (loop_v vectype = STMT_VINFO_VECTYPE (stmt_info); nelements = TYPE_VECTOR_SUBPARTS (vectype); - mis = DR_MISALIGNMENT (dr) / GET_MODE_SIZE (TYPE_MODE ( - TREE_TYPE (DR_REF (dr)))); + mis = DR_MISALIGNMENT (dr) / vect_get_scalar_dr_size (dr); if (DR_MISALIGNMENT (dr) != 0) npeel_tmp = (negative ? (mis - nelements) : (nelements - mis)) & (nelements - 1); @@ -1937,8 +1935,7 @@ vect_enhance_data_refs_alignment (loop_v updating DR_MISALIGNMENT values. The peeling factor is the vectorization factor minus the misalignment as an element count. */ - mis = DR_MISALIGNMENT (dr0); - mis /= GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (DR_REF (dr0)))); + mis = DR_MISALIGNMENT (dr0) / vect_get_scalar_dr_size (dr0); npeel = ((negative ? mis - nelements : nelements - mis) & (nelements - 1)); }
On Fri, Sep 22, 2017 at 10:15 AM, Richard Sandiford <richard.sandiford@linaro.org> wrote: > Richard Biener <richard.guenther@gmail.com> writes: >> On Thu, Sep 14, 2017 at 4:05 PM, Richard Sandiford >> <richard.sandiford@linaro.org> wrote: >>> Richard Biener <richard.guenther@gmail.com> writes: >>>> On Thu, Sep 14, 2017 at 1:23 PM, Richard Sandiford >>>> <richard.sandiford@linaro.org> wrote: >>>>> This patch adds a helper function for getting the number of >>>>> bytes accessed by a scalar data reference, which helps when general >>>>> modes have a variable size. >>>>> >>>>> Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64le-linux-gnu. >>>>> OK to install? >>>> >>>> Can you put it into tree-data-ref.h? >>> >>> The idea (which I forgot to say) was to collect the uses within the >>> vectoriser into one place so that we can assert in only one place >>> that the reference is constant-sized. >>> >>> A general data_reference can be variable-sized, so the guarantees >>> wouldn't be the same elsewhere. >>> >>> Would putting it in tree-vectorizer.h be OK? >> >> Maybe name it vect_get_scalar_dr_size then? >> >> Ok with that. > > As discussed on IRC, I tried also using SCALAR_TYPE_MODE instead of > TYPE_MODE, to go with the new function name, but it turns out that > the mode can be a single-element vector instead of a "true" scalar. > > This version of the patch sticks with vect_get_scalar_dr_size as > the name, but adds a comment to say that "scalar" includes "scalar > equivalent". Since SCALAR_TYPE_MODE is too strong an assertion, > I went with tree_to_uhwi instead. > > As also discussed on IRC, it might be possible in current sources > to use SCALAR_TYPE_MODE (TREE_TYPE (STMT_VINFO_VECTYPE ...)) instead. > But SVE has extending loads and truncating stores, for which the DR size > will be different from the vector element size, so if we switch it now, > we'd have to switch back to the DR size again later. > > (Sorry again for labouring such a simple change!) > > Tested as before. OK to install? Ok. Richard. > Thanks, > Richard > > > 2017-09-22 Richard Sandiford <richard.sandiford@linaro.org> > Alan Hayward <alan.hayward@arm.com> > David Sherwood <david.sherwood@arm.com> > > gcc/ > * tree-vectorizer.h (vect_get_scalar_dr_size): New function. > * tree-vect-data-refs.c (vect_update_misalignment_for_peel): Use it. > (vect_enhance_data_refs_alignment): Likewise. > > Index: gcc/tree-vectorizer.h > =================================================================== > --- gcc/tree-vectorizer.h 2017-09-18 16:49:51.902170781 +0100 > +++ gcc/tree-vectorizer.h 2017-09-22 08:58:24.308199570 +0100 > @@ -1095,6 +1095,19 @@ vect_get_num_copies (loop_vec_info loop_ > / TYPE_VECTOR_SUBPARTS (vectype)); > } > > +/* Return the size of the value accessed by unvectorized data reference DR. > + This is only valid once STMT_VINFO_VECTYPE has been calculated for the > + associated gimple statement, since that guarantees that DR accesses > + either a scalar or a scalar equivalent. ("Scalar equivalent" here > + includes things like V1SI, which can be vectorized in the same way > + as a plain SI.) */ > + > +inline unsigned int > +vect_get_scalar_dr_size (struct data_reference *dr) > +{ > + return tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (DR_REF (dr)))); > +} > + > /* Source location */ > extern source_location vect_location; > > Index: gcc/tree-vect-data-refs.c > =================================================================== > --- gcc/tree-vect-data-refs.c 2017-09-18 16:42:00.553203578 +0100 > +++ gcc/tree-vect-data-refs.c 2017-09-22 08:58:24.308199570 +0100 > @@ -955,7 +955,6 @@ vect_compute_data_ref_alignment (struct > return true; > } > > - > /* Function vect_update_misalignment_for_peel. > Sets DR's misalignment > - to 0 if it has the same alignment as DR_PEEL, > @@ -975,8 +974,8 @@ vect_update_misalignment_for_peel (struc > unsigned int i; > vec<dr_p> same_aligned_drs; > struct data_reference *current_dr; > - int dr_size = GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (DR_REF (dr)))); > - int dr_peel_size = GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (DR_REF (dr_peel)))); > + int dr_size = vect_get_scalar_dr_size (dr); > + int dr_peel_size = vect_get_scalar_dr_size (dr_peel); > stmt_vec_info stmt_info = vinfo_for_stmt (DR_STMT (dr)); > stmt_vec_info peel_stmt_info = vinfo_for_stmt (DR_STMT (dr_peel)); > > @@ -1664,8 +1663,7 @@ vect_enhance_data_refs_alignment (loop_v > > vectype = STMT_VINFO_VECTYPE (stmt_info); > nelements = TYPE_VECTOR_SUBPARTS (vectype); > - mis = DR_MISALIGNMENT (dr) / GET_MODE_SIZE (TYPE_MODE ( > - TREE_TYPE (DR_REF (dr)))); > + mis = DR_MISALIGNMENT (dr) / vect_get_scalar_dr_size (dr); > if (DR_MISALIGNMENT (dr) != 0) > npeel_tmp = (negative ? (mis - nelements) > : (nelements - mis)) & (nelements - 1); > @@ -1937,8 +1935,7 @@ vect_enhance_data_refs_alignment (loop_v > updating DR_MISALIGNMENT values. The peeling factor is the > vectorization factor minus the misalignment as an element > count. */ > - mis = DR_MISALIGNMENT (dr0); > - mis /= GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (DR_REF (dr0)))); > + mis = DR_MISALIGNMENT (dr0) / vect_get_scalar_dr_size (dr0); > npeel = ((negative ? mis - nelements : nelements - mis) > & (nelements - 1)); > }
Index: gcc/tree-vect-data-refs.c =================================================================== --- gcc/tree-vect-data-refs.c 2017-09-14 11:27:50.350257085 +0100 +++ gcc/tree-vect-data-refs.c 2017-09-14 11:29:19.649870912 +0100 @@ -950,6 +950,13 @@ vect_compute_data_ref_alignment (struct return true; } +/* Return the size of the value accessed by DR, which is always constant. */ + +static unsigned int +vect_get_dr_size (struct data_reference *dr) +{ + return GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (DR_REF (dr)))); +} /* Function vect_update_misalignment_for_peel. Sets DR's misalignment @@ -970,8 +977,8 @@ vect_update_misalignment_for_peel (struc unsigned int i; vec<dr_p> same_aligned_drs; struct data_reference *current_dr; - int dr_size = GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (DR_REF (dr)))); - int dr_peel_size = GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (DR_REF (dr_peel)))); + int dr_size = vect_get_dr_size (dr); + int dr_peel_size = vect_get_dr_size (dr_peel); stmt_vec_info stmt_info = vinfo_for_stmt (DR_STMT (dr)); stmt_vec_info peel_stmt_info = vinfo_for_stmt (DR_STMT (dr_peel)); @@ -1659,8 +1666,7 @@ vect_enhance_data_refs_alignment (loop_v vectype = STMT_VINFO_VECTYPE (stmt_info); nelements = TYPE_VECTOR_SUBPARTS (vectype); - mis = DR_MISALIGNMENT (dr) / GET_MODE_SIZE (TYPE_MODE ( - TREE_TYPE (DR_REF (dr)))); + mis = DR_MISALIGNMENT (dr) / vect_get_dr_size (dr); if (DR_MISALIGNMENT (dr) != 0) npeel_tmp = (negative ? (mis - nelements) : (nelements - mis)) & (nelements - 1); @@ -1932,8 +1938,7 @@ vect_enhance_data_refs_alignment (loop_v updating DR_MISALIGNMENT values. The peeling factor is the vectorization factor minus the misalignment as an element count. */ - mis = DR_MISALIGNMENT (dr0); - mis /= GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (DR_REF (dr0)))); + mis = DR_MISALIGNMENT (dr0) / vect_get_dr_size (dr0); npeel = ((negative ? mis - nelements : nelements - mis) & (nelements - 1)); }