On Sun, 5 Jun 2016, Jan Hubicka wrote: > Hi, > both loop-ch and loop-ivcanon want to trottle down the heuristics on paths > containing call. Testing for presence of GIMPLE_CALL is wrong for internal > call and cheap builtins that are expanded inline. > > Bootstrapped/regtested x86_64-linux, OK?
First of all the name is bad - I'd say gimple_inexpensive_call_p () is better. More comments below. > Honza > > * gimple.c: Include builtins.h. > (gimple_real_call_p): New function. > * gimple.h (gimple_real_call_p): Declare. > * tree-ssa-loop-ch.c (should_duplicate_loop_header_p): Use it. > * tree-ssa-loop-ivcanon.c (tree_estimate_loop_size): Likewise. > Index: gimple.c > =================================================================== > --- gimple.c (revision 237101) > +++ gimple.c (working copy) > @@ -38,6 +38,7 @@ along with GCC; see the file COPYING3. > #include "gimple-walk.h" > #include "gimplify.h" > #include "target.h" > +#include "builtins.h" > > > /* All the tuples have their operand vector (if present) at the very bottom > @@ -3018,3 +3019,20 @@ maybe_remove_unused_call_args (struct fu > update_stmt_fn (fn, stmt); > } > } > + > +/* Return true if STMT will likely expand to real call statment. */ > + > +bool > +gimple_real_call_p (gimple *stmt) > +{ > + if (gimple_code (stmt) != GIMPLE_CALL) > + return false; > + if (gimple_call_internal_p (stmt)) > + return false; > + tree decl = gimple_call_fndecl (stmt); > + if (decl && DECL_IS_BUILTIN (decl) > + && (is_simple_builtin (decl) > + || is_inexpensive_builtin (decl))) is_inexpensive_builtin includes is_simple_builtin handling and also tests decl && DECL_IS_BUILTIN properly already. > + return false; > + return true; > +} > Index: gimple.h > =================================================================== > --- gimple.h (revision 237101) > +++ gimple.h (working copy) > @@ -1525,6 +1525,7 @@ extern void preprocess_case_label_vec_fo > extern void gimple_seq_set_location (gimple_seq, location_t); > extern void gimple_seq_discard (gimple_seq); > extern void maybe_remove_unused_call_args (struct function *, gimple *); > +extern bool gimple_real_call_p (gimple *); > > /* Formal (expression) temporary table handling: multiple occurrences of > the same scalar expression are evaluated into the same temporary. */ > Index: tree-ssa-loop-ch.c > =================================================================== > --- tree-ssa-loop-ch.c (revision 237101) > +++ tree-ssa-loop-ch.c (working copy) > @@ -118,7 +118,7 @@ should_duplicate_loop_header_p (basic_bl > if (is_gimple_debug (last)) > continue; > > - if (is_gimple_call (last)) > + if (gimple_real_call_p (last)) > { > if (dump_file && (dump_flags & TDF_DETAILS)) > fprintf (dump_file, > Index: tree-ssa-loop-ivcanon.c > =================================================================== > --- tree-ssa-loop-ivcanon.c (revision 237101) > +++ tree-ssa-loop-ivcanon.c (working copy) > @@ -339,15 +339,11 @@ tree_estimate_loop_size (struct loop *lo > for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi)) > { > gimple *stmt = gsi_stmt (gsi); > - if (gimple_code (stmt) == GIMPLE_CALL) > + if (gimple_code (stmt) == GIMPLE_CALL > + && gimple_real_call_p (stmt)) gimple_code (stmt) == GIMPLE_CALL is redundant then. I'd prefer to make gimple_inexpensive_call_p take a gcall * argument and do the test at the callers though. > { > int flags = gimple_call_flags (stmt); > - tree decl = gimple_call_fndecl (stmt); > - > - if (decl && DECL_IS_BUILTIN (decl) > - && is_inexpensive_builtin (decl)) > - ; > - else if (flags & (ECF_PURE | ECF_CONST)) > + if (flags & (ECF_PURE | ECF_CONST)) > size->num_pure_calls_on_hot_path++; > else > size->num_non_pure_calls_on_hot_path++; > >