public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: "H.J. Lu" <hjl.tools@gmail.com>
To: Richard Biener <richard.guenther@gmail.com>,
	Noah Goldstein <goldstein.w.n@gmail.com>
Cc: "Joseph S. Myers" <joseph@codesourcery.com>,
	GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH] Add -fextra-libc-function=memcmpeq for __memcmpeq
Date: Wed, 15 Jun 2022 14:43:39 -0700	[thread overview]
Message-ID: <CAMe9rOp2oNibD8fWVzJpEiz0RaFoAQ-f1sRmHyOwps5P=GotPw@mail.gmail.com> (raw)
In-Reply-To: <466A0F0B-DB25-4866-AB4B-27992CEF9E2E@gmail.com>

On Mon, Jun 13, 2022 at 9:01 AM Richard Biener
<richard.guenther@gmail.com> wrote:
>
>
>
> > Am 13.06.2022 um 16:36 schrieb H.J. Lu <hjl.tools@gmail.com>:
> >
> > On Mon, Jun 13, 2022 at 3:11 AM Richard Biener
> > <richard.guenther@gmail.com> wrote:
> >>
> >>> On Tue, Jun 7, 2022 at 9:02 PM H.J. Lu via Gcc-patches
> >>> <gcc-patches@gcc.gnu.org> wrote:
> >>>
> >>> Add -fextra-libc-function=memcmpeq to map
> >>>
> >>> extern int __memcmpeq (const void *, const void *, size_t);
> >>>
> >>> which was added to GLIBC 2.35, to __builtin_memcmp_eq.
> >>
> >> Humm.  Can't we instead use the presence of a declaration
> >> of __memcmpeq with a GNU standard dialect as this instead of
> >> adding a weird -fextra-libc-function= option?  Maybe that's even
> >> reasonable with a non-GNU dialect standard in effect since
> >> __ prefixed names are in the implementation namespace?
> >
> > But not all source codes include <string.h> and GCC may generate
> > memcmp directly.  How should we handle these cases?
>
> Not.  Similar as to vectorized math functions.
> I think it’s not worth optimizing for this case.

Another question.  Should we consider any __memcmpeq prototype
or just the one in the system header file?

> Richard.
>
> >
> >> Richard.
> >>
> >>> gcc/
> >>>
> >>>        * builtins.cc: Include "opts.h".
> >>>        (expand_builtin): Generate BUILT_IN_MEMCMP_EQ if __memcmpeq is
> >>>        available.
> >>>        * builtins.def (BUILT_IN___MEMCMPEQ): New.
> >>>        * common.opt: Add -fextra-libc-function=.
> >>>        * opts.cc (extra_libc_functions): New.
> >>>        (parse_extra_libc_function): New function.
> >>>        (common_handle_option): Handle -fextra-libc-function=.
> >>>        * opts.h (extra_libc_function_list): New.
> >>>        (extra_libc_functions): Likewise.
> >>>        * doc/invoke.texi: Document -fextra-libc-function=memcmpeq.
> >>>
> >>> gcc/testsuite/
> >>>
> >>>        * c-c++-common/memcmpeq-1.c: New test.
> >>>        * c-c++-common/memcmpeq-2.c: Likewise.
> >>>        * c-c++-common/memcmpeq-3.c: Likewise.
> >>>        * c-c++-common/memcmpeq-4.c: Likewise.
> >>>        * c-c++-common/memcmpeq-5.c: Likewise.
> >>>        * c-c++-common/memcmpeq-6.c: Likewise.
> >>>        * c-c++-common/memcmpeq-7.c: Likewise.
> >>> ---
> >>> gcc/builtins.cc                         |  5 ++++-
> >>> gcc/builtins.def                        |  4 ++++
> >>> gcc/common.opt                          |  4 ++++
> >>> gcc/doc/invoke.texi                     |  6 ++++++
> >>> gcc/opts.cc                             | 23 +++++++++++++++++++++++
> >>> gcc/opts.h                              |  7 +++++++
> >>> gcc/testsuite/c-c++-common/memcmpeq-1.c | 11 +++++++++++
> >>> gcc/testsuite/c-c++-common/memcmpeq-2.c | 11 +++++++++++
> >>> gcc/testsuite/c-c++-common/memcmpeq-3.c | 11 +++++++++++
> >>> gcc/testsuite/c-c++-common/memcmpeq-4.c | 11 +++++++++++
> >>> gcc/testsuite/c-c++-common/memcmpeq-5.c | 11 +++++++++++
> >>> gcc/testsuite/c-c++-common/memcmpeq-6.c | 11 +++++++++++
> >>> gcc/testsuite/c-c++-common/memcmpeq-7.c | 11 +++++++++++
> >>> 13 files changed, 125 insertions(+), 1 deletion(-)
> >>> create mode 100644 gcc/testsuite/c-c++-common/memcmpeq-1.c
> >>> create mode 100644 gcc/testsuite/c-c++-common/memcmpeq-2.c
> >>> create mode 100644 gcc/testsuite/c-c++-common/memcmpeq-3.c
> >>> create mode 100644 gcc/testsuite/c-c++-common/memcmpeq-4.c
> >>> create mode 100644 gcc/testsuite/c-c++-common/memcmpeq-5.c
> >>> create mode 100644 gcc/testsuite/c-c++-common/memcmpeq-6.c
> >>> create mode 100644 gcc/testsuite/c-c++-common/memcmpeq-7.c
> >>>
> >>> diff --git a/gcc/builtins.cc b/gcc/builtins.cc
> >>> index b9d89b409b8..22269318e8c 100644
> >>> --- a/gcc/builtins.cc
> >>> +++ b/gcc/builtins.cc
> >>> @@ -81,6 +81,7 @@ along with GCC; see the file COPYING3.  If not see
> >>> #include "demangle.h"
> >>> #include "gimple-range.h"
> >>> #include "pointer-query.h"
> >>> +#include "opts.h"
> >>>
> >>> struct target_builtins default_target_builtins;
> >>> #if SWITCHABLE_TARGET
> >>> @@ -7410,7 +7411,9 @@ expand_builtin (tree exp, rtx target, rtx subtarget, machine_mode mode,
> >>>        return target;
> >>>       if (fcode == BUILT_IN_MEMCMP_EQ)
> >>>        {
> >>> -         tree newdecl = builtin_decl_explicit (BUILT_IN_MEMCMP);
> >>> +         tree newdecl = builtin_decl_explicit
> >>> +           (extra_libc_functions.has_memcmpeq
> >>> +            ? BUILT_IN___MEMCMPEQ : BUILT_IN_MEMCMP);
> >>>          TREE_OPERAND (exp, 1) = build_fold_addr_expr (newdecl);
> >>>        }
> >>>       break;
> >>> diff --git a/gcc/builtins.def b/gcc/builtins.def
> >>> index 005976f34e9..eb8d33b16e9 100644
> >>> --- a/gcc/builtins.def
> >>> +++ b/gcc/builtins.def
> >>> @@ -965,6 +965,10 @@ DEF_BUILTIN_STUB (BUILT_IN_ALLOCA_WITH_ALIGN_AND_MAX, "__builtin_alloca_with_ali
> >>>    equality with zero.  */
> >>> DEF_BUILTIN_STUB (BUILT_IN_MEMCMP_EQ, "__builtin_memcmp_eq")
> >>>
> >>> +/* Similar to BUILT_IN_MEMCMP_EQ, but is mapped to __memcmpeq only with
> >>> +   -fextra-libc-function=memcmpeq.  */
> >>> +DEF_EXT_LIB_BUILTIN (BUILT_IN___MEMCMPEQ, "__memcmpeq", BT_FN_INT_CONST_PTR_CONST_PTR_SIZE, ATTR_PURE_NOTHROW_NONNULL_LEAF)
> >>> +
> >>> /* An internal version of strcmp/strncmp, used when the result is only
> >>>    tested for equality with zero.  */
> >>> DEF_BUILTIN_STUB (BUILT_IN_STRCMP_EQ, "__builtin_strcmp_eq")
> >>> diff --git a/gcc/common.opt b/gcc/common.opt
> >>> index 7ca0cceed82..7a7631682b0 100644
> >>> --- a/gcc/common.opt
> >>> +++ b/gcc/common.opt
> >>> @@ -1587,6 +1587,10 @@ Enum(excess_precision) String(standard) Value(EXCESS_PRECISION_STANDARD)
> >>> EnumValue
> >>> Enum(excess_precision) String(16) Value(EXCESS_PRECISION_FLOAT16)
> >>>
> >>> +fextra-libc-function=
> >>> +Common Driver Joined
> >>> +Specify the extra function in the C library.
> >>> +
> >>> ; Whether we permit the extended set of values for FLT_EVAL_METHOD
> >>> ; introduced in ISO/IEC TS 18661-3, or limit ourselves to those in C99/C11.
> >>> fpermitted-flt-eval-methods=
> >>> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> >>> index 8cd5bdddc5d..fe1e3709953 100644
> >>> --- a/gcc/doc/invoke.texi
> >>> +++ b/gcc/doc/invoke.texi
> >>> @@ -676,6 +676,7 @@ Objective-C and Objective-C++ Dialects}.
> >>> -ffixed-@var{reg}  -fexceptions @gol
> >>> -fnon-call-exceptions  -fdelete-dead-exceptions  -funwind-tables @gol
> >>> -fasynchronous-unwind-tables @gol
> >>> +-fextra-libc-function=memcmpeq @gol
> >>> -fno-gnu-unique @gol
> >>> -finhibit-size-directive  -fcommon  -fno-ident @gol
> >>> -fpcc-struct-return  -fpic  -fPIC  -fpie  -fPIE  -fno-plt @gol
> >>> @@ -17250,6 +17251,11 @@ Generate unwind table in DWARF format, if supported by target machine.  The
> >>> table is exact at each instruction boundary, so it can be used for stack
> >>> unwinding from asynchronous events (such as debugger or garbage collector).
> >>>
> >>> +@item -fextra-libc-function=memcmpeq
> >>> +@opindex fextra-libc-function
> >>> +Generate @code{__memcmpeq}, which was added to GLIBC 2.35, for
> >>> +@code{__builtin_memcmp_eq}.
> >>> +
> >>> @item -fno-gnu-unique
> >>> @opindex fno-gnu-unique
> >>> @opindex fgnu-unique
> >>> diff --git a/gcc/opts.cc b/gcc/opts.cc
> >>> index bf06a55456a..e77f0922acc 100644
> >>> --- a/gcc/opts.cc
> >>> +++ b/gcc/opts.cc
> >>> @@ -38,6 +38,9 @@ along with GCC; see the file COPYING3.  If not see
> >>> /* In this file all option sets are explicit.  */
> >>> #undef OPTION_SET_P
> >>>
> >>> +/* The list of extra functions in the C library.  */
> >>> +extra_libc_function_list extra_libc_functions;
> >>> +
> >>> static void set_Wstrict_aliasing (struct gcc_options *opts, int onoff);
> >>>
> >>> /* Names of fundamental debug info formats indexed by enum
> >>> @@ -2269,6 +2272,22 @@ parse_no_sanitize_attribute (char *value)
> >>>   return flags;
> >>> }
> >>>
> >>> +/* Parse -fextra-libc-function= suboptions from ARG.  */
> >>> +
> >>> +static void
> >>> +parse_extra_libc_function (const char *arg)
> >>> +{
> >>> +  /* Check to see if the string matches a sub-option name.  */
> >>> +  if (strcmp (arg, "memcmpeq") == 0)
> >>> +    {
> >>> +      extra_libc_functions.has_memcmpeq = 1;
> >>> +      return;
> >>> +    }
> >>> +
> >>> +  error ("unrecognized argument to %<-fextra-libc-function=%>: %qs",
> >>> +        arg);
> >>> +}
> >>> +
> >>> /* Parse -fzero-call-used-regs suboptions from ARG, return the FLAGS.  */
> >>>
> >>> unsigned int
> >>> @@ -2940,6 +2959,10 @@ common_handle_option (struct gcc_options *opts,
> >>>       SET_OPTION_IF_UNSET (opts, opts_set, flag_profile_correction, value);
> >>>       break;
> >>>
> >>> +    case OPT_fextra_libc_function_:
> >>> +      parse_extra_libc_function (arg);
> >>> +      break;
> >>> +
> >>>     case OPT_fprofile_generate_:
> >>>       opts->x_profile_data_prefix = xstrdup (arg);
> >>>       value = true;
> >>> diff --git a/gcc/opts.h b/gcc/opts.h
> >>> index a43ce66cffe..e8d8835ba23 100644
> >>> --- a/gcc/opts.h
> >>> +++ b/gcc/opts.h
> >>> @@ -345,6 +345,13 @@ struct cl_option_handlers
> >>> extern const char *opt_fstack_limit_symbol_arg;
> >>> extern int opt_fstack_limit_register_no;
> >>>
> >>> +/* The list of extra functions in the C library.  */
> >>> +struct extra_libc_function_list
> >>> +{
> >>> +  unsigned int has_memcmpeq : 1;
> >>> +};
> >>> +extern extra_libc_function_list extra_libc_functions;
> >>> +
> >>> /* Input file names.  */
> >>>
> >>> extern const char **in_fnames;
> >>> diff --git a/gcc/testsuite/c-c++-common/memcmpeq-1.c b/gcc/testsuite/c-c++-common/memcmpeq-1.c
> >>> new file mode 100644
> >>> index 00000000000..487f6cd98db
> >>> --- /dev/null
> >>> +++ b/gcc/testsuite/c-c++-common/memcmpeq-1.c
> >>> @@ -0,0 +1,11 @@
> >>> +/* { dg-do compile } */
> >>> +/* { dg-options "-O2 -fextra-libc-function=memcmpeq" } */
> >>> +/* { dg-final { scan-assembler "__memcmpeq" } } */
> >>> +
> >>> +#include <stddef.h>
> >>> +
> >>> +int
> >>> +foo (const char *s1, const char *s2, size_t len)
> >>> +{
> >>> +  return __builtin_memcmp (s1, s2, len) != 0;
> >>> +}
> >>> diff --git a/gcc/testsuite/c-c++-common/memcmpeq-2.c b/gcc/testsuite/c-c++-common/memcmpeq-2.c
> >>> new file mode 100644
> >>> index 00000000000..1773dc2bee1
> >>> --- /dev/null
> >>> +++ b/gcc/testsuite/c-c++-common/memcmpeq-2.c
> >>> @@ -0,0 +1,11 @@
> >>> +/* { dg-do compile } */
> >>> +/* { dg-options "-O2 -fextra-libc-function=memcmpeq" } */
> >>> +/* { dg-final { scan-assembler "__memcmpeq" } } */
> >>> +
> >>> +#include <string.h>
> >>> +
> >>> +int
> >>> +foo (const char *s1, const char *s2, size_t len)
> >>> +{
> >>> +  return memcmp (s1, s2, len) == 0;
> >>> +}
> >>> diff --git a/gcc/testsuite/c-c++-common/memcmpeq-3.c b/gcc/testsuite/c-c++-common/memcmpeq-3.c
> >>> new file mode 100644
> >>> index 00000000000..69c9537d572
> >>> --- /dev/null
> >>> +++ b/gcc/testsuite/c-c++-common/memcmpeq-3.c
> >>> @@ -0,0 +1,11 @@
> >>> +/* { dg-do compile } */
> >>> +/* { dg-options "-O2 -fextra-libc-function=memcmpeq" } */
> >>> +/* { dg-final { scan-assembler-not "__memcmpeq" } } */
> >>> +
> >>> +#include <string.h>
> >>> +
> >>> +int
> >>> +foo (const char *s1, const char *s2, size_t len)
> >>> +{
> >>> +  return memcmp (s1, s2, len);
> >>> +}
> >>> diff --git a/gcc/testsuite/c-c++-common/memcmpeq-4.c b/gcc/testsuite/c-c++-common/memcmpeq-4.c
> >>> new file mode 100644
> >>> index 00000000000..a448312ea96
> >>> --- /dev/null
> >>> +++ b/gcc/testsuite/c-c++-common/memcmpeq-4.c
> >>> @@ -0,0 +1,11 @@
> >>> +/* { dg-do compile } */
> >>> +/* { dg-options "-O2" } */
> >>> +/* { dg-final { scan-assembler-not "__memcmpeq" } } */
> >>> +
> >>> +#include <stddef.h>
> >>> +
> >>> +int
> >>> +foo (const char *s1, const char *s2, size_t len)
> >>> +{
> >>> +  return __builtin_memcmp (s1, s2, len) != 0;
> >>> +}
> >>> diff --git a/gcc/testsuite/c-c++-common/memcmpeq-5.c b/gcc/testsuite/c-c++-common/memcmpeq-5.c
> >>> new file mode 100644
> >>> index 00000000000..4ef33a1c238
> >>> --- /dev/null
> >>> +++ b/gcc/testsuite/c-c++-common/memcmpeq-5.c
> >>> @@ -0,0 +1,11 @@
> >>> +/* { dg-do compile } */
> >>> +/* { dg-options "-O2" } */
> >>> +/* { dg-final { scan-assembler-not "__memcmpeq" } } */
> >>> +
> >>> +#include <string.h>
> >>> +
> >>> +int
> >>> +foo (const char *s1, const char *s2, size_t len)
> >>> +{
> >>> +  return memcmp (s1, s2, len) == 0;
> >>> +}
> >>> diff --git a/gcc/testsuite/c-c++-common/memcmpeq-6.c b/gcc/testsuite/c-c++-common/memcmpeq-6.c
> >>> new file mode 100644
> >>> index 00000000000..52304df7079
> >>> --- /dev/null
> >>> +++ b/gcc/testsuite/c-c++-common/memcmpeq-6.c
> >>> @@ -0,0 +1,11 @@
> >>> +/* { dg-do compile } */
> >>> +/* { dg-options "-O2 -fextra-libc-function=memcmpeq" } */
> >>> +/* { dg-final { scan-assembler "__memcmpeq" } } */
> >>> +
> >>> +#include <stddef.h>
> >>> +
> >>> +int
> >>> +foo (const char *s1, const char *s2, size_t len)
> >>> +{
> >>> +  return __builtin_memcmp_eq (s1, s2, len) != 0;
> >>> +}
> >>> diff --git a/gcc/testsuite/c-c++-common/memcmpeq-7.c b/gcc/testsuite/c-c++-common/memcmpeq-7.c
> >>> new file mode 100644
> >>> index 00000000000..d59765894e7
> >>> --- /dev/null
> >>> +++ b/gcc/testsuite/c-c++-common/memcmpeq-7.c
> >>> @@ -0,0 +1,11 @@
> >>> +/* { dg-do compile } */
> >>> +/* { dg-options "-O2" } */
> >>> +/* { dg-final { scan-assembler-not "__memcmpeq" } } */
> >>> +
> >>> +#include <stddef.h>
> >>> +
> >>> +int
> >>> +foo (const char *s1, const char *s2, size_t len)
> >>> +{
> >>> +  return __builtin_memcmp_eq (s1, s2, len) != 0;
> >>> +}
> >>> --
> >>> 2.36.1
> >>>
> >
> >
> >
> > --
> > H.J.



-- 
H.J.

  reply	other threads:[~2022-06-15 21:44 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-07 19:01 H.J. Lu
2022-06-13 10:11 ` Richard Biener
2022-06-13 14:36   ` H.J. Lu
2022-06-13 16:01     ` Richard Biener
2022-06-15 21:43       ` H.J. Lu [this message]
2022-06-15 23:38         ` Fangrui Song
2022-06-20  9:39           ` Richard Biener
2022-06-20 15:45             ` H.J. Lu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAMe9rOp2oNibD8fWVzJpEiz0RaFoAQ-f1sRmHyOwps5P=GotPw@mail.gmail.com' \
    --to=hjl.tools@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=goldstein.w.n@gmail.com \
    --cc=joseph@codesourcery.com \
    --cc=richard.guenther@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).