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? Thanks, Martin 2017-06-01 Martin Jambor <mjam...@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) -- 2.12.2