Re: [PATCH] Only accept BUILT_IN_NORMAL stringops for interesting_stringop_to_profile_p

[Yangfei (Felix)]

Thanks for the comments.  Attached please find the updated patch.  OK? 


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, 

Re: [PATCH] Only accept BUILT_IN_NORMAL stringops for interesting_stringop_to_profile_p

[Richard Biener]

On Thu, Aug 20, 2015 at 11:31 AM, Yangfei (Felix) felix.y...@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;

Re: [PATCH] Only accept BUILT_IN_NORMAL stringops for interesting_stringop_to_profile_p

[Richard Biener]

On Thu, Aug 20, 2015 at 5:17 AM, Yangfei (Felix) felix.y...@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.

 Index: gcc/value-prof.c
 ===
 --- gcc/value-prof.c(revision 141081)
 +++ gcc/value-prof.c(working copy)
 @@ -1547,8 +1547,12 @@ gimple_ic_transform (gimple_stmt_iterator *gsi)
  static bool
  interesting_stringop_to_profile_p (tree fndecl, gimple call, int *size_arg)
  {
 -  enum built_in_function fcode = DECL_FUNCTION_CODE (fndecl);
 +  enum built_in_function fcode;

 +  if (DECL_BUILT_IN_CLASS (fndecl) != BUILT_IN_NORMAL)
 +return false;
 +
 +  fcode = DECL_FUNCTION_CODE (fndecl);
if (fcode != BUILT_IN_MEMCPY  fcode != BUILT_IN_MEMPCPY
 fcode != BUILT_IN_MEMSET  fcode != BUILT_IN_BZERO)
  return false;
 Index: gcc/ChangeLog
 ===
 --- gcc/ChangeLog   (revision 141081)
 +++ gcc/ChangeLog   (working copy)
 @@ -1,3 +1,9 @@
 +2015-08-20  Felix Yang  felix.y...@huawei.com
 +   Jiji Jiang  jiangj...@huawei.com
 +
 +   * value-prof.c (interesting_stringop_to_profile_p): Only accept string
 +   operations which belong to the BUILT_IN_NORMAL builtin class.
 +
  2015-08-18  Segher Boessenkool  seg...@kernel.crashing.org

 Backport from mainline:

[PATCH] Only accept BUILT_IN_NORMAL stringops for interesting_stringop_to_profile_p

[Yangfei (Felix)]

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. 

Index: gcc/value-prof.c
===
--- gcc/value-prof.c(revision 141081)
+++ gcc/value-prof.c(working copy)
@@ -1547,8 +1547,12 @@ gimple_ic_transform (gimple_stmt_iterator *gsi)
 static bool
 interesting_stringop_to_profile_p (tree fndecl, gimple call, int *size_arg)
 {
-  enum built_in_function fcode = DECL_FUNCTION_CODE (fndecl);
+  enum built_in_function fcode;
 
+  if (DECL_BUILT_IN_CLASS (fndecl) != BUILT_IN_NORMAL)
+return false;
+
+  fcode = DECL_FUNCTION_CODE (fndecl);
   if (fcode != BUILT_IN_MEMCPY  fcode != BUILT_IN_MEMPCPY
fcode != BUILT_IN_MEMSET  fcode != BUILT_IN_BZERO)
 return false;
Index: gcc/ChangeLog
===
--- gcc/ChangeLog   (revision 141081)
+++ gcc/ChangeLog   (working copy)
@@ -1,3 +1,9 @@
+2015-08-20  Felix Yang  felix.y...@huawei.com
+   Jiji Jiang  jiangj...@huawei.com
+
+   * value-prof.c (interesting_stringop_to_profile_p): Only accept string
+   operations which belong to the BUILT_IN_NORMAL builtin class.
+
 2015-08-18  Segher Boessenkool  seg...@kernel.crashing.org
 
Backport from mainline: