diff mbox

Readd strchr constant folding (PR c++/71537)

Message ID 20161205165448.GO3541@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Dec. 5, 2016, 4:54 p.m. UTC 
Hi!

The recent changes to move strchr folding from builtins.c to gimple-fold.c
broke constexpr handling with __builtin_strchr etc. (which the libstdc++
folks want to use).

Fixed by handling it also in fold-const-call.c.  Bootstrapped/regtested on
x86_64-linux and i686-linux, ok for trunk?

2016-12-05  Jakub Jelinek  <jakub@redhat.com>

	PR c++/71537
	* fold-const-call.c (fold_const_call): Handle
	CFN_BUILT_IN_{INDEX,STRCHR,RINDEX,STRRCHR}.

	* g++.dg/cpp0x/constexpr-strchr.C: New test.


	Jakub

Comments

Jeff Law Dec. 5, 2016, 5:55 p.m. UTC | #1 
On 12/05/2016 09:54 AM, Jakub Jelinek wrote:
> Hi!
>
> The recent changes to move strchr folding from builtins.c to gimple-fold.c
> broke constexpr handling with __builtin_strchr etc. (which the libstdc++
> folks want to use).
>
> Fixed by handling it also in fold-const-call.c.  Bootstrapped/regtested on
> x86_64-linux and i686-linux, ok for trunk?
>
> 2016-12-05  Jakub Jelinek  <jakub@redhat.com>
>
> 	PR c++/71537
> 	* fold-const-call.c (fold_const_call): Handle
> 	CFN_BUILT_IN_{INDEX,STRCHR,RINDEX,STRRCHR}.
>
> 	* g++.dg/cpp0x/constexpr-strchr.C: New test.
Thoughts on moving this into match.pd?  I don't see any string builtins 
there, so perhaps leave it in fold-const-call for now, then move them as 
a group later?


jeff
Jakub Jelinek Dec. 5, 2016, 5:59 p.m. UTC | #2 
On Mon, Dec 05, 2016 at 10:55:05AM -0700, Jeff Law wrote:
> On 12/05/2016 09:54 AM, Jakub Jelinek wrote:
> >The recent changes to move strchr folding from builtins.c to gimple-fold.c
> >broke constexpr handling with __builtin_strchr etc. (which the libstdc++
> >folks want to use).
> >
> >Fixed by handling it also in fold-const-call.c.  Bootstrapped/regtested on
> >x86_64-linux and i686-linux, ok for trunk?
> >
> >2016-12-05  Jakub Jelinek  <jakub@redhat.com>
> >
> >	PR c++/71537
> >	* fold-const-call.c (fold_const_call): Handle
> >	CFN_BUILT_IN_{INDEX,STRCHR,RINDEX,STRRCHR}.
> >
> >	* g++.dg/cpp0x/constexpr-strchr.C: New test.
> Thoughts on moving this into match.pd?  I don't see any string builtins
> there, so perhaps leave it in fold-const-call for now, then move them as a
> group later?

At least my understanding has been that such stuff goes into
fold-const-call.c in the new world.  The GIMPLE optimizers for these functions
have also been added to gimple-fold.c, not to match.pd.

	Jakub
Jeff Law Dec. 5, 2016, 6:02 p.m. UTC | #3 
On 12/05/2016 10:59 AM, Jakub Jelinek wrote:
> On Mon, Dec 05, 2016 at 10:55:05AM -0700, Jeff Law wrote:
>> On 12/05/2016 09:54 AM, Jakub Jelinek wrote:
>>> The recent changes to move strchr folding from builtins.c to gimple-fold.c
>>> broke constexpr handling with __builtin_strchr etc. (which the libstdc++
>>> folks want to use).
>>>
>>> Fixed by handling it also in fold-const-call.c.  Bootstrapped/regtested on
>>> x86_64-linux and i686-linux, ok for trunk?
>>>
>>> 2016-12-05  Jakub Jelinek  <jakub@redhat.com>
>>>
>>> 	PR c++/71537
>>> 	* fold-const-call.c (fold_const_call): Handle
>>> 	CFN_BUILT_IN_{INDEX,STRCHR,RINDEX,STRRCHR}.
>>>
>>> 	* g++.dg/cpp0x/constexpr-strchr.C: New test.
>> Thoughts on moving this into match.pd?  I don't see any string builtins
>> there, so perhaps leave it in fold-const-call for now, then move them as a
>> group later?
>
> At least my understanding has been that such stuff goes into
> fold-const-call.c in the new world.  The GIMPLE optimizers for these functions
> have also been added to gimple-fold.c, not to match.pd.
Good enough for me...

OK for the trunk, if I wasn't clear about that..

jeff
Richard Biener Dec. 6, 2016, 8:15 a.m. UTC | #4 
On Mon, 5 Dec 2016, Jakub Jelinek wrote:

> On Mon, Dec 05, 2016 at 10:55:05AM -0700, Jeff Law wrote:
> > On 12/05/2016 09:54 AM, Jakub Jelinek wrote:
> > >The recent changes to move strchr folding from builtins.c to gimple-fold.c
> > >broke constexpr handling with __builtin_strchr etc. (which the libstdc++
> > >folks want to use).
> > >
> > >Fixed by handling it also in fold-const-call.c.  Bootstrapped/regtested on
> > >x86_64-linux and i686-linux, ok for trunk?
> > >
> > >2016-12-05  Jakub Jelinek  <jakub@redhat.com>
> > >
> > >	PR c++/71537
> > >	* fold-const-call.c (fold_const_call): Handle
> > >	CFN_BUILT_IN_{INDEX,STRCHR,RINDEX,STRRCHR}.
> > >
> > >	* g++.dg/cpp0x/constexpr-strchr.C: New test.
> > Thoughts on moving this into match.pd?  I don't see any string builtins
> > there, so perhaps leave it in fold-const-call for now, then move them as a
> > group later?
> 
> At least my understanding has been that such stuff goes into
> fold-const-call.c in the new world.  The GIMPLE optimizers for these functions
> have also been added to gimple-fold.c, not to match.pd.

Yes, constant folding goes to fold-const-call.c and folding to 
gimple-fold.c.  In theory match.pd would be an option but I need to
sit down and invent a syntax for properly handling virtual operands
(very few special cases would already work automagically but in general
you need to be explicit).

So for now, fold-const-call.c and gimple-fold.c.

Richard.
diff mbox

Patch

--- gcc/fold-const-call.c.jj	2016-11-09 18:54:03.000000000 +0100
+++ gcc/fold-const-call.c	2016-12-05 12:53:38.597090946 +0100
@@ -1383,6 +1383,7 @@  tree
 fold_const_call (combined_fn fn, tree type, tree arg0, tree arg1)
 {
   const char *p0, *p1;
+  char c;
   switch (fn)
     {
     case CFN_BUILT_IN_STRSPN:
@@ -1409,6 +1410,30 @@  fold_const_call (combined_fn fn, tree ty
 	}
       return NULL_TREE;
 
+    case CFN_BUILT_IN_INDEX:
+    case CFN_BUILT_IN_STRCHR:
+      if ((p0 = c_getstr (arg0)) && target_char_cst_p (arg1, &c))
+	{
+	  const char *r = strchr (p0, c);
+	  if (r == NULL)
+	    return build_int_cst (type, 0);
+	  return fold_convert (type,
+			       fold_build_pointer_plus_hwi (arg0, r - p0));
+	}
+      return NULL_TREE;
+
+    case CFN_BUILT_IN_RINDEX:
+    case CFN_BUILT_IN_STRRCHR:
+      if ((p0 = c_getstr (arg0)) && target_char_cst_p (arg1, &c))
+	{
+	  const char *r = strrchr (p0, c);
+	  if (r == NULL)
+	    return build_int_cst (type, 0);
+	  return fold_convert (type,
+			       fold_build_pointer_plus_hwi (arg0, r - p0));
+	}
+      return NULL_TREE;
+
     default:
       return fold_const_call_1 (fn, type, arg0, arg1);
     }
--- gcc/testsuite/g++.dg/cpp0x/constexpr-strchr.C.jj	2016-12-05 13:00:19.448101292 +0100
+++ gcc/testsuite/g++.dg/cpp0x/constexpr-strchr.C	2016-12-05 13:00:36.270888812 +0100
@@ -0,0 +1,27 @@ 
+// { dg-do compile { target c++11 } }
+
+constexpr const char *f1 (const char *p, int q) { return __builtin_strchr (p, q); }
+constexpr const char *f2 (const char *p, int q) { return __builtin_index (p, q); }
+constexpr const char *f3 (const char *p, int q) { return __builtin_strrchr (p, q); }
+constexpr const char *f4 (const char *p, int q) { return __builtin_rindex (p, q); }
+constexpr const char a[] = "abcdefedcba";
+static_assert (f1 ("abcde", 'f') == nullptr, "");
+static_assert (f1 (a, 'g') == nullptr, "");
+static_assert (f1 (a, 'f') == a + 5, "");
+static_assert (f1 (a, 'c') == a + 2, "");
+static_assert (f1 (a, '\0') == a + 11, "");
+static_assert (f2 ("abcde", 'f') == nullptr, "");
+static_assert (f2 (a, 'g') == nullptr, "");
+static_assert (f2 (a, 'f') == a + 5, "");
+static_assert (f2 (a, 'c') == a + 2, "");
+static_assert (f2 (a, '\0') == a + 11, "");
+static_assert (f3 ("abcde", 'f') == nullptr, "");
+static_assert (f3 (a, 'g') == nullptr, "");
+static_assert (f3 (a, 'f') == a + 5, "");
+static_assert (f3 (a, 'c') == a + 8, "");
+static_assert (f3 (a, '\0') == a + 11, "");
+static_assert (f4 ("abcde", 'f') == nullptr, "");
+static_assert (f4 (a, 'g') == nullptr, "");
+static_assert (f4 (a, 'f') == a + 5, "");
+static_assert (f4 (a, 'c') == a + 8, "");
+static_assert (f4 (a, '\0') == a + 11, "");