| Message ID | 20170601113923.rx3sdkm4xjik5zzt@virgil.suse.cz |
|---|---|
| State | New |
| Headers | show |
On Thu, 1 Jun 2017, Martin Jambor wrote: > Hi, > > when I wrote the lazy setting of grp_write flag early next year, I > made a mistake when thinking about what to do about SRA candidates > that were disqualified but form a RHS of an assignment link which was > to be used to set grp_write of the LHS when appropriate. The code > expects that the RHS accesses form an access tree, but given that some > are rejected exactly because such a tree cannot be built, it does not > work. > > The solution is to move dealing with disqualified RHSs to the > assignment link processing. The patch below checks RHS and if it is > disqualified, marks the corresponding LHS as containing data. As the > second testcase shows, that information must be then also propagated > downwards (this is not necessary in the normal propagation case > because there propagate_subaccesses_across_link will already do that > more elaborately) as well as upwards. > > Bootstrapped and tested on x86_64-linux without any issues. OK for > trunk? Ok. Thanks, Richard. > > Thanks, > > Martin > > > 2017-06-01 Martin Jambor <mjambor@suse.cz> > > PR tree-optimization/80898 > * tree-sra.c (process_subtree_disqualification): Removed. > (disqualify_candidate): Do not call > process_subtree_disqualification. > (subtree_mark_written_and_enqueue): New function. > (propagate_all_subaccesses): Set grp_write of LHS subtree if the > RHS has been disqualified and re-queue LHS if necessary. Apart > from that, ignore disqualified RHS. > > testsuite/ > * gcc.dg/tree-ssa/pr80898.c: New test. > * gcc.dg/tree-ssa/pr80898-2.c: Likewise. > --- > gcc/testsuite/gcc.dg/tree-ssa/pr80898-2.c | 71 +++++++++++++++++++++++++++++++ > gcc/testsuite/gcc.dg/tree-ssa/pr80898.c | 20 +++++++++ > gcc/tree-sra.c | 56 +++++++++++++++--------- > 3 files changed, 126 insertions(+), 21 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr80898-2.c > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr80898.c > > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr80898-2.c b/gcc/testsuite/gcc.dg/tree-ssa/pr80898-2.c > new file mode 100644 > index 00000000000..cb4799c5ced > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr80898-2.c > @@ -0,0 +1,71 @@ > +/* { dg-do run } */ > +/* { dg-options "-O2" } */ > + > +struct S0 > +{ > + unsigned a : 15; > + int b; > + int c; > +}; > + > +struct S1 > +{ > + struct S0 s0; > + int e; > +}; > + > +struct Z > +{ > + char c; > + int z; > +} __attribute__((packed)); > + > +union U > +{ > + struct S1 s1; > + struct Z z; > +}; > + > + > +int __attribute__((noinline, noclone)) > +return_zero (void) > +{ > + return 0; > +} > + > +volatile union U gu; > +struct S0 gs; > + > +int __attribute__((noinline, noclone)) > +check_outcome () > +{ > + if (gs.a != 6 > + || gs.b != 80000) > + __builtin_abort (); > +} > + > +int > +main (int argc, char *argv[]) > +{ > + union U u; > + struct S1 m; > + struct S0 l; > + > + if (return_zero ()) > + u.z.z = 20000; > + else > + { > + u.s1.s0.a = 6; > + u.s1.s0.b = 80000; > + u.s1.e = 2; > + > + m = u.s1; > + m.s0.c = 0; > + l = m.s0; > + gs = l; > + } > + > + gu = u; > + check_outcome (); > + return 0; > +} > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr80898.c b/gcc/testsuite/gcc.dg/tree-ssa/pr80898.c > new file mode 100644 > index 00000000000..ed88f2cbd1a > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr80898.c > @@ -0,0 +1,20 @@ > +/* { dg-do run } */ > +/* { dg-options "-O2" } */ > + > +struct S0 { > + int f0 : 24; > + int f1; > + int f74; > +} a, *c = &a; > +struct S0 fn1() { > + struct S0 b = {4, 3}; > + return b; > +} > + > +int main() { > + *c = fn1(); > + > + if (a.f1 != 3) > + __builtin_abort (); > + return 0; > +} > diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c > index 6a8a0a4a427..f25818f4481 100644 > --- a/gcc/tree-sra.c > +++ b/gcc/tree-sra.c > @@ -694,21 +694,9 @@ static bool constant_decl_p (tree decl) > return VAR_P (decl) && DECL_IN_CONSTANT_POOL (decl); > } > > - > -/* Mark LHS of assign links out of ACCESS and its children as written to. */ > - > -static void > -process_subtree_disqualification (struct access *access) > -{ > - struct access *child; > - for (struct assign_link *link = access->first_link; link; link = link->next) > - link->lacc->grp_write = true; > - for (child = access->first_child; child; child = child->next_sibling) > - process_subtree_disqualification (child); > -} > - > /* Remove DECL from candidates for SRA and write REASON to the dump file if > there is one. */ > + > static void > disqualify_candidate (tree decl, const char *reason) > { > @@ -723,13 +711,6 @@ disqualify_candidate (tree decl, const char *reason) > print_generic_expr (dump_file, decl); > fprintf (dump_file, " - %s\n", reason); > } > - > - struct access *access = get_first_repr_for_decl (decl); > - while (access) > - { > - process_subtree_disqualification (access); > - access = access->next_grp; > - } > } > > /* Return true iff the type contains a field or an element which does not allow > @@ -2679,6 +2660,26 @@ propagate_subaccesses_across_link (struct access *lacc, struct access *racc) > return ret; > } > > +/* Beginning with ACCESS, traverse its whole access subtree and mark all > + sub-trees as written to. If any of them has not been marked so previously > + and has assignment links leading from it, re-enqueue it. */ > + > +static void > +subtree_mark_written_and_enqueue (struct access *access) > +{ > + if (access->grp_write) > + return; > + access->grp_write = true; > + if (access->first_link) > + add_access_to_work_queue (access); > + > + struct access *child; > + for (child = access->first_child; child; child = child->next_sibling) > + subtree_mark_written_and_enqueue (child); > +} > + > + > + > /* Propagate all subaccesses across assignment links. */ > > static void > @@ -2698,7 +2699,20 @@ propagate_all_subaccesses (void) > if (!bitmap_bit_p (candidate_bitmap, DECL_UID (lacc->base))) > continue; > lacc = lacc->group_representative; > - if (propagate_subaccesses_across_link (lacc, racc)) > + > + bool reque_parents = false; > + if (!bitmap_bit_p (candidate_bitmap, DECL_UID (racc->base))) > + { > + if (!lacc->grp_write) > + { > + subtree_mark_written_and_enqueue (lacc); > + reque_parents = true; > + } > + } > + else if (propagate_subaccesses_across_link (lacc, racc)) > + reque_parents = true; > + > + if (reque_parents) > do > { > if (lacc->first_link) >
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr80898-2.c b/gcc/testsuite/gcc.dg/tree-ssa/pr80898-2.c new file mode 100644 index 00000000000..cb4799c5ced --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr80898-2.c @@ -0,0 +1,71 @@ +/* { dg-do run } */ +/* { dg-options "-O2" } */ + +struct S0 +{ + unsigned a : 15; + int b; + int c; +}; + +struct S1 +{ + struct S0 s0; + int e; +}; + +struct Z +{ + char c; + int z; +} __attribute__((packed)); + +union U +{ + struct S1 s1; + struct Z z; +}; + + +int __attribute__((noinline, noclone)) +return_zero (void) +{ + return 0; +} + +volatile union U gu; +struct S0 gs; + +int __attribute__((noinline, noclone)) +check_outcome () +{ + if (gs.a != 6 + || gs.b != 80000) + __builtin_abort (); +} + +int +main (int argc, char *argv[]) +{ + union U u; + struct S1 m; + struct S0 l; + + if (return_zero ()) + u.z.z = 20000; + else + { + u.s1.s0.a = 6; + u.s1.s0.b = 80000; + u.s1.e = 2; + + m = u.s1; + m.s0.c = 0; + l = m.s0; + gs = l; + } + + gu = u; + check_outcome (); + return 0; +} diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr80898.c b/gcc/testsuite/gcc.dg/tree-ssa/pr80898.c new file mode 100644 index 00000000000..ed88f2cbd1a --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr80898.c @@ -0,0 +1,20 @@ +/* { dg-do run } */ +/* { dg-options "-O2" } */ + +struct S0 { + int f0 : 24; + int f1; + int f74; +} a, *c = &a; +struct S0 fn1() { + struct S0 b = {4, 3}; + return b; +} + +int main() { + *c = fn1(); + + if (a.f1 != 3) + __builtin_abort (); + return 0; +} diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c index 6a8a0a4a427..f25818f4481 100644 --- a/gcc/tree-sra.c +++ b/gcc/tree-sra.c @@ -694,21 +694,9 @@ static bool constant_decl_p (tree decl) return VAR_P (decl) && DECL_IN_CONSTANT_POOL (decl); } - -/* Mark LHS of assign links out of ACCESS and its children as written to. */ - -static void -process_subtree_disqualification (struct access *access) -{ - struct access *child; - for (struct assign_link *link = access->first_link; link; link = link->next) - link->lacc->grp_write = true; - for (child = access->first_child; child; child = child->next_sibling) - process_subtree_disqualification (child); -} - /* Remove DECL from candidates for SRA and write REASON to the dump file if there is one. */ + static void disqualify_candidate (tree decl, const char *reason) { @@ -723,13 +711,6 @@ disqualify_candidate (tree decl, const char *reason) print_generic_expr (dump_file, decl); fprintf (dump_file, " - %s\n", reason); } - - struct access *access = get_first_repr_for_decl (decl); - while (access) - { - process_subtree_disqualification (access); - access = access->next_grp; - } } /* Return true iff the type contains a field or an element which does not allow @@ -2679,6 +2660,26 @@ propagate_subaccesses_across_link (struct access *lacc, struct access *racc) return ret; } +/* Beginning with ACCESS, traverse its whole access subtree and mark all + sub-trees as written to. If any of them has not been marked so previously + and has assignment links leading from it, re-enqueue it. */ + +static void +subtree_mark_written_and_enqueue (struct access *access) +{ + if (access->grp_write) + return; + access->grp_write = true; + if (access->first_link) + add_access_to_work_queue (access); + + struct access *child; + for (child = access->first_child; child; child = child->next_sibling) + subtree_mark_written_and_enqueue (child); +} + + + /* Propagate all subaccesses across assignment links. */ static void @@ -2698,7 +2699,20 @@ propagate_all_subaccesses (void) if (!bitmap_bit_p (candidate_bitmap, DECL_UID (lacc->base))) continue; lacc = lacc->group_representative; - if (propagate_subaccesses_across_link (lacc, racc)) + + bool reque_parents = false; + if (!bitmap_bit_p (candidate_bitmap, DECL_UID (racc->base))) + { + if (!lacc->grp_write) + { + subtree_mark_written_and_enqueue (lacc); + reque_parents = true; + } + } + else if (propagate_subaccesses_across_link (lacc, racc)) + reque_parents = true; + + if (reque_parents) do { if (lacc->first_link)