Re: Patch for builtin strpbrk

["Kaveh R. Ghazi" <ghazi at caip dot rutgers dot edu>]

 > From: Mark Mitchell <mark@codesourcery.com>
 > 
 > >>>>> "Kaveh" == Kaveh R Ghazi <ghazi@caip.rutgers.edu> writes:
 > 
 > 
 > [...]
 > 
 >     Kaveh> Bootstrap and testsuite underway.  Assuming no regressions,
 >     Kaveh> okay to install?
 >  
 > Yes.
 >  
 > You might as well also do the case where both operands are constants;
 > that can always be computed at compile-time...
 > 
 > [...]
 > 
 >     Kaveh> doing is correct.  Also since you're not sure Checker
 >     Kaveh> actually works, I can't figure it out through trial and
 >     Kaveh> error and test what I come up with.
 > 
 > FWIW, I disapprove of the insertion of checker/stack-protection/etc.
 > code into the core of GCC.

OK.  I went ahead and installed the following.  It handles the case
where both operands are constants and also ensures that side effects
are honored.  I added a bits to the testcase to check both issues.  I
did not add anything for Checker.

Although it passed bootstrap/testsuite on solaris2.7 and its already
installed, I'd appreciate a sanity check on the two changes added
since the last time I posted it.

WRT evaluating two constants, I called strpbrk, find the offset and
return ("string1" + offset).  Is that the "right thing" to do?  Do I
need to `fold' the PLUS_EXPR?  (It seems to be done elsewhere pretty
often so I did.)

WRT side effects, I evaluated the second argument, but I couldn't
think of a testcase in which the second string has side effects but
still gets transformed.  Any ideas?

		Thanks,
		--Kaveh


2000-11-06  Kaveh R. Ghazi  <ghazi@caip.rutgers.edu>

	* builtins.c (expand_builtin_strpbrk): New function.
	(expand_builtin): Handle BUILT_IN_STRPBRK.

	* builtins.def (BUILT_IN_STRPBRK): New entry.

	* c-common.c (c_common_nodes_and_builtins): Declare builtin
	strpbrk.

testsuite:
	* gcc.c-torture/execute/string-opt-2.c: New test.

diff -rup orig/egcs-CVS20001105/gcc/builtins.c egcs-CVS20001105/gcc/builtins.c
--- orig/egcs-CVS20001105/gcc/builtins.c	Sun Nov  5 00:07:51 2000
+++ egcs-CVS20001105/gcc/builtins.c	Tue Nov  7 00:44:02 2000
@@ -109,6 +109,8 @@ static rtx expand_builtin_bzero		PARAMS 
 static rtx expand_builtin_strlen	PARAMS ((tree, rtx));
 static rtx expand_builtin_strstr	PARAMS ((tree, rtx,
 						 enum machine_mode));
+static rtx expand_builtin_strpbrk	PARAMS ((tree, rtx,
+						 enum machine_mode));
 static rtx expand_builtin_alloca	PARAMS ((tree, rtx));
 static rtx expand_builtin_ffs		PARAMS ((tree, rtx, rtx));
 static rtx expand_builtin_frame_address	PARAMS ((tree));
@@ -1459,6 +1461,100 @@ expand_builtin_strstr (arglist, target, 
     }
 }
 
+/* Expand a call to the strpbrk builtin.  Return 0 if we failed the
+   caller should emit a normal call, otherwise try to get the result
+   in TARGET, if convenient (and in mode MODE if that's convenient).  */
+
+static rtx
+expand_builtin_strpbrk (arglist, target, mode)
+     tree arglist;
+     rtx target;
+     enum machine_mode mode;
+{
+  if (arglist == 0
+      || TREE_CODE (TREE_TYPE (TREE_VALUE (arglist))) != POINTER_TYPE
+      || TREE_CHAIN (arglist) == 0
+      || TREE_CODE (TREE_TYPE (TREE_VALUE (TREE_CHAIN (arglist)))) != POINTER_TYPE)
+    return 0;
+  else
+    {
+      tree s1 = TREE_VALUE (arglist), s2 = TREE_VALUE (TREE_CHAIN (arglist));
+      tree len1 = c_strlen (s1), len2 = c_strlen (s2);
+      tree stripped_s1 = s1, stripped_s2 = s2;
+
+      STRIP_NOPS (stripped_s1);
+      if (stripped_s1 && TREE_CODE (stripped_s1) == ADDR_EXPR)
+	stripped_s1 = TREE_OPERAND (stripped_s1, 0);
+      STRIP_NOPS (stripped_s2);
+      if (stripped_s2 && TREE_CODE (stripped_s2) == ADDR_EXPR)
+	stripped_s2 = TREE_OPERAND (stripped_s2, 0);
+
+      /* If both arguments are constants, calculate the result now.  */
+      if (len1 && len2
+	  && TREE_CODE (stripped_s1) == STRING_CST
+	  && TREE_CODE (stripped_s2) == STRING_CST)
+        {
+	  const char *const result =
+	    strpbrk (TREE_STRING_POINTER (stripped_s1),
+		     TREE_STRING_POINTER (stripped_s2));
+
+	  if (result)
+	    {
+	      long offset = result - TREE_STRING_POINTER (stripped_s1);
+
+	      /* Return an offset into the constant string argument.  */
+	      return expand_expr (fold (build (PLUS_EXPR, TREE_TYPE (s1),
+					       s1, ssize_int (offset))),
+				  target, mode, EXPAND_NORMAL);
+	    }
+	  else
+	    return const0_rtx;
+	}
+
+      /* We must have been able to figure out the second argument's
+         length to do anything else.  */
+      if (!len2)
+	return 0;
+
+      /* OK, handle some cases.  */
+      switch (compare_tree_int (len2, 1))
+        {
+	case -1: /* length is 0, return NULL.  */
+	  {
+	    /* Evaluate and ignore the arguments in case they had
+	       side-effects.  */
+	    expand_expr (s1, const0_rtx, VOIDmode, EXPAND_NORMAL);
+	    expand_expr (s2, const0_rtx, VOIDmode, EXPAND_NORMAL);
+	    return const0_rtx;
+	  }
+	case 0: /* length is 1, return strchr(s1, s2[0]).  */
+	  {
+	    tree call_expr, fn = built_in_decls[BUILT_IN_STRCHR];
+
+	    if (!fn)
+	      return 0;
+
+	    /* New argument list transforming strpbrk(s1, s2) to
+	       strchr(s1, s2[0]).  */
+	    arglist =
+	      build_tree_list (NULL_TREE, build_int_2
+			       (TREE_STRING_POINTER (stripped_s2)[0], 0));
+	    arglist = tree_cons (NULL_TREE, s1, arglist);
+	    call_expr = build1 (ADDR_EXPR,
+				build_pointer_type (TREE_TYPE (fn)), fn);
+	    call_expr = build (CALL_EXPR, TREE_TYPE (TREE_TYPE (fn)),
+			       call_expr, arglist, NULL_TREE);
+	    TREE_SIDE_EFFECTS (call_expr) = 1;
+	    return expand_expr (call_expr, target, mode, EXPAND_NORMAL);
+	  }
+	case 1: /* length is greater than 1, really call strpbrk.  */
+	  return 0;
+	default:
+	  abort();
+	}
+    }
+}
+
 /* Expand a call to the memcpy builtin, with arguments in ARGLIST.  */
 static rtx
 expand_builtin_memcpy (arglist)
@@ -2503,7 +2599,7 @@ expand_builtin (exp, target, subtarget, 
 	  || fcode == BUILT_IN_MEMCPY || fcode == BUILT_IN_MEMCMP
 	  || fcode == BUILT_IN_BCMP || fcode == BUILT_IN_BZERO
 	  || fcode == BUILT_IN_STRLEN || fcode == BUILT_IN_STRCPY
-	  || fcode == BUILT_IN_STRSTR
+	  || fcode == BUILT_IN_STRSTR || fcode == BUILT_IN_STRPBRK
 	  || fcode == BUILT_IN_STRCMP || fcode == BUILT_IN_FFS
 	  || fcode == BUILT_IN_PUTCHAR || fcode == BUILT_IN_PUTS
 	  || fcode == BUILT_IN_PRINTF || fcode == BUILT_IN_FPUTC
@@ -2634,6 +2730,12 @@ expand_builtin (exp, target, subtarget, 
       
     case BUILT_IN_STRSTR:
       target = expand_builtin_strstr (arglist, target, mode);
+      if (target)
+	return target;
+      break;
+      
+    case BUILT_IN_STRPBRK:
+      target = expand_builtin_strpbrk (arglist, target, mode);
       if (target)
 	return target;
       break;
diff -rup orig/egcs-CVS20001105/gcc/builtins.def egcs-CVS20001105/gcc/builtins.def
--- orig/egcs-CVS20001105/gcc/builtins.def	Tue Oct 31 22:17:32 2000
+++ egcs-CVS20001105/gcc/builtins.def	Mon Nov  6 16:32:12 2000
@@ -40,6 +40,7 @@ DEF_BUILTIN(BUILT_IN_STRCPY)
 DEF_BUILTIN(BUILT_IN_STRCMP)
 DEF_BUILTIN(BUILT_IN_STRLEN)
 DEF_BUILTIN(BUILT_IN_STRSTR)
+DEF_BUILTIN(BUILT_IN_STRPBRK)
 DEF_BUILTIN(BUILT_IN_STRCHR)
 DEF_BUILTIN(BUILT_IN_FSQRT)
 DEF_BUILTIN(BUILT_IN_SIN)
diff -rup orig/egcs-CVS20001105/gcc/c-common.c egcs-CVS20001105/gcc/c-common.c
--- orig/egcs-CVS20001105/gcc/c-common.c	Thu Nov  2 21:44:31 2000
+++ egcs-CVS20001105/gcc/c-common.c	Tue Nov  7 01:03:01 2000
@@ -4935,7 +4935,7 @@ c_common_nodes_and_builtins ()
 						 const_string_type_node,
 						 endlink)));
 
-  /* Prototype for strstr, etc.  */
+  /* Prototype for strstr, strpbrk, etc.  */
   string_ftype_string_string
     = build_function_type (string_type_node,
 			   tree_cons (NULL_TREE, const_string_type_node,
@@ -5174,6 +5174,8 @@ c_common_nodes_and_builtins ()
 		    BUILT_IN_STRCMP, BUILT_IN_NORMAL, "strcmp");
   builtin_function ("__builtin_strstr", string_ftype_string_string,
 		    BUILT_IN_STRSTR, BUILT_IN_NORMAL, "strstr");
+  builtin_function ("__builtin_strpbrk", string_ftype_string_string,
+		    BUILT_IN_STRPBRK, BUILT_IN_NORMAL, "strpbrk");
   built_in_decls[BUILT_IN_STRCHR] =
     builtin_function ("__builtin_strchr", string_ftype_string_int,
 		    BUILT_IN_STRCHR, BUILT_IN_NORMAL, "strchr");
@@ -5246,6 +5248,8 @@ c_common_nodes_and_builtins ()
       builtin_function ("strcmp", int_ftype_string_string, BUILT_IN_STRCMP,
 			BUILT_IN_NORMAL, NULL_PTR);
       builtin_function ("strstr", string_ftype_string_string, BUILT_IN_STRSTR,
+			BUILT_IN_NORMAL, NULL_PTR);
+      builtin_function ("strpbrk", string_ftype_string_string, BUILT_IN_STRPBRK,
 			BUILT_IN_NORMAL, NULL_PTR);
       builtin_function ("strcpy", string_ftype_ptr_ptr, BUILT_IN_STRCPY,
 			BUILT_IN_NORMAL, NULL_PTR);
diff -rup orig/egcs-CVS20001105/gcc/testsuite/gcc.c-torture/execute/string-opt-2.c egcs-CVS20001105/gcc/testsuite/gcc.c-torture/execute/string-opt-2.c
--- orig/egcs-CVS20001105/gcc/testsuite/gcc.c-torture/execute/string-opt-2.c	Mon Nov  6 16:58:56 2000
+++ egcs-CVS20001105/gcc/testsuite/gcc.c-torture/execute/string-opt-2.c	Tue Nov  7 00:54:55 2000
@@ -0,0 +1,48 @@
+/* Copyright (C) 2000  Free Software Foundation.
+
+   Ensure all expected transformations of builtin strpbrk occur and
+   perform correctly.
+
+   Written by Kaveh R. Ghazi, 11/6/2000.  */
+
+extern void abort(void);
+extern char *strpbrk (const char *, const char *);
+
+void fn (const char *foo, const char *const *bar)
+{
+  if (strcmp(strpbrk ("hello world", "lrooo"), "llo world") != 0)
+    abort();
+  if (strpbrk (foo, "") != 0)
+    abort();
+  if (strpbrk (foo + 4, "") != 0)
+    abort();
+  if (strpbrk (*bar--, "") != 0)
+    abort();
+  if (strpbrk (*bar, "h") != foo)
+    abort();
+  if (strpbrk (foo, "h") != foo)
+    abort();
+  if (strpbrk (foo, "w") != foo + 6)
+    abort();
+  if (strpbrk (foo + 6, "o") != foo + 7)
+    abort();
+}
+
+int main()
+{
+  const char *const foo[] = { "hello world", "bye bye world" };
+  fn (foo[0], foo + 1);
+  return 0;
+}
+
+
+#ifdef __OPTIMIZE__
+/* When optimizing, all the above cases should be transformed into
+   something else.  So any remaining calls to the original function
+   should abort.  */
+char *
+strpbrk(const char *s1, const char *s2)
+{
+  abort();
+}
+#endif