* [PATCH] middle-end: Fix dominator information with loop duplication PR114197
@ 2024-03-01 22:14 Edwin Lu
2024-03-04 10:42 ` Richard Biener
0 siblings, 1 reply; 2+ messages in thread
From: Edwin Lu @ 2024-03-01 22:14 UTC (permalink / raw)
To: gcc-patches; +Cc: gnu-toolchain, Edwin Lu
When adding the new_preheader to the cfg, only the new_preheader's dominator
information is updated. If one of the new basic block's children was part
of the original cfg and adding new_preheader to the cfg introduces another path
to that child, the child's dominator information will not be updated. This may
cause verify_dominator's assertion to fail.
Force recalculating dominators for all duplicated basic blocks and their
successors when updating new_preheader's dominator information.
PR 114197
gcc/ChangeLog:
* tree-vect-loop-manip.cc (slpeel_tree_duplicate_loop_to_edge_cfg):
Recalculate dominator info when adding new_preheader to cfg
gcc/testsuite/ChangeLog:
* gcc.dg/vect/pr114197.c: New test.
Signed-off-by: Edwin Lu <ewlu@rivosinc.com>
---
gcc/testsuite/gcc.dg/vect/pr114197.c | 18 ++++++++++++++++++
gcc/tree-vect-loop-manip.cc | 17 ++++++++++++++++-
2 files changed, 34 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.dg/vect/pr114197.c
diff --git a/gcc/testsuite/gcc.dg/vect/pr114197.c b/gcc/testsuite/gcc.dg/vect/pr114197.c
new file mode 100644
index 00000000000..b1fb807729c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vect/pr114197.c
@@ -0,0 +1,18 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-O3" } */
+
+
+#pragma pack(push)
+struct a {
+ volatile signed b : 8;
+};
+#pragma pack(pop)
+int c;
+static struct a d = {5};
+void e() {
+f:
+ for (c = 8; c < 55; ++c)
+ if (!d.b)
+ goto f;
+}
+
diff --git a/gcc/tree-vect-loop-manip.cc b/gcc/tree-vect-loop-manip.cc
index f72da915103..0f3a489e78c 100644
--- a/gcc/tree-vect-loop-manip.cc
+++ b/gcc/tree-vect-loop-manip.cc
@@ -1840,7 +1840,22 @@ slpeel_tree_duplicate_loop_to_edge_cfg (class loop *loop, edge loop_exit,
}
if (was_imm_dom || duplicate_outer_loop)
- set_immediate_dominator (CDI_DOMINATORS, exit_dest, new_exit->src);
+ {
+ set_immediate_dominator (CDI_DOMINATORS, exit_dest, new_exit->src);
+
+ /* Update the dominator info for children of duplicated bbs. */
+ for (unsigned i = 0; i < scalar_loop->num_nodes; i++)
+ {
+ basic_block dom_bb = NULL;
+ edge e;
+ edge_iterator ei;
+ FOR_EACH_EDGE (e, ei, new_bbs[i]->succs)
+ {
+ dom_bb = recompute_dominator (CDI_DOMINATORS, e->dest);
+ set_immediate_dominator (CDI_DOMINATORS, e->dest, dom_bb);
+ }
+ }
+ }
/* And remove the non-necessary forwarder again. Keep the other
one so we have a proper pre-header for the loop at the exit edge. */
--
2.34.1
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] middle-end: Fix dominator information with loop duplication PR114197
2024-03-01 22:14 [PATCH] middle-end: Fix dominator information with loop duplication PR114197 Edwin Lu
@ 2024-03-04 10:42 ` Richard Biener
0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2024-03-04 10:42 UTC (permalink / raw)
To: Edwin Lu; +Cc: gcc-patches, gnu-toolchain
On Fri, Mar 1, 2024 at 11:16 PM Edwin Lu <ewlu@rivosinc.com> wrote:
>
> When adding the new_preheader to the cfg, only the new_preheader's dominator
> information is updated. If one of the new basic block's children was part
> of the original cfg and adding new_preheader to the cfg introduces another path
> to that child, the child's dominator information will not be updated. This may
> cause verify_dominator's assertion to fail.
>
> Force recalculating dominators for all duplicated basic blocks and their
> successors when updating new_preheader's dominator information.
We're already doing this (which IMO is bad), via the
iterate_fix_dominators call.
You're adding another bunch of similar things and I think the use of
recompute_dominator isn't safe as it assumes all predecessors have valid
dominator info.
I'll have a look.
Richard.
> PR 114197
>
> gcc/ChangeLog:
>
> * tree-vect-loop-manip.cc (slpeel_tree_duplicate_loop_to_edge_cfg):
> Recalculate dominator info when adding new_preheader to cfg
>
> gcc/testsuite/ChangeLog:
>
> * gcc.dg/vect/pr114197.c: New test.
>
> Signed-off-by: Edwin Lu <ewlu@rivosinc.com>
> ---
> gcc/testsuite/gcc.dg/vect/pr114197.c | 18 ++++++++++++++++++
> gcc/tree-vect-loop-manip.cc | 17 ++++++++++++++++-
> 2 files changed, 34 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/gcc.dg/vect/pr114197.c
>
> diff --git a/gcc/testsuite/gcc.dg/vect/pr114197.c b/gcc/testsuite/gcc.dg/vect/pr114197.c
> new file mode 100644
> index 00000000000..b1fb807729c
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/vect/pr114197.c
> @@ -0,0 +1,18 @@
> +/* { dg-do compile } */
> +/* { dg-additional-options "-O3" } */
> +
> +
> +#pragma pack(push)
> +struct a {
> + volatile signed b : 8;
> +};
> +#pragma pack(pop)
> +int c;
> +static struct a d = {5};
> +void e() {
> +f:
> + for (c = 8; c < 55; ++c)
> + if (!d.b)
> + goto f;
> +}
> +
> diff --git a/gcc/tree-vect-loop-manip.cc b/gcc/tree-vect-loop-manip.cc
> index f72da915103..0f3a489e78c 100644
> --- a/gcc/tree-vect-loop-manip.cc
> +++ b/gcc/tree-vect-loop-manip.cc
> @@ -1840,7 +1840,22 @@ slpeel_tree_duplicate_loop_to_edge_cfg (class loop *loop, edge loop_exit,
> }
>
> if (was_imm_dom || duplicate_outer_loop)
> - set_immediate_dominator (CDI_DOMINATORS, exit_dest, new_exit->src);
> + {
> + set_immediate_dominator (CDI_DOMINATORS, exit_dest, new_exit->src);
> +
> + /* Update the dominator info for children of duplicated bbs. */
> + for (unsigned i = 0; i < scalar_loop->num_nodes; i++)
> + {
> + basic_block dom_bb = NULL;
> + edge e;
> + edge_iterator ei;
> + FOR_EACH_EDGE (e, ei, new_bbs[i]->succs)
> + {
> + dom_bb = recompute_dominator (CDI_DOMINATORS, e->dest);
> + set_immediate_dominator (CDI_DOMINATORS, e->dest, dom_bb);
> + }
> + }
> + }
>
> /* And remove the non-necessary forwarder again. Keep the other
> one so we have a proper pre-header for the loop at the exit edge. */
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-03-04 10:42 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-01 22:14 [PATCH] middle-end: Fix dominator information with loop duplication PR114197 Edwin Lu
2024-03-04 10:42 ` 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).