[PATCH] -finline-stringops: allow expansion into edges [PR113002]

[PATCH] -finline-stringops: allow expansion into edges [PR113002]

[Alexandre Oliva]

Builtin expanders for memset and memcpy may involve conditionals and
loops, but their sequences may be end up emitted in edges.  Alas,
commit_one_edge_insertion rejects sequences that end with a jump, a
requirement that makes sense for insertions after expand, but not so
much during expand.

During expand, jumps may appear in the middle of the insert sequence
as much as in the end, and it's only after committing edge insertions
out of PHI nodes that we go through the entire function splitting
blocks where needed, so relax the assert in commit_one_edge_insertion
so that jumps are accepted during expand even at the end of the
sequence.

Regstrapping on x86_64-linux-gnu, after testing the testcase with a
cross compiler to ppc64le-linux-gnu.  Ok to install?


Builtin expanders for memset and memcpy may involve conditionals and
loops, but their sequences may be end up emitted in edges.  Alas,
commit_one_edge_insertion rejects sequences that end with a jump, a
requirement that makes sense for insertions after expand, but not so
much during expand.

During expand, jumps may appear in the middle of the insert sequence
as much as in the end, and it's only after committing edge insertions
out of PHI nodes that we go through the entire function splitting
blocks where needed, so relax the assert in commit_one_edge_insertion
so that jumps are accepted during expand even at the end of the
sequence.


for  gcc/ChangeLog

	PR rtl-optimization/113002
	* cfgrtl.cc (commit_one_edge_insertion): Tolerate jumps in the
	inserted sequence during expand.

for  gcc/testsuite/ChangeLog

	PR rtl-optimization/113002
	* gcc.dg/vect/pr113002.c: New.
---
 gcc/cfgrtl.cc                        |    8 +++++++-
 gcc/testsuite/gcc.dg/vect/pr113002.c |   13 +++++++++++++
 2 files changed, 20 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/vect/pr113002.c

diff --git a/gcc/cfgrtl.cc b/gcc/cfgrtl.cc
index 2a3f853eed59b..437eb3a86d5c2 100644
--- a/gcc/cfgrtl.cc
+++ b/gcc/cfgrtl.cc
@@ -2092,7 +2092,13 @@ commit_one_edge_insertion (edge e)
 	delete_insn (before);
     }
   else
-    gcc_assert (!JUMP_P (last));
+    /* Some builtin expanders, such as those for memset and memcpy,
+       may generate loops and conditionals, and those may get emitted
+       into edges.  That's ok while expanding to rtl, basic block
+       boundaries will be identified and split afterwards.  ???  Need
+       we check whether the destination labels of any inserted jumps
+       are also part of the inserted sequence?  */
+    gcc_assert (!JUMP_P (last) || currently_expanding_to_rtl);
 }
 
 /* Update the CFG for all queued instructions.  */
diff --git a/gcc/testsuite/gcc.dg/vect/pr113002.c b/gcc/testsuite/gcc.dg/vect/pr113002.c
new file mode 100644
index 0000000000000..186710f64b422
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vect/pr113002.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target int128 } */
+/* { dg-options "-finline-stringops -Os" } */
+
+typedef __int128 v64u128 __attribute__((vector_size(64)));
+int c;
+v64u128 u;
+void foo() {
+  if (c)
+    u = (v64u128){0};
+  else
+    u = (v64u128){1};
+}

-- 
Alexandre Oliva, happy hacker            https://FSFLA.org/blogs/lxo/
   Free Software Activist                   GNU Toolchain Engineer
More tolerance and less prejudice are key for inclusion and diversity
Excluding neuro-others for not behaving ""normal"" is *not* inclusive

Re: [PATCH] -finline-stringops: allow expansion into edges [PR113002]

[Richard Biener]

On Wed, Dec 20, 2023 at 3:54 AM Alexandre Oliva <oliva@adacore.com> wrote:
>
>
> Builtin expanders for memset and memcpy may involve conditionals and
> loops, but their sequences may be end up emitted in edges.  Alas,
> commit_one_edge_insertion rejects sequences that end with a jump, a
> requirement that makes sense for insertions after expand, but not so
> much during expand.
>
> During expand, jumps may appear in the middle of the insert sequence
> as much as in the end, and it's only after committing edge insertions
> out of PHI nodes that we go through the entire function splitting
> blocks where needed, so relax the assert in commit_one_edge_insertion
> so that jumps are accepted during expand even at the end of the
> sequence.
>
> Regstrapping on x86_64-linux-gnu, after testing the testcase with a
> cross compiler to ppc64le-linux-gnu.  Ok to install?

OK.

>
> Builtin expanders for memset and memcpy may involve conditionals and
> loops, but their sequences may be end up emitted in edges.  Alas,
> commit_one_edge_insertion rejects sequences that end with a jump, a
> requirement that makes sense for insertions after expand, but not so
> much during expand.
>
> During expand, jumps may appear in the middle of the insert sequence
> as much as in the end, and it's only after committing edge insertions
> out of PHI nodes that we go through the entire function splitting
> blocks where needed, so relax the assert in commit_one_edge_insertion
> so that jumps are accepted during expand even at the end of the
> sequence.
>
>
> for  gcc/ChangeLog
>
>         PR rtl-optimization/113002
>         * cfgrtl.cc (commit_one_edge_insertion): Tolerate jumps in the
>         inserted sequence during expand.
>
> for  gcc/testsuite/ChangeLog
>
>         PR rtl-optimization/113002
>         * gcc.dg/vect/pr113002.c: New.
> ---
>  gcc/cfgrtl.cc                        |    8 +++++++-
>  gcc/testsuite/gcc.dg/vect/pr113002.c |   13 +++++++++++++
>  2 files changed, 20 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/gcc.dg/vect/pr113002.c
>
> diff --git a/gcc/cfgrtl.cc b/gcc/cfgrtl.cc
> index 2a3f853eed59b..437eb3a86d5c2 100644
> --- a/gcc/cfgrtl.cc
> +++ b/gcc/cfgrtl.cc
> @@ -2092,7 +2092,13 @@ commit_one_edge_insertion (edge e)
>         delete_insn (before);
>      }
>    else
> -    gcc_assert (!JUMP_P (last));
> +    /* Some builtin expanders, such as those for memset and memcpy,
> +       may generate loops and conditionals, and those may get emitted
> +       into edges.  That's ok while expanding to rtl, basic block
> +       boundaries will be identified and split afterwards.  ???  Need
> +       we check whether the destination labels of any inserted jumps
> +       are also part of the inserted sequence?  */
> +    gcc_assert (!JUMP_P (last) || currently_expanding_to_rtl);
>  }
>
>  /* Update the CFG for all queued instructions.  */
> diff --git a/gcc/testsuite/gcc.dg/vect/pr113002.c b/gcc/testsuite/gcc.dg/vect/pr113002.c
> new file mode 100644
> index 0000000000000..186710f64b422
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/vect/pr113002.c
> @@ -0,0 +1,13 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target int128 } */
> +/* { dg-options "-finline-stringops -Os" } */
> +
> +typedef __int128 v64u128 __attribute__((vector_size(64)));
> +int c;
> +v64u128 u;
> +void foo() {
> +  if (c)
> +    u = (v64u128){0};
> +  else
> +    u = (v64u128){1};
> +}
>
> --
> Alexandre Oliva, happy hacker            https://FSFLA.org/blogs/lxo/
>    Free Software Activist                   GNU Toolchain Engineer
> More tolerance and less prejudice are key for inclusion and diversity
> Excluding neuro-others for not behaving ""normal"" is *not* inclusive