On Fri, Sep 27, 2013 at 1:20 AM, Richard Biener <richard.guent...@gmail.com> wrote: > On Fri, Sep 27, 2013 at 12:23 AM, Jan Hubicka <hubi...@ucw.cz> wrote: >>> Hi, >>> >>> builtin_expect should be a NOP in size_estimation. Indeed, the call >>> stmt itself is 0 weight in size and time. But it may introduce >>> an extra relation expr which has non-zero size/time. The end result >>> is: for w/ and w/o builtin_expect, we have different size/time estimation >>> for early inlining. >>> >>> This patch fixes this problem. >>> >>> -Rong >> >>> 2013-09-26 Rong Xu <x...@google.com> >>> >>> * ipa-inline-analysis.c (estimate_function_body_sizes): fix >>> the size estimation for builtin_expect. >> >> This seems fine with an comment in the code what it is about. >> I also think we want to support mutiple builtin_expects in a BB so perhaps >> we want to have pointer set of statements to ignore? >> >> To avoid spagetti code, please just move the new logic into separate >> functions. > > Looks like this could use tree-ssa.c:walk_use_def_chains (please > change its implementation as necessary, make it C++, etc. - you will > be the first user again). >
Thanks for the suggestion. But it seems walk_use_def_chains() was removed by r201951. -Rong > Richard. > >> Honza >>> >>> Index: ipa-inline-analysis.c >>> =================================================================== >>> --- ipa-inline-analysis.c (revision 202638) >>> +++ ipa-inline-analysis.c (working copy) >>> @@ -2266,6 +2266,8 @@ estimate_function_body_sizes (struct cgraph_node * >>> /* Estimate static overhead for function prologue/epilogue and >>> alignment. */ >>> int overhead = PARAM_VALUE (PARAM_INLINE_FUNCTION_OVERHEAD_SIZE); >>> int size = overhead; >>> + gimple fix_expect_builtin; >>> + >>> /* Benefits are scaled by probability of elimination that is in range >>> <0,2>. */ >>> basic_block bb; >>> @@ -2359,14 +2361,73 @@ estimate_function_body_sizes (struct cgraph_node * >>> } >>> } >>> >>> + fix_expect_builtin = NULL; >>> for (bsi = gsi_start_bb (bb); !gsi_end_p (bsi); gsi_next (&bsi)) >>> { >>> gimple stmt = gsi_stmt (bsi); >>> + if (gimple_call_builtin_p (stmt, BUILT_IN_EXPECT)) >>> + { >>> + tree var = gimple_call_lhs (stmt); >>> + tree arg = gimple_call_arg (stmt, 0); >>> + use_operand_p use_p; >>> + gimple use_stmt; >>> + bool match = false; >>> + bool done = false; >>> + gcc_assert (var && arg); >>> + gcc_assert (TREE_CODE (var) == SSA_NAME); >>> + >>> + while (TREE_CODE (arg) == SSA_NAME) >>> + { >>> + gimple stmt_tmp = SSA_NAME_DEF_STMT (arg); >>> + switch (gimple_assign_rhs_code (stmt_tmp)) >>> + { >>> + case LT_EXPR: >>> + case LE_EXPR: >>> + case GT_EXPR: >>> + case GE_EXPR: >>> + case EQ_EXPR: >>> + case NE_EXPR: >>> + match = true; >>> + done = true; >>> + break; >>> + case NOP_EXPR: >>> + break; >>> + default: >>> + done = true; >>> + break; >>> + } >>> + if (done) >>> + break; >>> + arg = gimple_assign_rhs1 (stmt_tmp); >>> + } >>> + >>> + if (match && single_imm_use (var, &use_p, &use_stmt)) >>> + { >>> + if (gimple_code (use_stmt) == GIMPLE_COND) >>> + { >>> + fix_expect_builtin = use_stmt; >>> + } >>> + } >>> + >>> + /* we should see one builtin_expert call in one bb. */ >>> + break; >>> + } >>> + } >>> + >>> + for (bsi = gsi_start_bb (bb); !gsi_end_p (bsi); gsi_next (&bsi)) >>> + { >>> + gimple stmt = gsi_stmt (bsi); >>> int this_size = estimate_num_insns (stmt, &eni_size_weights); >>> int this_time = estimate_num_insns (stmt, &eni_time_weights); >>> int prob; >>> struct predicate will_be_nonconstant; >>> >>> + if (stmt == fix_expect_builtin) >>> + { >>> + this_size--; >>> + this_time--; >>> + } >>> + >>> if (dump_file && (dump_flags & TDF_DETAILS)) >>> { >>> fprintf (dump_file, " "); >>