[C++ PATCH RFC] PR c++/80265 - constexpr __builtin_mem*.

[C++ PATCH RFC] PR c++/80265 - constexpr __builtin_mem*.

[Jason Merrill]

The library has already worked around this issue, but I was curious about
why it wasn't working.  The answer: because we were passing &var to fold,
which doesn't know about the constexpr values hash table.  Fixed by passing
&"str" instead.

Tested x86_64-pc-linux-gnu.  Does this seem useful even though it isn't
necessary anymore?

	* constexpr.c (cxx_eval_builtin_function_call): Expose STRING_CST
	to str/mem builtins.
---
 gcc/cp/constexpr.c                            | 70 +++++++++++++++++--
 gcc/testsuite/g++.dg/ext/constexpr-builtin1.C | 37 ++++++++++
 2 files changed, 102 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/ext/constexpr-builtin1.C

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 3ca3b9ecd65..3a9fb566a52 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -1260,28 +1260,76 @@ cxx_eval_builtin_function_call (const constexpr_ctx *ctx, tree t, tree fun,
   if (fndecl_built_in_p (fun, CP_BUILT_IN_SOURCE_LOCATION, BUILT_IN_FRONTEND))
     return fold_builtin_source_location (EXPR_LOCATION (t));
 
+  int strops = 0;
+  int strret = 0;
+  if (fndecl_built_in_p (fun, BUILT_IN_NORMAL))
+    switch (DECL_FUNCTION_CODE (fun))
+      {
+      case BUILT_IN_STRLEN:
+      case BUILT_IN_STRNLEN:
+	strops = 1;
+	break;
+      case BUILT_IN_MEMCHR:
+      case BUILT_IN_STRCHR:
+      case BUILT_IN_STRRCHR:
+	strops = 1;
+	strret = 1;
+	break;
+      case BUILT_IN_MEMCMP:
+      case BUILT_IN_STRCMP:
+	strops = 2;
+	break;
+      case BUILT_IN_STRSTR:
+	strops = 2;
+	strret = 1;
+      default:
+	break;
+      }
+
   /* Be permissive for arguments to built-ins; __builtin_constant_p should
      return constant false for a non-constant argument.  */
   constexpr_ctx new_ctx = *ctx;
   new_ctx.quiet = true;
   for (i = 0; i < nargs; ++i)
     {
-      args[i] = CALL_EXPR_ARG (t, i);
+      tree arg = CALL_EXPR_ARG (t, i);
+
+      /* To handle string built-ins we need to pass ADDR_EXPR<STRING_CST> since
+	 expand_builtin doesn't know how to look in the values table.  */
+      bool strop = i < strops;
+      if (strop)
+	{
+	  STRIP_NOPS (arg);
+	  if (TREE_CODE (arg) == ADDR_EXPR)
+	    arg = TREE_OPERAND (arg, 0);
+	  else
+	    strop = false;
+	}
+
       /* If builtin_valid_in_constant_expr_p is true,
 	 potential_constant_expression_1 has not recursed into the arguments
 	 of the builtin, verify it here.  */
       if (!builtin_valid_in_constant_expr_p (fun)
-	  || potential_constant_expression (args[i]))
+	  || potential_constant_expression (arg))
 	{
 	  bool dummy1 = false, dummy2 = false;
-	  args[i] = cxx_eval_constant_expression (&new_ctx, args[i], false,
-						  &dummy1, &dummy2);
+	  arg = cxx_eval_constant_expression (&new_ctx, arg, false,
+					      &dummy1, &dummy2);
 	}
 
       if (bi_const_p)
 	/* For __builtin_constant_p, fold all expressions with constant values
 	   even if they aren't C++ constant-expressions.  */
-	args[i] = cp_fold_rvalue (args[i]);
+	arg = cp_fold_rvalue (arg);
+      else if (strop)
+	{
+	  if (TREE_CODE (arg) == CONSTRUCTOR)
+	    arg = braced_lists_to_strings (TREE_TYPE (arg), arg);
+	  if (TREE_CODE (arg) == STRING_CST)
+	    arg = build_address (arg);
+	}
+
+      args[i] = arg;
     }
 
   bool save_ffbcp = force_folding_builtin_constant_p;
@@ -1325,6 +1373,18 @@ cxx_eval_builtin_function_call (const constexpr_ctx *ctx, tree t, tree fun,
       return t;
     }
 
+  if (strret)
+    {
+      /* memchr returns a pointer into the first argument, but we replaced the
+	 argument above with a STRING_CST; put it back it now.  */
+      tree op = CALL_EXPR_ARG (t, strret-1);
+      STRIP_NOPS (new_call);
+      if (TREE_CODE (new_call) == POINTER_PLUS_EXPR)
+	TREE_OPERAND (new_call, 0) = op;
+      else if (TREE_CODE (new_call) == ADDR_EXPR)
+	new_call = op;
+    }
+
   return cxx_eval_constant_expression (&new_ctx, new_call, lval,
 				       non_constant_p, overflow_p);
 }
diff --git a/gcc/testsuite/g++.dg/ext/constexpr-builtin1.C b/gcc/testsuite/g++.dg/ext/constexpr-builtin1.C
new file mode 100644
index 00000000000..2d0904b6e08
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/constexpr-builtin1.C
@@ -0,0 +1,37 @@
+// PR c++/80265
+// { dg-do compile { target c++14 } }
+
+constexpr bool compare() {
+  char s1[] = "foo";
+  char s2[] = "fxo";
+  if (!__builtin_memcmp(s1, s2, 3))
+    return false;
+  s2[1] = 'o';
+  if (__builtin_memcmp(s1, s2, 3))
+    return false;
+  if (__builtin_strcmp(s1, s2))
+    return false;
+  return true;
+}
+
+constexpr bool length() {
+  char s[] = "foo";
+  if (__builtin_strlen(s) != 3)
+    return false;
+  return true;
+}
+
+constexpr bool find() {
+  char s[] = "foo";
+  if (__builtin_memchr(s, 'f', 3) != s)
+    return false;
+  if (__builtin_strchr(s, 'o') != s+1)
+    return false;
+  if (__builtin_strstr(s, "oo") != s+1)
+    return false;
+  return true;
+}
+
+static_assert( compare(), "" );
+static_assert( length(), "" );
+static_assert( find(), "" );

base-commit: d3cf980217ce13b1eda01d4c42a7e3afd2bf3217
-- 
2.18.1

Re: [C++ PATCH RFC] PR c++/80265 - constexpr __builtin_mem*.

[Jonathan Wakely]

On 11/01/20 00:03 -0500, Jason Merrill wrote:
>The library has already worked around this issue, but I was curious about
>why it wasn't working.  The answer: because we were passing &var to fold,
>which doesn't know about the constexpr values hash table.  Fixed by passing
>&"str" instead.
>
>Tested x86_64-pc-linux-gnu.  Does this seem useful even though it isn't
>necessary anymore?

I'd still like to be able to use memcpy and memmove in constexpr
evaluation. We've added our own "std::__memmove" for use at compile
time, but it doesn't really have the same semantics. We could fix
that, but it would be better to just use the real thing.

Re: [C++ PATCH RFC] PR c++/80265 - constexpr __builtin_mem*.

[Jason Merrill]

On Mon, Jan 13, 2020 at 6:11 AM Jonathan Wakely <jwakely@redhat.com> wrote:

> On 11/01/20 00:03 -0500, Jason Merrill wrote:
> >The library has already worked around this issue, but I was curious about
> >why it wasn't working.  The answer: because we were passing &var to fold,
> >which doesn't know about the constexpr values hash table.  Fixed by
> passing
> >&"str" instead.
> >
> >Tested x86_64-pc-linux-gnu.  Does this seem useful even though it isn't
> >necessary anymore?
>
> I'd still like to be able to use memcpy and memmove in constexpr
> evaluation. We've added our own "std::__memmove" for use at compile
> time, but it doesn't really have the same semantics. We could fix
> that, but it would be better to just use the real thing.
>

Ah, supporting memcpy/memmove would be more work than the cmp/chr
functions, I'd have to reimplement them entirely in constexpr.c.

Jason

Re: [C++ PATCH RFC] PR c++/80265 - constexpr __builtin_mem*.

[Jonathan Wakely]

On 13/01/20 12:53 -0500, Jason Merrill wrote:
>On Mon, Jan 13, 2020 at 6:11 AM Jonathan Wakely <jwakely@redhat.com> wrote:
>
>> On 11/01/20 00:03 -0500, Jason Merrill wrote:
>> >The library has already worked around this issue, but I was curious about
>> >why it wasn't working.  The answer: because we were passing &var to fold,
>> >which doesn't know about the constexpr values hash table.  Fixed by
>> passing
>> >&"str" instead.
>> >
>> >Tested x86_64-pc-linux-gnu.  Does this seem useful even though it isn't
>> >necessary anymore?
>>
>> I'd still like to be able to use memcpy and memmove in constexpr
>> evaluation. We've added our own "std::__memmove" for use at compile
>> time, but it doesn't really have the same semantics. We could fix
>> that, but it would be better to just use the real thing.
>>
>
>Ah, supporting memcpy/memmove would be more work than the cmp/chr
>functions, I'd have to reimplement them entirely in constexpr.c.

Ah, OK, nevermind then. We'll reimplement just what we need in the
library.

Re: [C++ PATCH RFC] PR c++/80265 - constexpr __builtin_mem*.

[Jakub Jelinek]

On Mon, Jan 13, 2020 at 06:38:23PM +0000, Jonathan Wakely wrote:
> > > On 11/01/20 00:03 -0500, Jason Merrill wrote:
> > > >The library has already worked around this issue, but I was curious about
> > > >why it wasn't working.  The answer: because we were passing &var to fold,
> > > >which doesn't know about the constexpr values hash table.  Fixed by
> > > passing
> > > >&"str" instead.
> > > >
> > > >Tested x86_64-pc-linux-gnu.  Does this seem useful even though it isn't
> > > >necessary anymore?
> > > 
> > > I'd still like to be able to use memcpy and memmove in constexpr
> > > evaluation. We've added our own "std::__memmove" for use at compile
> > > time, but it doesn't really have the same semantics. We could fix
> > > that, but it would be better to just use the real thing.
> > > 
> > 
> > Ah, supporting memcpy/memmove would be more work than the cmp/chr
> > functions, I'd have to reimplement them entirely in constexpr.c.
> 
> Ah, OK, nevermind then. We'll reimplement just what we need in the
> library.

Guess the hardest part is write helper functions that reads or stores a
single byte at specific address, the builtins then could be easily
implemented by just calling those in a way simplest C implementation of
those functions would do.  But if we have those, we could handle easily many
of the str/mem* functions that way.

	Jakub