diff mbox

Only accept BUILT_IN_NORMAL stringops for interesting_stringop_to_profile_p

Message ID DA41BE1DDCA941489001C7FBD7A8820E8381E7AD@szxema507-mbx.china.huawei.com
State New
Headers show

Commit Message

Yangfei (Felix) Aug. 20, 2015, 9:31 a.m. UTC
Thanks for the comments.  Attached please find the updated patch.  OK? 





> 

> On Thu, Aug 20, 2015 at 5:17 AM, Yangfei (Felix) <felix.yang@huawei.com>

> wrote:

> > Hi,

> >

> >     As DECL_FUNCTION_CODE is overloaded for builtin functions in different

> classes, so need to check builtin class before using fcode.

> >     Patch posted below.  Bootstrapped on x86_64-suse-linux, OK for trunk?

> >     Thanks.

> 

> Ugh.  The code in the callers already looks like it could have some TLC, like

> instead of

> 

>   fndecl = gimple_call_fndecl (stmt);

>   if (!fndecl)

>     return false;

>   fcode = DECL_FUNCTION_CODE (fndecl);

>   if (!interesting_stringop_to_profile_p (fndecl, stmt, &size_arg))

>     return false;

> 

> simply do

> 

>   if (!gimple_call_builtin_p (stmt, BUILT_IN_NORMAL))

>     return false;

>   if (!interesting_stringop_to_profile_p (gimple_call_fndecl (stmt), ....))

> 

> similar for the other caller.  interesting_stringop_to_profile_p can also get

> function-code directly from stmt, removing the redundant first argument or even

> do the gimple_call_builtin_p call itself.

> 

> Mind reworking the patch accordingly?

> 

> Thanks,

> Richard.

Comments

Richard Biener Aug. 20, 2015, 11:24 a.m. UTC | #1
On Thu, Aug 20, 2015 at 11:31 AM, Yangfei (Felix) <felix.yang@huawei.com> wrote:
> Thanks for the comments.  Attached please find the updated patch.  OK?

Ok.

Thanks,
Richard.

>
> Index: gcc/value-prof.c
> ===================================================================
> --- gcc/value-prof.c    (revision 141081)
> +++ gcc/value-prof.c    (working copy)
> @@ -209,7 +209,6 @@ gimple_add_histogram_value (struct function *fun,
>    hist->fun = fun;
>  }
>
> -
>  /* Remove histogram HIST from STMT's histogram list.  */
>
>  void
> @@ -234,7 +233,6 @@ gimple_remove_histogram_value (struct function *fu
>    free (hist);
>  }
>
> -
>  /* Lookup histogram of type TYPE in the STMT.  */
>
>  histogram_value
> @@ -389,6 +387,7 @@ stream_out_histogram_value (struct output_block *o
>    if (hist->hvalue.next)
>      stream_out_histogram_value (ob, hist->hvalue.next);
>  }
> +
>  /* Dump information about HIST to DUMP_FILE.  */
>
>  void
> @@ -488,7 +487,6 @@ gimple_duplicate_stmt_histograms (struct function
>      }
>  }
>
> -
>  /* Move all histograms associated with OSTMT to STMT.  */
>
>  void
> @@ -529,7 +527,6 @@ visit_hist (void **slot, void *data)
>    return 1;
>  }
>
> -
>  /* Verify sanity of the histograms.  */
>
>  DEBUG_FUNCTION void
> @@ -594,7 +591,6 @@ free_histograms (void)
>      }
>  }
>
> -
>  /* The overall number of invocations of the counter should match
>     execution count of basic block.  Report it as error rather than
>     internal error as it might mean that user has misused the profile
> @@ -638,7 +634,6 @@ check_counter (gimple stmt, const char * name,
>    return false;
>  }
>
> -
>  /* GIMPLE based transformations. */
>
>  bool
> @@ -697,7 +692,6 @@ gimple_value_profile_transformations (void)
>    return changed;
>  }
>
> -
>  /* Generate code for transformation 1 (with parent gimple assignment
>     STMT and probability of taking the optimal path PROB, which is
>     equivalent to COUNT/ALL within roundoff error).  This generates the
> @@ -859,6 +853,7 @@ gimple_divmod_fixed_value_transform (gimple_stmt_i
>     probability of taking the optimal path PROB, which is equivalent to COUNT/ALL
>     within roundoff error).  This generates the result into a temp and returns
>     the temp; it does not replace or alter the original STMT.  */
> +
>  static tree
>  gimple_mod_pow2 (gimple stmt, int prob, gcov_type count, gcov_type all)
>  {
> @@ -938,6 +933,7 @@ gimple_mod_pow2 (gimple stmt, int prob, gcov_type
>  }
>
>  /* Do transform 2) on INSN if applicable.  */
> +
>  static bool
>  gimple_mod_pow2_value_transform (gimple_stmt_iterator *si)
>  {
> @@ -1540,15 +1536,15 @@ gimple_ic_transform (gimple_stmt_iterator *gsi)
>    return true;
>  }
>
> -/* 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.
> -*/
> +/* Return true if the stringop CALL 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, int *size_arg)
> +interesting_stringop_to_profile_p (gimple call, int *size_arg)
>  {
> -  enum built_in_function fcode = DECL_FUNCTION_CODE (fndecl);
> +  enum built_in_function fcode;
>
> +  fcode = DECL_FUNCTION_CODE (gimple_call_fndecl (call));
>    if (fcode != BUILT_IN_MEMCPY && fcode != BUILT_IN_MEMPCPY
>        && fcode != BUILT_IN_MEMSET && fcode != BUILT_IN_BZERO)
>      return false;
> @@ -1573,7 +1569,7 @@ static bool
>      }
>  }
>
> -/* Convert   stringop (..., vcall_size)
> +/* Convert stringop (..., vcall_size)
>     into
>     if (vcall_size == icall_size)
>       stringop (..., icall_size);
> @@ -1590,11 +1586,9 @@ gimple_stringop_fixed_value (gimple vcall_stmt, tr
>    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))
> +  if (!interesting_stringop_to_profile_p (vcall_stmt, &size_arg))
>      gcc_unreachable ();
>
>    cond_bb = gimple_bb (vcall_stmt);
> @@ -1673,11 +1667,11 @@ gimple_stringop_fixed_value (gimple vcall_stmt, tr
>
>  /* Find values inside STMT for that we want to measure histograms for
>     division/modulo optimization.  */
> +
>  static bool
>  gimple_stringops_transform (gimple_stmt_iterator *gsi)
>  {
>    gimple stmt = gsi_stmt (*gsi);
> -  tree fndecl;
>    tree blck_size;
>    enum built_in_function fcode;
>    histogram_value histogram;
> @@ -1688,14 +1682,11 @@ gimple_stringops_transform (gimple_stmt_iterator *
>    tree tree_val;
>    int size_arg;
>
> -  if (gimple_code (stmt) != GIMPLE_CALL)
> +  if (!gimple_call_builtin_p (stmt, BUILT_IN_NORMAL))
>      return false;
> -  fndecl = gimple_call_fndecl (stmt);
> -  if (!fndecl)
> +
> +  if (!interesting_stringop_to_profile_p (stmt, &size_arg))
>      return false;
> -  fcode = DECL_FUNCTION_CODE (fndecl);
> -  if (!interesting_stringop_to_profile_p (fndecl, stmt, &size_arg))
> -    return false;
>
>    blck_size = gimple_call_arg (stmt, size_arg);
>    if (TREE_CODE (blck_size) == INTEGER_CST)
> @@ -1704,10 +1695,12 @@ gimple_stringops_transform (gimple_stmt_iterator *
>    histogram = gimple_histogram_value_of_type (cfun, stmt, HIST_TYPE_SINGLE_VALUE);
>    if (!histogram)
>      return false;
> +
>    val = histogram->hvalue.counters[0];
>    count = histogram->hvalue.counters[1];
>    all = histogram->hvalue.counters[2];
>    gimple_remove_histogram_value (cfun, stmt, histogram);
> +
>    /* We require that count is at least half of all; this means
>       that for the transformation to fire the value must be constant
>       at least 80% of time.  */
> @@ -1719,8 +1712,10 @@ gimple_stringops_transform (gimple_stmt_iterator *
>      prob = GCOV_COMPUTE_SCALE (count, all);
>    else
>      prob = 0;
> +
>    dest = gimple_call_arg (stmt, 0);
>    dest_align = get_pointer_alignment (dest);
> +  fcode = DECL_FUNCTION_CODE (gimple_call_fndecl (stmt));
>    switch (fcode)
>      {
>      case BUILT_IN_MEMCPY:
> @@ -1811,6 +1806,7 @@ stringop_block_profile (gimple stmt, unsigned int
>
>
>  /* Find values inside STMT for that we want to measure histograms for
>     division/modulo optimization.  */
> +
>  static void
>  gimple_divmod_values_to_profile (gimple stmt, histogram_values *values)
>  {
> @@ -1891,21 +1887,18 @@ gimple_indirect_call_to_profile (gimple stmt, hist
>
>  /* Find values inside STMT for that we want to measure histograms for
>     string operations.  */
> +
>  static void
>  gimple_stringops_values_to_profile (gimple stmt, histogram_values *values)
>  {
> -  tree fndecl;
> -  tree blck_size;
>    tree dest;
> +  tree blck_size;
>    int size_arg;
>
> -  if (gimple_code (stmt) != GIMPLE_CALL)
> +  if (!gimple_call_builtin_p (stmt, BUILT_IN_NORMAL))
>      return;
> -  fndecl = gimple_call_fndecl (stmt);
> -  if (!fndecl)
> -    return;
>
> -  if (!interesting_stringop_to_profile_p (fndecl, stmt, &size_arg))
> +  if (!interesting_stringop_to_profile_p (stmt, &size_arg))
>      return;
>
>    dest = gimple_call_arg (stmt, 0);
> @@ -1919,6 +1912,7 @@ gimple_stringops_values_to_profile (gimple stmt, h
>        values->safe_push (gimple_alloc_histogram_value (cfun, HIST_TYPE_AVERAGE,
>                                                        stmt, blck_size));
>      }
> +
>    if (TREE_CODE (blck_size) != INTEGER_CST)
>      values->safe_push (gimple_alloc_histogram_value (cfun, HIST_TYPE_IOR,
>                                                      stmt, dest));
> Index: gcc/ChangeLog
> ===================================================================
> --- gcc/ChangeLog       (revision 141081)
> +++ gcc/ChangeLog       (working copy)
> @@ -1,3 +1,12 @@
> +2014-08-20  Felix Yang  <felix.yang@huawei.com>
> +           Jiji Jiang  <jiangjiji@huawei.com>
> +
> +       * value-prof.c (interesting_stringop_to_profile_p): Removed FNDECL argument
> +       and get builtin function code directly from CALL.
> +       (gimple_stringop_fixed_value): Modified accordingly.
> +       (gimple_stringops_transform, gimple_stringops_values_to_profile): Modified
> +       accordingly and only accept BUILT_IN_NORMAL string operations.
> +
>  2015-08-18  Segher Boessenkool  <segher@kernel.crashing.org>
>
>         Backport from mainline:
>
>
>
>>
>> On Thu, Aug 20, 2015 at 5:17 AM, Yangfei (Felix) <felix.yang@huawei.com>
>> wrote:
>> > Hi,
>> >
>> >     As DECL_FUNCTION_CODE is overloaded for builtin functions in different
>> classes, so need to check builtin class before using fcode.
>> >     Patch posted below.  Bootstrapped on x86_64-suse-linux, OK for trunk?
>> >     Thanks.
>>
>> Ugh.  The code in the callers already looks like it could have some TLC, like
>> instead of
>>
>>   fndecl = gimple_call_fndecl (stmt);
>>   if (!fndecl)
>>     return false;
>>   fcode = DECL_FUNCTION_CODE (fndecl);
>>   if (!interesting_stringop_to_profile_p (fndecl, stmt, &size_arg))
>>     return false;
>>
>> simply do
>>
>>   if (!gimple_call_builtin_p (stmt, BUILT_IN_NORMAL))
>>     return false;
>>   if (!interesting_stringop_to_profile_p (gimple_call_fndecl (stmt), ....))
>>
>> similar for the other caller.  interesting_stringop_to_profile_p can also get
>> function-code directly from stmt, removing the redundant first argument or even
>> do the gimple_call_builtin_p call itself.
>>
>> Mind reworking the patch accordingly?
>>
>> Thanks,
>> Richard.
>
diff mbox

Patch

Index: gcc/value-prof.c
===================================================================
--- gcc/value-prof.c	(revision 141081)
+++ gcc/value-prof.c	(working copy)
@@ -209,7 +209,6 @@  gimple_add_histogram_value (struct function *fun,
   hist->fun = fun;
 }
 
-
 /* Remove histogram HIST from STMT's histogram list.  */
 
 void
@@ -234,7 +233,6 @@  gimple_remove_histogram_value (struct function *fu
   free (hist);
 }
 
-
 /* Lookup histogram of type TYPE in the STMT.  */
 
 histogram_value
@@ -389,6 +387,7 @@  stream_out_histogram_value (struct output_block *o
   if (hist->hvalue.next)
     stream_out_histogram_value (ob, hist->hvalue.next);
 }
+
 /* Dump information about HIST to DUMP_FILE.  */
 
 void
@@ -488,7 +487,6 @@  gimple_duplicate_stmt_histograms (struct function
     }
 }
 
-
 /* Move all histograms associated with OSTMT to STMT.  */
 
 void
@@ -529,7 +527,6 @@  visit_hist (void **slot, void *data)
   return 1;
 }
 
-
 /* Verify sanity of the histograms.  */
 
 DEBUG_FUNCTION void
@@ -594,7 +591,6 @@  free_histograms (void)
     }
 }
 
-
 /* The overall number of invocations of the counter should match
    execution count of basic block.  Report it as error rather than
    internal error as it might mean that user has misused the profile
@@ -638,7 +634,6 @@  check_counter (gimple stmt, const char * name,
   return false;
 }
 
-
 /* GIMPLE based transformations. */
 
 bool
@@ -697,7 +692,6 @@  gimple_value_profile_transformations (void)
   return changed;
 }
 
-
 /* Generate code for transformation 1 (with parent gimple assignment
    STMT and probability of taking the optimal path PROB, which is
    equivalent to COUNT/ALL within roundoff error).  This generates the
@@ -859,6 +853,7 @@  gimple_divmod_fixed_value_transform (gimple_stmt_i
    probability of taking the optimal path PROB, which is equivalent to COUNT/ALL
    within roundoff error).  This generates the result into a temp and returns
    the temp; it does not replace or alter the original STMT.  */
+
 static tree
 gimple_mod_pow2 (gimple stmt, int prob, gcov_type count, gcov_type all)
 {
@@ -938,6 +933,7 @@  gimple_mod_pow2 (gimple stmt, int prob, gcov_type
 }
 
 /* Do transform 2) on INSN if applicable.  */
+
 static bool
 gimple_mod_pow2_value_transform (gimple_stmt_iterator *si)
 {
@@ -1540,15 +1536,15 @@  gimple_ic_transform (gimple_stmt_iterator *gsi)
   return true;
 }
 
-/* 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.
-*/
+/* Return true if the stringop CALL 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, int *size_arg)
+interesting_stringop_to_profile_p (gimple call, int *size_arg)
 {
-  enum built_in_function fcode = DECL_FUNCTION_CODE (fndecl);
+  enum built_in_function fcode;
 
+  fcode = DECL_FUNCTION_CODE (gimple_call_fndecl (call));
   if (fcode != BUILT_IN_MEMCPY && fcode != BUILT_IN_MEMPCPY
       && fcode != BUILT_IN_MEMSET && fcode != BUILT_IN_BZERO)
     return false;
@@ -1573,7 +1569,7 @@  static bool
     }
 }
 
-/* Convert   stringop (..., vcall_size)
+/* Convert stringop (..., vcall_size)
    into
    if (vcall_size == icall_size)
      stringop (..., icall_size);
@@ -1590,11 +1586,9 @@  gimple_stringop_fixed_value (gimple vcall_stmt, tr
   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))
+  if (!interesting_stringop_to_profile_p (vcall_stmt, &size_arg))
     gcc_unreachable ();
 
   cond_bb = gimple_bb (vcall_stmt);
@@ -1673,11 +1667,11 @@  gimple_stringop_fixed_value (gimple vcall_stmt, tr
 
 /* Find values inside STMT for that we want to measure histograms for
    division/modulo optimization.  */
+
 static bool
 gimple_stringops_transform (gimple_stmt_iterator *gsi)
 {
   gimple stmt = gsi_stmt (*gsi);
-  tree fndecl;
   tree blck_size;
   enum built_in_function fcode;
   histogram_value histogram;
@@ -1688,14 +1682,11 @@  gimple_stringops_transform (gimple_stmt_iterator *
   tree tree_val;
   int size_arg;
 
-  if (gimple_code (stmt) != GIMPLE_CALL)
+  if (!gimple_call_builtin_p (stmt, BUILT_IN_NORMAL))
     return false;
-  fndecl = gimple_call_fndecl (stmt);
-  if (!fndecl)
+
+  if (!interesting_stringop_to_profile_p (stmt, &size_arg))
     return false;
-  fcode = DECL_FUNCTION_CODE (fndecl);
-  if (!interesting_stringop_to_profile_p (fndecl, stmt, &size_arg))
-    return false;
 
   blck_size = gimple_call_arg (stmt, size_arg);
   if (TREE_CODE (blck_size) == INTEGER_CST)
@@ -1704,10 +1695,12 @@  gimple_stringops_transform (gimple_stmt_iterator *
   histogram = gimple_histogram_value_of_type (cfun, stmt, HIST_TYPE_SINGLE_VALUE);
   if (!histogram)
     return false;
+
   val = histogram->hvalue.counters[0];
   count = histogram->hvalue.counters[1];
   all = histogram->hvalue.counters[2];
   gimple_remove_histogram_value (cfun, stmt, histogram);
+
   /* We require that count is at least half of all; this means
      that for the transformation to fire the value must be constant
      at least 80% of time.  */
@@ -1719,8 +1712,10 @@  gimple_stringops_transform (gimple_stmt_iterator *
     prob = GCOV_COMPUTE_SCALE (count, all);
   else
     prob = 0;
+
   dest = gimple_call_arg (stmt, 0);
   dest_align = get_pointer_alignment (dest);
+  fcode = DECL_FUNCTION_CODE (gimple_call_fndecl (stmt));
   switch (fcode)
     {
     case BUILT_IN_MEMCPY:
@@ -1811,6 +1806,7 @@  stringop_block_profile (gimple stmt, unsigned int
 

 /* Find values inside STMT for that we want to measure histograms for
    division/modulo optimization.  */
+
 static void
 gimple_divmod_values_to_profile (gimple stmt, histogram_values *values)
 {
@@ -1891,21 +1887,18 @@  gimple_indirect_call_to_profile (gimple stmt, hist
 
 /* Find values inside STMT for that we want to measure histograms for
    string operations.  */
+
 static void
 gimple_stringops_values_to_profile (gimple stmt, histogram_values *values)
 {
-  tree fndecl;
-  tree blck_size;
   tree dest;
+  tree blck_size;
   int size_arg;
 
-  if (gimple_code (stmt) != GIMPLE_CALL)
+  if (!gimple_call_builtin_p (stmt, BUILT_IN_NORMAL))
     return;
-  fndecl = gimple_call_fndecl (stmt);
-  if (!fndecl)
-    return;
 
-  if (!interesting_stringop_to_profile_p (fndecl, stmt, &size_arg))
+  if (!interesting_stringop_to_profile_p (stmt, &size_arg))
     return;
 
   dest = gimple_call_arg (stmt, 0);
@@ -1919,6 +1912,7 @@  gimple_stringops_values_to_profile (gimple stmt, h
       values->safe_push (gimple_alloc_histogram_value (cfun, HIST_TYPE_AVERAGE,
 						       stmt, blck_size));
     }
+
   if (TREE_CODE (blck_size) != INTEGER_CST)
     values->safe_push (gimple_alloc_histogram_value (cfun, HIST_TYPE_IOR,
 						     stmt, dest));
Index: gcc/ChangeLog
===================================================================
--- gcc/ChangeLog	(revision 141081)
+++ gcc/ChangeLog	(working copy)
@@ -1,3 +1,12 @@ 
+2014-08-20  Felix Yang  <felix.yang@huawei.com>
+	    Jiji Jiang  <jiangjiji@huawei.com>
+
+	* value-prof.c (interesting_stringop_to_profile_p): Removed FNDECL argument
+	and get builtin function code directly from CALL.
+	(gimple_stringop_fixed_value): Modified accordingly.
+	(gimple_stringops_transform, gimple_stringops_values_to_profile): Modified
+	accordingly and only accept BUILT_IN_NORMAL string operations.
+
 2015-08-18  Segher Boessenkool  <segher@kernel.crashing.org>
 
 	Backport from mainline: