Re: [PATCH] Fix an ICE caused by assuming all builtin stringops have 3 arguments

[Neil Vachharajani <nvachhar at google dot com>]

On Fri, Sep 18, 2009 at 3:39 AM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> Neil Vachharajani <nvachhar@google.com> writes:
>
>> --- gcc/value-prof.c.orig
>> +++ gcc/value-prof.c
>> @@ -1239,9 +1239,12 @@
>> ? ?return true;
>> ?}
>>
>> -/* Return true if the stringop CALL with FNDECL shall be profiled. ?*/
>> +/* Return true if the stringop CALL with FNDECL shall be profiled. ?If
>> + ? SIZE_ARG is not null, it will be the argument index for the size of
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? is set to the ...
>
> It appears that all calls to interesting_stringop_to_profile_p pass an
> non-NULL size_arg, so testing for non-NULL is probably not needed.

Good suggestion.  I incorporated in the final submitted patch.

>
> Andreas.
>
> --
> Andreas Schwab, schwab@linux-m68k.org
> GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 ?01D3 44D5 214B 8276 4ED5
> "And now for something completely different."
>

The patch was submitted in rev r151864.  Here is the final patch:

2009-09-18  Neil Vachharajani  <nvachhar@google.com>

	* value-prof.c (interesting_stringop_to_profile_p): Added output
	argument to indicate which parameter is the size parameter.
	* value-prof.c (gimple_stringop_fixed_value): Use
	INTERESTING_STRINGOP_TO_PROFILE_P to find size argument.
	* value-prof.c (gimple_stringops_transform): Update call sites to
	INTERESTING_STRINGOP_TO_PROFILE_P to reflect parameter change.
	* testsuite/gcc.dg/tree-prof/val-prof-7.c: Added test case.

--- gcc/value-prof.c	(revision 151863)
+++ gcc/value-prof.c	(revision 151864)
@@ -1241,9 +1241,12 @@
   return true;
 }

-/* Return true if the stringop CALL with FNDECL shall be profiled.  */
+/* Return true if the stringop CALL with FNDECL shall be profiled.
+   SIZE_ARG be set to the argument index for the size of the string
+   operation.
+*/
 static bool
-interesting_stringop_to_profile_p (tree fndecl, gimple call)
+interesting_stringop_to_profile_p (tree fndecl, gimple call, int *size_arg)
 {
   enum built_in_function fcode = DECL_FUNCTION_CODE (fndecl);

@@ -1255,12 +1258,15 @@
     {
      case BUILT_IN_MEMCPY:
      case BUILT_IN_MEMPCPY:
+       *size_arg = 2;
        return validate_gimple_arglist (call, POINTER_TYPE, POINTER_TYPE,
 				       INTEGER_TYPE, VOID_TYPE);
      case BUILT_IN_MEMSET:
+       *size_arg = 2;
        return validate_gimple_arglist (call, POINTER_TYPE, INTEGER_TYPE,
 				      INTEGER_TYPE, VOID_TYPE);
      case BUILT_IN_BZERO:
+       *size_arg = 1;
        return validate_gimple_arglist (call, POINTER_TYPE, INTEGER_TYPE,
 				       VOID_TYPE);
      default:
@@ -1285,11 +1291,17 @@
   basic_block cond_bb, icall_bb, vcall_bb, join_bb;
   edge e_ci, e_cv, e_iv, e_ij, e_vj;
   gimple_stmt_iterator gsi;
+  tree fndecl;
+  int size_arg;

+  fndecl = gimple_call_fndecl (vcall_stmt);
+  if (!interesting_stringop_to_profile_p (fndecl, vcall_stmt, &size_arg))
+    gcc_unreachable();
+
   cond_bb = gimple_bb (vcall_stmt);
   gsi = gsi_for_stmt (vcall_stmt);

-  vcall_size = gimple_call_arg (vcall_stmt, 2);
+  vcall_size = gimple_call_arg (vcall_stmt, size_arg);
   optype = TREE_TYPE (vcall_size);

   tmpv = create_tmp_var (optype, "PROF");
@@ -1304,7 +1316,7 @@
   gsi_insert_before (&gsi, cond_stmt, GSI_SAME_STMT);

   icall_stmt = gimple_copy (vcall_stmt);
-  gimple_call_set_arg (icall_stmt, 2, icall_size);
+  gimple_call_set_arg (icall_stmt, size_arg, icall_size);
   gsi_insert_before (&gsi, icall_stmt, GSI_SAME_STMT);

   /* Fix CFG. */
@@ -1359,6 +1371,7 @@
   unsigned int dest_align, src_align;
   gcov_type prob;
   tree tree_val;
+  int size_arg;

   if (gimple_code (stmt) != GIMPLE_CALL)
     return false;
@@ -1366,13 +1379,10 @@
   if (!fndecl)
     return false;
   fcode = DECL_FUNCTION_CODE (fndecl);
-  if (!interesting_stringop_to_profile_p (fndecl, stmt))
+  if (!interesting_stringop_to_profile_p (fndecl, stmt, &size_arg))
     return false;

-  if (fcode == BUILT_IN_BZERO)
-    blck_size = gimple_call_arg (stmt, 1);
-  else
-    blck_size = gimple_call_arg (stmt, 2);
+  blck_size = gimple_call_arg (stmt, size_arg);
   if (TREE_CODE (blck_size) == INTEGER_CST)
     return false;

@@ -1583,6 +1593,7 @@
   tree blck_size;
   tree dest;
   enum built_in_function fcode;
+  int size_arg;

   if (gimple_code (stmt) != GIMPLE_CALL)
     return;
@@ -1591,14 +1602,11 @@
     return;
   fcode = DECL_FUNCTION_CODE (fndecl);

-  if (!interesting_stringop_to_profile_p (fndecl, stmt))
+  if (!interesting_stringop_to_profile_p (fndecl, stmt, &size_arg))
     return;

   dest = gimple_call_arg (stmt, 0);
-  if (fcode == BUILT_IN_BZERO)
-    blck_size = gimple_call_arg (stmt, 1);
-  else
-    blck_size = gimple_call_arg (stmt, 2);
+  blck_size = gimple_call_arg (stmt, size_arg);

   if (TREE_CODE (blck_size) != INTEGER_CST)
     {

--- gcc/testsuite/gcc.dg/tree-prof/val-prof-7.c	(revision 0)
+++ gcc/testsuite/gcc.dg/tree-prof/val-prof-7.c	(revision 151864)
@@ -0,0 +1,26 @@
+/* { dg-options "-O2 -fdump-tree-tree_profile -mtune=core2" } */
+/* { dg-skip-if "" { ! { i?86-*-* x86_64-*-* } } { "*" } { "" } } */
+
+#include <strings.h>
+
+int foo(int len)
+{
+  char array[1000];
+  bzero(array, len);
+  return 0;
+}
+
+int main() {
+  int i;
+  for (i = 0; i < 1000; i++)
+    {
+      if (i > 990)
+	foo(16);
+      else
+	foo(8);
+    }
+  return 0;
+}
+
+/* { dg-final-use { scan-tree-dump "Single value 8 stringop
transformation on bzero" "tree_profile"} } */
+/* { dg-final-use { cleanup-tree-dump "tree_profile" } } */