* [PR 80898] Propagate grp_write from disqualified SRA candidates
@ 2017-06-01 11:39 Martin Jambor
2017-06-01 11:52 ` Richard Biener
0 siblings, 1 reply; 2+ messages in thread
From: Martin Jambor @ 2017-06-01 11:39 UTC (permalink / raw)
To: GCC Patches; +Cc: Richard Biener
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 <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)
--
2.12.2
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PR 80898] Propagate grp_write from disqualified SRA candidates
2017-06-01 11:39 [PR 80898] Propagate grp_write from disqualified SRA candidates Martin Jambor
@ 2017-06-01 11:52 ` Richard Biener
0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2017-06-01 11:52 UTC (permalink / raw)
To: Martin Jambor; +Cc: GCC Patches
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)
>
--
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-06-01 11:52 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-01 11:39 [PR 80898] Propagate grp_write from disqualified SRA candidates Martin Jambor
2017-06-01 11:52 ` Richard Biener
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).