| Message ID | 1440500777-25966-3-git-send-email-alan.lawrence@arm.com |
|---|---|
| State | New |
| Headers | show |
On 08/25/2015 05:06 AM, Alan Lawrence wrote: > This changes the completely_scalarize_record path to also work on arrays (thus > allowing records containing arrays, etc.). This just required extending the > existing type_consists_of_records_p and completely_scalarize_record methods > to handle things of ARRAY_TYPE as well as RECORD_TYPE. Hence, I renamed both > methods so as not to mention 'record'. > > Bootstrapped + check-gcc on aarch64-none-linux-gnu, arm-none-linux-gnueabihf and x86_64-none-linux-gnu. > > Have also verified the scan-tree-dump check in the new sra-15.c passes (using a stage 1 compiler only, no execution test) on alpha, hppa, powerpc, sparc, avr and sh. > > gcc/ChangeLog: > > * 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. > > gcc/testsuite/ChangeLog: > * gcc.dg/tree-ssa/sra-15.c: New. > --- > gcc/testsuite/gcc.dg/tree-ssa/sra-15.c | 38 +++++++++ > gcc/tree-sra.c | 146 ++++++++++++++++++++++----------- > 2 files changed, 135 insertions(+), 49 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/sra-15.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..e251058 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/tree-ssa/sra-15.c > @@ -0,0 +1,38 @@ > +/* Verify that SRA total scalarization works on records containing arrays. */ > +/* Test skipped for targets with small (often default) MOVE_RATIO. */ ?!? I don't see anything that skips this test for any targets. Presumably this was copied from sra-12.c. I suspect this comment should just be removed. With that comment removed from the testcase, this is OK. jeff
Hi, On Tue, Aug 25, 2015 at 12:06:14PM +0100, Alan Lawrence wrote: > This changes the completely_scalarize_record path to also work on arrays (thus > allowing records containing arrays, etc.). This just required extending the > existing type_consists_of_records_p and completely_scalarize_record methods > to handle things of ARRAY_TYPE as well as RECORD_TYPE. Hence, I renamed both > methods so as not to mention 'record'. thanks for working on this. I see Jeff has already approved the patch, but I have two comments nevertheless. First, I would be much happier if you added a proper comment to scalarize_elem function which you forgot completely. The name is not very descriptive and it has quite few parameters too. Second, this patch should also fix PR 67283. It would be great if you could verify that and add it to the changelog when committing if that is indeed the case. Thanks, Martin > > Bootstrapped + check-gcc on aarch64-none-linux-gnu, arm-none-linux-gnueabihf and x86_64-none-linux-gnu. > > Have also verified the scan-tree-dump check in the new sra-15.c passes (using a stage 1 compiler only, no execution test) on alpha, hppa, powerpc, sparc, avr and sh. > > gcc/ChangeLog: > > * 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. > > gcc/testsuite/ChangeLog: > * gcc.dg/tree-ssa/sra-15.c: New. > --- > gcc/testsuite/gcc.dg/tree-ssa/sra-15.c | 38 +++++++++ > gcc/tree-sra.c | 146 ++++++++++++++++++++++----------- > 2 files changed, 135 insertions(+), 49 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/sra-15.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..e251058 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/tree-ssa/sra-15.c > @@ -0,0 +1,38 @@ > +/* Verify that SRA total scalarization works on records containing arrays. */ > +/* Test skipped for targets with small (often default) MOVE_RATIO. */ > +/* { 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/tree-sra.c b/gcc/tree-sra.c > index a0c92b0..08fa8dc 100644 > --- a/gcc/tree-sra.c > +++ b/gcc/tree-sra.c > @@ -915,74 +915,122 @@ 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 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: > + { > + tree elem = TREE_TYPE (type); > + if (DECL_P (elem) && DECL_BIT_FIELD (elem)) > + return false; > + 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_uhwi_p (elem_size)); > + int el_size = tree_to_uhwi (elem_size); > + gcc_assert (el_size); > + > + tree minidx = TYPE_MIN_VALUE (TYPE_DOMAIN (decl_type)); > + tree maxidx = TYPE_MAX_VALUE (TYPE_DOMAIN (decl_type)); > + gcc_assert (TREE_CODE (minidx) == INTEGER_CST > + && TREE_CODE (maxidx) == INTEGER_CST); > + unsigned HOST_WIDE_INT len = tree_to_uhwi (maxidx) > + + 1 - tree_to_uhwi (minidx); > + /* 4th operand to ARRAY_REF is size in units of the type alignment. */ > + for (unsigned HOST_WIDE_INT idx = 0; idx < len; idx++) > { > - 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. */ > + tree t_idx = build_int_cst (TYPE_DOMAIN (decl_type), idx); > + tree nref = build4 (ARRAY_REF, elemtype, ref, t_idx, NULL_TREE, > + NULL_TREE); > + int el_off = offset + idx * el_size; > + scalarize_elem (base, el_off, el_size, nref, elemtype); > } > - else > - completely_scalarize_record (base, fld, pos, nref); > } > + break; > + default: > + gcc_unreachable (); > + } > +} > + > +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 total_scalarization accesses for all scalar type fields in VAR and > - for VAR as a whole. VAR must be of a RECORD_TYPE conforming to > - type_consists_of_records_p. */ > + for VAR as a whole. VAR must be of a RECORD_TYPE or ARRAY_TYPE conforming to > + scalarizable_type_p. */ > > static void > create_total_scalarization_access (tree var) > @@ -2522,13 +2570,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 "); > -- > 1.8.3 >
On 08/25/2015 03:42 PM, Martin Jambor wrote: > Hi, > > On Tue, Aug 25, 2015 at 12:06:14PM +0100, Alan Lawrence wrote: >> This changes the completely_scalarize_record path to also work on arrays (thus >> allowing records containing arrays, etc.). This just required extending the >> existing type_consists_of_records_p and completely_scalarize_record methods >> to handle things of ARRAY_TYPE as well as RECORD_TYPE. Hence, I renamed both >> methods so as not to mention 'record'. > > thanks for working on this. I see Jeff has already approved the > patch, but I have two comments nevertheless. First, I would be much > happier if you added a proper comment to scalarize_elem function which > you forgot completely. The name is not very descriptive and it has > quite few parameters too. Right. I mentioned that I missed the lack of function comments when looking at #3 and asked Alan to go back and fix them in #1 and #2. > > Second, this patch should also fix PR 67283. It would be great if you > could verify that and add it to the changelog when committing if that > is indeed the case. Excellent. Yes, definitely mention the BZ. jeff
On Tue, Aug 25, 2015 at 11:44 PM, Jeff Law <law@redhat.com> wrote: > On 08/25/2015 03:42 PM, Martin Jambor wrote: >> >> Hi, >> >> On Tue, Aug 25, 2015 at 12:06:14PM +0100, Alan Lawrence wrote: >>> >>> This changes the completely_scalarize_record path to also work on arrays >>> (thus >>> allowing records containing arrays, etc.). This just required extending >>> the >>> existing type_consists_of_records_p and completely_scalarize_record >>> methods >>> to handle things of ARRAY_TYPE as well as RECORD_TYPE. Hence, I renamed >>> both >>> methods so as not to mention 'record'. >> >> >> thanks for working on this. I see Jeff has already approved the >> patch, but I have two comments nevertheless. First, I would be much >> happier if you added a proper comment to scalarize_elem function which >> you forgot completely. The name is not very descriptive and it has >> quite few parameters too. > > Right. I mentioned that I missed the lack of function comments when looking > at #3 and asked Alan to go back and fix them in #1 and #2. > >> >> Second, this patch should also fix PR 67283. It would be great if you >> could verify that and add it to the changelog when committing if that >> is indeed the case. > > Excellent. Yes, definitely mention the BZ. One extra question is does the way we limit total scalarization work well for arrays? I suppose we have either sth like the maximum size of an aggregate we scalarize or the maximum number of component accesses we create? Thanks, Richard. > jeff >
Hi, On Wed, Aug 26, 2015 at 09:07:33AM +0200, Richard Biener wrote: > On Tue, Aug 25, 2015 at 11:44 PM, Jeff Law <law@redhat.com> wrote: > > On 08/25/2015 03:42 PM, Martin Jambor wrote: > >> > >> Hi, > >> > >> On Tue, Aug 25, 2015 at 12:06:14PM +0100, Alan Lawrence wrote: > >>> > >>> This changes the completely_scalarize_record path to also work on arrays > >>> (thus > >>> allowing records containing arrays, etc.). This just required extending > >>> the > >>> existing type_consists_of_records_p and completely_scalarize_record > >>> methods > >>> to handle things of ARRAY_TYPE as well as RECORD_TYPE. Hence, I renamed > >>> both > >>> methods so as not to mention 'record'. > >> > >> > >> thanks for working on this. I see Jeff has already approved the > >> patch, but I have two comments nevertheless. First, I would be much > >> happier if you added a proper comment to scalarize_elem function which > >> you forgot completely. The name is not very descriptive and it has > >> quite few parameters too. > > > > Right. I mentioned that I missed the lack of function comments when looking > > at #3 and asked Alan to go back and fix them in #1 and #2. > > > >> > >> Second, this patch should also fix PR 67283. It would be great if you > >> could verify that and add it to the changelog when committing if that > >> is indeed the case. > > > > Excellent. Yes, definitely mention the BZ. > > One extra question is does the way we limit total scalarization work well > for arrays? I suppose we have either sth like the maximum size of an > aggregate we scalarize or the maximum number of component accesses > we create? > Only the former and that would be kept intact. It is in fact visible in the context of the last hunk of the patch. Martin
On August 26, 2015 11:30:26 AM GMT+02:00, Martin Jambor <mjambor@suse.cz> wrote: >Hi, > >On Wed, Aug 26, 2015 at 09:07:33AM +0200, Richard Biener wrote: >> On Tue, Aug 25, 2015 at 11:44 PM, Jeff Law <law@redhat.com> wrote: >> > On 08/25/2015 03:42 PM, Martin Jambor wrote: >> >> >> >> Hi, >> >> >> >> On Tue, Aug 25, 2015 at 12:06:14PM +0100, Alan Lawrence wrote: >> >>> >> >>> This changes the completely_scalarize_record path to also work on >arrays >> >>> (thus >> >>> allowing records containing arrays, etc.). This just required >extending >> >>> the >> >>> existing type_consists_of_records_p and >completely_scalarize_record >> >>> methods >> >>> to handle things of ARRAY_TYPE as well as RECORD_TYPE. Hence, I >renamed >> >>> both >> >>> methods so as not to mention 'record'. >> >> >> >> >> >> thanks for working on this. I see Jeff has already approved the >> >> patch, but I have two comments nevertheless. First, I would be >much >> >> happier if you added a proper comment to scalarize_elem function >which >> >> you forgot completely. The name is not very descriptive and it >has >> >> quite few parameters too. >> > >> > Right. I mentioned that I missed the lack of function comments >when looking >> > at #3 and asked Alan to go back and fix them in #1 and #2. >> > >> >> >> >> Second, this patch should also fix PR 67283. It would be great if >you >> >> could verify that and add it to the changelog when committing if >that >> >> is indeed the case. >> > >> > Excellent. Yes, definitely mention the BZ. >> >> One extra question is does the way we limit total scalarization work >well >> for arrays? I suppose we have either sth like the maximum size of an >> aggregate we scalarize or the maximum number of component accesses >> we create? >> > >Only the former and that would be kept intact. It is in fact visible >in the context of the last hunk of the patch. OK. IIRC the gimplification code also has the latter and also considers zeroing the whole aggregate before initializing non-zero fields. IMHO it makes sense to reuse some of the analysis and classification routines it has. Richard. >Martin
Richard Biener wrote: >>> One extra question is does the way we limit total scalarization work >> well >>> for arrays? I suppose we have either sth like the maximum size of an >>> aggregate we scalarize or the maximum number of component accesses >>> we create? >>> >> Only the former and that would be kept intact. It is in fact visible >> in the context of the last hunk of the patch. > > OK. IIRC the gimplification code also has the latter and also considers zeroing the whole aggregate before initializing non-zero fields. IMHO it makes sense to reuse some of the analysis and classification routines it has. Do you mean gimplify_init_constructor? Yes, there's quite a lot of logic there ;). That feels like a separate patch - and belonging to the constant-handling subseries of this series - as gimplify_init_constructor already deals with both record and array types, and I don't see anything there that's specifically good for total-scalarization of arrays....? IOW, do you mean that to block this patch, or can it be separate (I can address Martin + Jeff's comments fairly quickly and independently) ? Cheers, Alan
On August 26, 2015 6:08:55 PM GMT+02:00, Alan Lawrence <alan.lawrence@arm.com> wrote: >Richard Biener wrote: > >>>> One extra question is does the way we limit total scalarization >work >>> well >>>> for arrays? I suppose we have either sth like the maximum size of >an >>>> aggregate we scalarize or the maximum number of component accesses >>>> we create? >>>> >>> Only the former and that would be kept intact. It is in fact >visible >>> in the context of the last hunk of the patch. >> >> OK. IIRC the gimplification code also has the latter and also >considers zeroing the whole aggregate before initializing non-zero >fields. IMHO it makes sense to reuse some of the analysis and >classification routines it has. > >Do you mean gimplify_init_constructor? Yes, there's quite a lot of >logic there >;). That feels like a separate patch - and belonging to the >constant-handling >subseries of this series Yes. - as gimplify_init_constructor already deals >with both >record and array types, and I don't see anything there that's >specifically good >for total-scalarization of arrays....? > >IOW, do you mean that to block this patch, or can it be separate (I can >address >Martin + Jeff's comments fairly quickly and independently) ? No, but I'd like this being explores with the init sub series. We don't want two places doing total scalarization of initualizers , gimplification and SRA and with different/conflicting heuristics. IMHO the gimplification total scalarization happens too early. Richard. >Cheers, Alan
Jeff Law wrote: > >> 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..e251058 >> --- /dev/null >> +++ b/gcc/testsuite/gcc.dg/tree-ssa/sra-15.c >> @@ -0,0 +1,38 @@ >> +/* Verify that SRA total scalarization works on records containing arrays. */ >> +/* Test skipped for targets with small (often default) MOVE_RATIO. */ > ?!? I don't see anything that skips this test for any targets. > Presumably this was copied from sra-12.c. I suspect this comment should > just be removed. You are right, thank you. Copied from sra-12.c, was I that obvious? ;). Indeed I notice that the same trick (of passing --param sra-max-scalarization-size-Ospeed=32) enables sra-12.c to pass on avr-unknown-linux-gnu and sh-unknown-linux-gnu, too; I haven't managed to build a compiler for nds32 yet... --Alan
On 08/27/2015 10:48 AM, Alan Lawrence wrote: > Jeff Law wrote: >> >>> 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..e251058 >>> --- /dev/null >>> +++ b/gcc/testsuite/gcc.dg/tree-ssa/sra-15.c >>> @@ -0,0 +1,38 @@ >>> +/* Verify that SRA total scalarization works on records containing >>> arrays. */ >>> +/* Test skipped for targets with small (often default) MOVE_RATIO. */ >> ?!? I don't see anything that skips this test for any targets. >> Presumably this was copied from sra-12.c. I suspect this comment >> should just be removed. > > You are right, thank you. Copied from sra-12.c, was I that obvious? ;). > Indeed I notice that the same trick (of passing --param > sra-max-scalarization-size-Ospeed=32) enables sra-12.c to pass on > avr-unknown-linux-gnu and sh-unknown-linux-gnu, too; I haven't managed > to build a compiler for nds32 yet... The comment just caught my eye as I didn't see any target bits in the test. A quick search for MOVE_RATIO in the testsuite popped up sra-12. Jeff
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..e251058 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/sra-15.c @@ -0,0 +1,38 @@ +/* Verify that SRA total scalarization works on records containing arrays. */ +/* Test skipped for targets with small (often default) MOVE_RATIO. */ +/* { 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/tree-sra.c b/gcc/tree-sra.c index a0c92b0..08fa8dc 100644 --- a/gcc/tree-sra.c +++ b/gcc/tree-sra.c @@ -915,74 +915,122 @@ 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 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: + { + tree elem = TREE_TYPE (type); + if (DECL_P (elem) && DECL_BIT_FIELD (elem)) + return false; + 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_uhwi_p (elem_size)); + int el_size = tree_to_uhwi (elem_size); + gcc_assert (el_size); + + tree minidx = TYPE_MIN_VALUE (TYPE_DOMAIN (decl_type)); + tree maxidx = TYPE_MAX_VALUE (TYPE_DOMAIN (decl_type)); + gcc_assert (TREE_CODE (minidx) == INTEGER_CST + && TREE_CODE (maxidx) == INTEGER_CST); + unsigned HOST_WIDE_INT len = tree_to_uhwi (maxidx) + + 1 - tree_to_uhwi (minidx); + /* 4th operand to ARRAY_REF is size in units of the type alignment. */ + for (unsigned HOST_WIDE_INT idx = 0; idx < len; idx++) { - 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. */ + tree t_idx = build_int_cst (TYPE_DOMAIN (decl_type), idx); + tree nref = build4 (ARRAY_REF, elemtype, ref, t_idx, NULL_TREE, + NULL_TREE); + int el_off = offset + idx * el_size; + scalarize_elem (base, el_off, el_size, nref, elemtype); } - else - completely_scalarize_record (base, fld, pos, nref); } + break; + default: + gcc_unreachable (); + } +} + +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 total_scalarization accesses for all scalar type fields in VAR and - for VAR as a whole. VAR must be of a RECORD_TYPE conforming to - type_consists_of_records_p. */ + for VAR as a whole. VAR must be of a RECORD_TYPE or ARRAY_TYPE conforming to + scalarizable_type_p. */ static void create_total_scalarization_access (tree var) @@ -2522,13 +2570,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 ");