diff mbox series

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

Message ID 20200111050314.27660-1-jason@redhat.com
State New
Headers show
Series [C++,RFC] PR c++/80265 - constexpr __builtin_mem*. | expand

Commit Message

Jason Merrill Jan. 11, 2020, 5:03 a.m. UTC
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


base-commit: d3cf980217ce13b1eda01d4c42a7e3afd2bf3217

Comments

Jonathan Wakely Jan. 13, 2020, 11:11 a.m. UTC | #1
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.
Jason Merrill Jan. 13, 2020, 5:53 p.m. UTC | #2
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
Jonathan Wakely Jan. 13, 2020, 6:38 p.m. UTC | #3
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.
Jakub Jelinek Jan. 13, 2020, 6:43 p.m. UTC | #4
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
diff mbox series

Patch

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(), "" );