From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 101603 invoked by alias); 18 Sep 2015 08:36:31 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 101587 invoked by uid 89); 18 Sep 2015 08:36:30 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.1 required=5.0 tests=AWL,BAYES_00,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx2.suse.de Received: from mx2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (CAMELLIA256-SHA encrypted) ESMTPS; Fri, 18 Sep 2015 08:36:28 +0000 Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 88E17AD23; Fri, 18 Sep 2015 08:36:24 +0000 (UTC) Date: Fri, 18 Sep 2015 08:36:00 -0000 From: Richard Biener To: Alan Lawrence cc: gcc-patches@gcc.gnu.org, mjambor@suse.cz, christophe.lyon@linaro.org, law@redhat.com Subject: Re: [PATCH 2/5] completely_scalarize arrays as well as records. In-Reply-To: <1442509901-31777-1-git-send-email-alan.lawrence@arm.com> Message-ID: References: <1442509901-31777-1-git-send-email-alan.lawrence@arm.com> User-Agent: Alpine 2.11 (LSU 23 2013-08-11) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-SW-Source: 2015-09/txt/msg01373.txt.bz2 On Thu, 17 Sep 2015, Alan Lawrence wrote: > On 15/09/15 08:43, Richard Biener wrote: > > > > Sorry for chiming in so late... > > Not at all, TYVM for your help! > > > TREE_CONSTANT isn't the correct thing to test. You should use > > TREE_CODE () == INTEGER_CST instead. > > Done (in some cases, via tree_fits_shwi_p). > > > Also you need to handle > > NULL_TREE TYPE_MIN/MAX_VALUE as that can happen as well. > > I've not found any documentation as to what these mean, but from experiment, > it seems that a zero-length array has MIN_VALUE 0 and MAX_VALUE null (as well > as zero TYPE_SIZE) - so I allow that. In contrast a variable-length array also > has zero TYPE_SIZE, but a large range of MIN-MAX, and I think I want to rule > those out. > > Another awkward case is Ada arrays with large offset (e.g. array(2^32...2^32+1) > which has only two elements); I don't see either of tree_to_shwi or tree_to_uhwi > as necessarily being "better" here, each will handle (some) (rare) cases the > other will not, so I've tried to use tree_to_shwi throughout for consistency. > > Summary: taken advantage of tree_fits_shwi_p, as this includes a check against > NULL_TREE and that TREE_CODE () == INTEGER_CST. > > > + if (DECL_P (elem) && DECL_BIT_FIELD (elem)) > > + return false; > > > > that can't happen (TREE_TYPE (array-type) is never a DECL). > > Removed. > > > + int el_size = tree_to_uhwi (elem_size); > > + gcc_assert (el_size); > > > > so you assert on el_size being > 0 later but above you test > > only array size ... > > Good point, thanks. > > > + tree t_idx = build_int_cst (TYPE_DOMAIN (decl_type), idx); > > > > use t_idx = size_int (idx); > > Done. > > I've also added another test, of scalarizing a structure containing a > zero-length array, as earlier attempts accidentally prevented this. > > Bootstrapped + check-gcc,g++,ada,fortran on ARM and x86_64; > Bootstrapped + check-gcc,g++,fortran on AArch64. > > OK for trunk? Ok. Thanks, Richard. > Thanks, > Alan > > gcc/ChangeLog: > > PR tree-optimization/67283 > * tree-sra.c (type_consists_of_records_p): Rename to... > (scalarizable_type_p): ...this, add case for ARRAY_TYPE. > (completely_scalarize_record): Rename to... > (completely_scalarize): ...this, add ARRAY_TYPE case, move some code to: > (scalarize_elem): New. > (analyze_all_variable_accesses): Follow renamings. > > gcc/testsuite/ChangeLog: > > PR tree-optimization/67283 > * gcc.dg/tree-ssa/sra-15.c: New. > * gcc.dg/tree-ssa/sra-16.c: New. > --- > gcc/testsuite/gcc.dg/tree-ssa/sra-15.c | 37 ++++++++ > gcc/testsuite/gcc.dg/tree-ssa/sra-16.c | 37 ++++++++ > gcc/tree-sra.c | 165 +++++++++++++++++++++++---------- > 3 files changed, 191 insertions(+), 48 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/sra-15.c > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/sra-16.c > > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/sra-15.c b/gcc/testsuite/gcc.dg/tree-ssa/sra-15.c > new file mode 100644 > index 0000000..a22062e > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/tree-ssa/sra-15.c > @@ -0,0 +1,37 @@ > +/* Verify that SRA total scalarization works on records containing arrays. */ > +/* { dg-do run } */ > +/* { dg-options "-O1 -fdump-tree-release_ssa --param sra-max-scalarization-size-Ospeed=32" } */ > + > +extern void abort (void); > + > +struct S > +{ > + char c; > + unsigned short f[2][2]; > + int i; > + unsigned short f3, f4; > +}; > + > + > +int __attribute__ ((noinline)) > +foo (struct S *p) > +{ > + struct S l; > + > + l = *p; > + l.i++; > + l.f[1][0] += 3; > + *p = l; > +} > + > +int > +main (int argc, char **argv) > +{ > + struct S a = {0, { {5, 7}, {9, 11} }, 4, 0, 0}; > + foo (&a); > + if (a.i != 5 || a.f[1][0] != 12) > + abort (); > + return 0; > +} > + > +/* { dg-final { scan-tree-dump-times "l;" 0 "release_ssa" } } */ > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/sra-16.c b/gcc/testsuite/gcc.dg/tree-ssa/sra-16.c > new file mode 100644 > index 0000000..fef34c0 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/tree-ssa/sra-16.c > @@ -0,0 +1,37 @@ > +/* Verify that SRA total scalarization works on records containing arrays. */ > +/* { dg-do run } */ > +/* { dg-options "-O1 -fdump-tree-release_ssa --param sra-max-scalarization-size-Ospeed=16" } */ > + > +extern void abort (void); > + > +struct S > +{ > + long zilch[0]; > + char c; > + int i; > + unsigned short f3, f4; > +}; > + > + > +int __attribute__ ((noinline)) > +foo (struct S *p) > +{ > + struct S l; > + > + l = *p; > + l.i++; > + l.f3++; > + *p = l; > +} > + > +int > +main (int argc, char **argv) > +{ > + struct S a = { { }, 0, 4, 0, 0}; > + foo (&a); > + if (a.i != 5 || a.f3 != 1) > + abort (); > + return 0; > +} > + > +/* { dg-final { scan-tree-dump-times "l;" 0 "release_ssa" } } */ > diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c > index 8b3a0ad..8247554 100644 > --- a/gcc/tree-sra.c > +++ b/gcc/tree-sra.c > @@ -915,73 +915,142 @@ create_access (tree expr, gimple stmt, bool write) > } > > > -/* Return true iff TYPE is a RECORD_TYPE with fields that are either of gimple > - register types or (recursively) records with only these two kinds of fields. > - It also returns false if any of these records contains a bit-field. */ > +/* Return true iff TYPE is scalarizable - i.e. a RECORD_TYPE or fixed-length > + ARRAY_TYPE with fields that are either of gimple register types (excluding > + bit-fields) or (recursively) scalarizable types. */ > > static bool > -type_consists_of_records_p (tree type) > +scalarizable_type_p (tree type) > { > - tree fld; > + gcc_assert (!is_gimple_reg_type (type)); > > - if (TREE_CODE (type) != RECORD_TYPE) > - return false; > + switch (TREE_CODE (type)) > + { > + case RECORD_TYPE: > + for (tree fld = TYPE_FIELDS (type); fld; fld = DECL_CHAIN (fld)) > + if (TREE_CODE (fld) == FIELD_DECL) > + { > + tree ft = TREE_TYPE (fld); > > - for (fld = TYPE_FIELDS (type); fld; fld = DECL_CHAIN (fld)) > - if (TREE_CODE (fld) == FIELD_DECL) > - { > - tree ft = TREE_TYPE (fld); > + if (DECL_BIT_FIELD (fld)) > + return false; > > - if (DECL_BIT_FIELD (fld)) > - return false; > + if (!is_gimple_reg_type (ft) > + && !scalarizable_type_p (ft)) > + return false; > + } > > - if (!is_gimple_reg_type (ft) > - && !type_consists_of_records_p (ft)) > - return false; > - } > + return true; > > - return true; > + case ARRAY_TYPE: > + { > + if (TYPE_DOMAIN (type) == NULL_TREE > + || !tree_fits_shwi_p (TYPE_SIZE (type)) > + || !tree_fits_shwi_p (TYPE_SIZE (TREE_TYPE (type))) > + || (tree_to_shwi (TYPE_SIZE (TREE_TYPE (type))) <= 0) > + || !tree_fits_shwi_p (TYPE_MIN_VALUE (TYPE_DOMAIN (type)))) > + return false; > + if (tree_to_shwi (TYPE_SIZE (type)) == 0 > + && TYPE_MAX_VALUE (TYPE_DOMAIN (type)) == NULL_TREE) > + /* Zero-element array, should not prevent scalarization. */ > + ; > + else if ((tree_to_shwi (TYPE_SIZE (type)) <= 0) > + || !tree_fits_shwi_p (TYPE_MAX_VALUE (TYPE_DOMAIN (type)))) > + return false; > + > + tree elem = TREE_TYPE (type); > + if (!is_gimple_reg_type (elem) > + && !scalarizable_type_p (elem)) > + return false; > + return true; > + } > + default: > + return false; > + } > } > > -/* Create total_scalarization accesses for all scalar type fields in DECL that > - must be of a RECORD_TYPE conforming to type_consists_of_records_p. BASE > - must be the top-most VAR_DECL representing the variable, OFFSET must be the > - offset of DECL within BASE. REF must be the memory reference expression for > - the given decl. */ > +static void scalarize_elem (tree, HOST_WIDE_INT, HOST_WIDE_INT, tree, tree); > + > +/* Create total_scalarization accesses for all scalar fields of a member > + of type DECL_TYPE conforming to scalarizable_type_p. BASE > + must be the top-most VAR_DECL representing the variable; within that, > + OFFSET locates the member and REF must be the memory reference expression for > + the member. */ > > static void > -completely_scalarize_record (tree base, tree decl, HOST_WIDE_INT offset, > - tree ref) > +completely_scalarize (tree base, tree decl_type, HOST_WIDE_INT offset, tree ref) > { > - tree fld, decl_type = TREE_TYPE (decl); > + switch (TREE_CODE (decl_type)) > + { > + case RECORD_TYPE: > + for (tree fld = TYPE_FIELDS (decl_type); fld; fld = DECL_CHAIN (fld)) > + if (TREE_CODE (fld) == FIELD_DECL) > + { > + HOST_WIDE_INT pos = offset + int_bit_position (fld); > + tree ft = TREE_TYPE (fld); > + tree nref = build3 (COMPONENT_REF, ft, ref, fld, NULL_TREE); > > - for (fld = TYPE_FIELDS (decl_type); fld; fld = DECL_CHAIN (fld)) > - if (TREE_CODE (fld) == FIELD_DECL) > + scalarize_elem (base, pos, tree_to_uhwi (DECL_SIZE (fld)), nref, > + ft); > + } > + break; > + case ARRAY_TYPE: > { > - HOST_WIDE_INT pos = offset + int_bit_position (fld); > - tree ft = TREE_TYPE (fld); > - tree nref = build3 (COMPONENT_REF, TREE_TYPE (fld), ref, fld, > - NULL_TREE); > - > - if (is_gimple_reg_type (ft)) > + tree elemtype = TREE_TYPE (decl_type); > + tree elem_size = TYPE_SIZE (elemtype); > + gcc_assert (elem_size && tree_fits_shwi_p (elem_size)); > + HOST_WIDE_INT el_size = tree_to_shwi (elem_size); > + gcc_assert (el_size > 0); > + > + tree minidx = TYPE_MIN_VALUE (TYPE_DOMAIN (decl_type)); > + gcc_assert (TREE_CODE (minidx) == INTEGER_CST); > + tree maxidx = TYPE_MAX_VALUE (TYPE_DOMAIN (decl_type)); > + if (maxidx) > { > - struct access *access; > - HOST_WIDE_INT size; > - > - size = tree_to_uhwi (DECL_SIZE (fld)); > - access = create_access_1 (base, pos, size); > - access->expr = nref; > - access->type = ft; > - access->grp_total_scalarization = 1; > - /* Accesses for intraprocedural SRA can have their stmt NULL. */ > + gcc_assert (TREE_CODE (maxidx) == INTEGER_CST); > + /* MINIDX and MAXIDX are inclusive. Try to avoid overflow. */ > + unsigned HOST_WIDE_INT lenp1 = tree_to_shwi (maxidx) > + - tree_to_shwi (minidx); > + unsigned HOST_WIDE_INT idx = 0; > + do > + { > + tree nref = build4 (ARRAY_REF, elemtype, ref, size_int (idx), > + NULL_TREE, NULL_TREE); > + int el_off = offset + idx * el_size; > + scalarize_elem (base, el_off, el_size, nref, elemtype); > + } > + while (++idx <= lenp1); > } > - else > - completely_scalarize_record (base, fld, pos, nref); > } > + break; > + default: > + gcc_unreachable (); > + } > +} > + > +/* Create total_scalarization accesses for a member of type TYPE, which must > + satisfy either is_gimple_reg_type or scalarizable_type_p. BASE must be the > + top-most VAR_DECL representing the variable; within that, POS and SIZE locate > + the member and REF must be the reference expression for it. */ > + > +static void > +scalarize_elem (tree base, HOST_WIDE_INT pos, HOST_WIDE_INT size, > + tree ref, tree type) > +{ > + if (is_gimple_reg_type (type)) > + { > + struct access *access = create_access_1 (base, pos, size); > + access->expr = ref; > + access->type = type; > + access->grp_total_scalarization = 1; > + /* Accesses for intraprocedural SRA can have their stmt NULL. */ > + } > + else > + completely_scalarize (base, type, pos, ref); > } > > /* Create a total_scalarization access for VAR as a whole. VAR must be of a > - RECORD_TYPE conforming to type_consists_of_records_p. */ > + RECORD_TYPE or ARRAY_TYPE conforming to scalarizable_type_p. */ > > static void > create_total_scalarization_access (tree var) > @@ -2521,13 +2590,13 @@ analyze_all_variable_accesses (void) > tree var = candidate (i); > > if (TREE_CODE (var) == VAR_DECL > - && type_consists_of_records_p (TREE_TYPE (var))) > + && scalarizable_type_p (TREE_TYPE (var))) > { > if (tree_to_uhwi (TYPE_SIZE (TREE_TYPE (var))) > <= max_scalarization_size) > { > create_total_scalarization_access (var); > - completely_scalarize_record (var, var, 0, var); > + completely_scalarize (var, TREE_TYPE (var), 0, var); > if (dump_file && (dump_flags & TDF_DETAILS)) > { > fprintf (dump_file, "Will attempt to totally scalarize "); > -- Richard Biener SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)