diff mbox series

bpf: add size threshold for inlining mem builtins

Message ID 20240307175438.25468-1-david.faust@oracle.com
State New
Headers show
Series bpf: add size threshold for inlining mem builtins | expand

Commit Message

David Faust March 7, 2024, 5:54 p.m. UTC 
BPF cannot fall back on library calls to implement memmove, memcpy and
memset, so we attempt to expand these inline always if possible.
However, this inline expansion was being attempted even for excessively
large operations, which could result in gcc consuming huge amounts of
memory and hanging.

Add a size threshold in the BPF backend below which to always expand
these operations inline, and introduce an option
-minline-memops-threshold= to control the threshold. Defaults to
1024 bytes.

Tested on x86_64-linux-gnu host for bpf-unknown-none target.
Fixes hang in test gcc.c-torture/compile/20050622-1.c for BPF, which
returns to (correctly) failing due to exceeding the eBPF stack limit.

gcc/

	* config/bpf/bpf.cc (bpf_expand_cpymem, bpf_expand_setmem): Do
	not attempt inline expansion if size is above threshold.
	* config/bpf/bpf.opt (-minline-memops-threshold): New option.
	* doc/invoke.texi (eBPF Options) <-minline-memops-threshold>:
	Document.

gcc/testsuite/

	* gcc.target/bpf/inline-memops-threshold-1.c: New test.
	* gcc.target/bpf/inline-memops-threshold-2.c: New test.
---
 gcc/config/bpf/bpf.cc                             |  8 ++++++++
 gcc/config/bpf/bpf.opt                            |  4 ++++
 gcc/doc/invoke.texi                               |  9 ++++++++-
 .../gcc.target/bpf/inline-memops-threshold-1.c    | 15 +++++++++++++++
 .../gcc.target/bpf/inline-memops-threshold-2.c    | 14 ++++++++++++++
 5 files changed, 49 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.target/bpf/inline-memops-threshold-1.c
 create mode 100644 gcc/testsuite/gcc.target/bpf/inline-memops-threshold-2.c

Comments

Jose E. Marchesi March 8, 2024, 8:16 a.m. UTC | #1 
Hi Faust.

> BPF cannot fall back on library calls to implement memmove, memcpy and
> memset, so we attempt to expand these inline always if possible.
> However, this inline expansion was being attempted even for excessively
> large operations, which could result in gcc consuming huge amounts of
> memory and hanging.
>
> Add a size threshold in the BPF backend below which to always expand
> these operations inline, and introduce an option
> -minline-memops-threshold= to control the threshold. Defaults to
> 1024 bytes.
>
> Tested on x86_64-linux-gnu host for bpf-unknown-none target.
> Fixes hang in test gcc.c-torture/compile/20050622-1.c for BPF, which
> returns to (correctly) failing due to exceeding the eBPF stack limit.
>
> gcc/
>
> 	* config/bpf/bpf.cc (bpf_expand_cpymem, bpf_expand_setmem): Do
> 	not attempt inline expansion if size is above threshold.
> 	* config/bpf/bpf.opt (-minline-memops-threshold): New option.
> 	* doc/invoke.texi (eBPF Options) <-minline-memops-threshold>:
> 	Document.
>
> gcc/testsuite/
>
> 	* gcc.target/bpf/inline-memops-threshold-1.c: New test.
> 	* gcc.target/bpf/inline-memops-threshold-2.c: New test.
> ---
>  gcc/config/bpf/bpf.cc                             |  8 ++++++++
>  gcc/config/bpf/bpf.opt                            |  4 ++++
>  gcc/doc/invoke.texi                               |  9 ++++++++-
>  .../gcc.target/bpf/inline-memops-threshold-1.c    | 15 +++++++++++++++
>  .../gcc.target/bpf/inline-memops-threshold-2.c    | 14 ++++++++++++++
>  5 files changed, 49 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/gcc.target/bpf/inline-memops-threshold-1.c
>  create mode 100644 gcc/testsuite/gcc.target/bpf/inline-memops-threshold-2.c
>
> diff --git a/gcc/config/bpf/bpf.cc b/gcc/config/bpf/bpf.cc
> index 0e33f4347ba..3f3dcb0a46b 100644
> --- a/gcc/config/bpf/bpf.cc
> +++ b/gcc/config/bpf/bpf.cc
> @@ -1275,6 +1275,10 @@ bpf_expand_cpymem (rtx *operands, bool is_move)
>        gcc_unreachable ();
>      }
>  
> +  /* For sizes above threshold, always use a libcall.  */
> +  if (size_bytes > (unsigned HOST_WIDE_INT) bpf_inline_memops_threshold)
> +    return false;

Shouldn't we emit warning or error in these cases, like in the other
situations that prevent inlining?  Something like:

  if (flag_building_libgcc)
    warning (0, "could not inline call to %<__builtin_%s%>: "
             "too many bytes, use -minline-memops-threshold", name);
  else
    error ("could not inline call to %<__builtin_%s%>: "
           "too many bytes, use -minline-memops-threshold", name);

> +
>    unsigned iters = size_bytes >> ceil_log2 (align);
>    unsigned remainder = size_bytes & (align - 1);
>  
> @@ -1347,6 +1351,10 @@ bpf_expand_setmem (rtx *operands)
>        gcc_unreachable ();
>      }
>  
> +  /* For sizes above threshold, always use a libcall.  */
> +  if (size_bytes > (unsigned HOST_WIDE_INT) bpf_inline_memops_threshold)
> +    return false;
> +
>    unsigned iters = size_bytes >> ceil_log2 (align);
>    unsigned remainder = size_bytes & (align - 1);
>    unsigned inc = GET_MODE_SIZE (mode);
> diff --git a/gcc/config/bpf/bpf.opt b/gcc/config/bpf/bpf.opt
> index acfddebdad7..541ebe4dfc4 100644
> --- a/gcc/config/bpf/bpf.opt
> +++ b/gcc/config/bpf/bpf.opt
> @@ -108,3 +108,7 @@ Enum(asm_dialect) String(normal) Value(ASM_NORMAL)
>  
>  EnumValue
>  Enum(asm_dialect) String(pseudoc) Value(ASM_PSEUDOC)
> +
> +minline-memops-threshold=
> +Target RejectNegative Joined UInteger Var(bpf_inline_memops_threshold) Init(1024)
> +-minline-memops-threshold=<number> Maximum size of memset/memmove/memcpy to inline, larger sizes will use a library call.
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 2390d478121..7a965631123 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -971,7 +971,7 @@ Objective-C and Objective-C++ Dialects}.
>  @gccoptlist{-mbig-endian -mlittle-endian
>  -mframe-limit=@var{bytes} -mxbpf -mco-re -mno-co-re -mjmpext
>  -mjmp32 -malu32 -mv3-atomics -mbswap -msdiv -msmov -mcpu=@var{version}
> --masm=@var{dialect}}
> +-masm=@var{dialect} -minline-memops-threshold=@var{bytes}}
>  
>  @emph{FR30 Options}
>  @gccoptlist{-msmall-model  -mno-lsim}
> @@ -25700,6 +25700,13 @@ Outputs pseudo-c assembly dialect.
>  
>  @end table
>  
> +@opindex -minline-memops-threshold
> +@item -minline-memops-threshold=@var{bytes}
> +Specifies a size threshold in bytes at or below which memmove, memcpy
> +and memset shall always be expanded inline.  Operations dealing with
> +sizes larger than this threshold will be implemented using a library
> +call instead of being expanded inline.  The default is @samp{1024}.

"[...] would have to be implemented using a library call instead of
 being expanded inline, but since BPF doesn't allow libcalls exceeding
 this threshold results in a compile-time error."

> +
>  @end table
>  
>  @node FR30 Options
> diff --git a/gcc/testsuite/gcc.target/bpf/inline-memops-threshold-1.c b/gcc/testsuite/gcc.target/bpf/inline-memops-threshold-1.c
> new file mode 100644
> index 00000000000..c2ba4db5b7b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/bpf/inline-memops-threshold-1.c
> @@ -0,0 +1,15 @@
> +
> +/* { dg-do compile } */
> +/* { dg-options "-O2" "-minline-memops-threshold=256"} */
> +
> +char buf[512];
> +
> +void
> +mov_small (void)
> +{
> +  __builtin_memmove (buf, buf + 2, 255);
> +}
> +
> +/* { dg-final { scan-assembler-not "call" } } */
> +/* { dg-final { scan-assembler "ldxb" } } */
> +/* { dg-final { scan-assembler "stxb" } } */
> diff --git a/gcc/testsuite/gcc.target/bpf/inline-memops-threshold-2.c b/gcc/testsuite/gcc.target/bpf/inline-memops-threshold-2.c
> new file mode 100644
> index 00000000000..190b10b579c
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/bpf/inline-memops-threshold-2.c
> @@ -0,0 +1,14 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -minline-memops-threshold=256" } */
> +
> +char buf[512];
> +
> +void
> +mov_big (void)
> +{
> +  __builtin_memmove (buf, buf + 12, 354);
> +}
> +
> +/* { dg-final { scan-assembler-times "call" 1 } } */
> +/* { dg-final { scan-assembler-not "ldxb" } } */
> +/* { dg-final { scan-assembler-not "stxb" } } */
diff mbox series

Patch

diff --git a/gcc/config/bpf/bpf.cc b/gcc/config/bpf/bpf.cc
index 0e33f4347ba..3f3dcb0a46b 100644
--- a/gcc/config/bpf/bpf.cc
+++ b/gcc/config/bpf/bpf.cc
@@ -1275,6 +1275,10 @@  bpf_expand_cpymem (rtx *operands, bool is_move)
       gcc_unreachable ();
     }
 
+  /* For sizes above threshold, always use a libcall.  */
+  if (size_bytes > (unsigned HOST_WIDE_INT) bpf_inline_memops_threshold)
+    return false;
+
   unsigned iters = size_bytes >> ceil_log2 (align);
   unsigned remainder = size_bytes & (align - 1);
 
@@ -1347,6 +1351,10 @@  bpf_expand_setmem (rtx *operands)
       gcc_unreachable ();
     }
 
+  /* For sizes above threshold, always use a libcall.  */
+  if (size_bytes > (unsigned HOST_WIDE_INT) bpf_inline_memops_threshold)
+    return false;
+
   unsigned iters = size_bytes >> ceil_log2 (align);
   unsigned remainder = size_bytes & (align - 1);
   unsigned inc = GET_MODE_SIZE (mode);
diff --git a/gcc/config/bpf/bpf.opt b/gcc/config/bpf/bpf.opt
index acfddebdad7..541ebe4dfc4 100644
--- a/gcc/config/bpf/bpf.opt
+++ b/gcc/config/bpf/bpf.opt
@@ -108,3 +108,7 @@  Enum(asm_dialect) String(normal) Value(ASM_NORMAL)
 
 EnumValue
 Enum(asm_dialect) String(pseudoc) Value(ASM_PSEUDOC)
+
+minline-memops-threshold=
+Target RejectNegative Joined UInteger Var(bpf_inline_memops_threshold) Init(1024)
+-minline-memops-threshold=<number> Maximum size of memset/memmove/memcpy to inline, larger sizes will use a library call.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 2390d478121..7a965631123 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -971,7 +971,7 @@  Objective-C and Objective-C++ Dialects}.
 @gccoptlist{-mbig-endian -mlittle-endian
 -mframe-limit=@var{bytes} -mxbpf -mco-re -mno-co-re -mjmpext
 -mjmp32 -malu32 -mv3-atomics -mbswap -msdiv -msmov -mcpu=@var{version}
--masm=@var{dialect}}
+-masm=@var{dialect} -minline-memops-threshold=@var{bytes}}
 
 @emph{FR30 Options}
 @gccoptlist{-msmall-model  -mno-lsim}
@@ -25700,6 +25700,13 @@  Outputs pseudo-c assembly dialect.
 
 @end table
 
+@opindex -minline-memops-threshold
+@item -minline-memops-threshold=@var{bytes}
+Specifies a size threshold in bytes at or below which memmove, memcpy
+and memset shall always be expanded inline.  Operations dealing with
+sizes larger than this threshold will be implemented using a library
+call instead of being expanded inline.  The default is @samp{1024}.
+
 @end table
 
 @node FR30 Options
diff --git a/gcc/testsuite/gcc.target/bpf/inline-memops-threshold-1.c b/gcc/testsuite/gcc.target/bpf/inline-memops-threshold-1.c
new file mode 100644
index 00000000000..c2ba4db5b7b
--- /dev/null
+++ b/gcc/testsuite/gcc.target/bpf/inline-memops-threshold-1.c
@@ -0,0 +1,15 @@ 
+
+/* { dg-do compile } */
+/* { dg-options "-O2" "-minline-memops-threshold=256"} */
+
+char buf[512];
+
+void
+mov_small (void)
+{
+  __builtin_memmove (buf, buf + 2, 255);
+}
+
+/* { dg-final { scan-assembler-not "call" } } */
+/* { dg-final { scan-assembler "ldxb" } } */
+/* { dg-final { scan-assembler "stxb" } } */
diff --git a/gcc/testsuite/gcc.target/bpf/inline-memops-threshold-2.c b/gcc/testsuite/gcc.target/bpf/inline-memops-threshold-2.c
new file mode 100644
index 00000000000..190b10b579c
--- /dev/null
+++ b/gcc/testsuite/gcc.target/bpf/inline-memops-threshold-2.c
@@ -0,0 +1,14 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -minline-memops-threshold=256" } */
+
+char buf[512];
+
+void
+mov_big (void)
+{
+  __builtin_memmove (buf, buf + 12, 354);
+}
+
+/* { dg-final { scan-assembler-times "call" 1 } } */
+/* { dg-final { scan-assembler-not "ldxb" } } */
+/* { dg-final { scan-assembler-not "stxb" } } */