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).
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, " ");
>
