Thanks for the comments.
Regards,
Wei.
On Thu, Nov 21, 2013 at 12:48 AM, Bin.Cheng <amker.ch...@gmail.com> wrote:
> I don't know very much about the problem but willing to study since I
> am looking into IVO recently :)
>
>> --- tree-ssa-loop-ivopts.c (revision 204792)
>> +++ tree-ssa-loop-ivopts.c (working copy)
>> @@ -135,6 +135,8 @@ struct iv
>> tree ssa_name; /* The ssa name with the value. */
>> bool biv_p; /* Is it a biv? */
>> bool have_use_for; /* Do we already have a use for it? */
>> + bool builtin_mem_param; /* Used as param of a builtin, so it could not be
>> + removed by remove_unused_ivs. */
>
> As comment below, address parameter may be not limited to builtin
> function only, how about a variable name more generic?
Ok, I will change it to a different name.
>
>> unsigned use_id; /* The identifier in the use if it is the case. */
>> };
>>
>> @@ -952,6 +954,7 @@ alloc_iv (tree base, tree step)
>> iv->step = step;
>> iv->biv_p = false;
>> iv->have_use_for = false;
>> + iv->builtin_mem_param = false;
>> iv->use_id = 0;
>> iv->ssa_name = NULL_TREE;
>>
>> @@ -1874,13 +1877,36 @@ find_invariants_stmt (struct ivopts_data
>> }
>> }
>>
>> +/* Find whether the Ith param of the BUILTIN is a mem
>> + reference. If I is -1, it returns whether the BUILTIN
>> + contains any mem reference type param. */
>> +
>> +static bool
>> +builtin_has_mem_ref_p (gimple builtin, int i)
>> +{
>> + tree fndecl = gimple_call_fndecl (builtin);
>> + if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL)
>> + {
>> + switch (DECL_FUNCTION_CODE (fndecl))
>> + {
>> + case BUILT_IN_PREFETCH:
>> + if (i == -1 || i == 0)
>> + return true;
>> + }
> This switch looks strange, could be refactored I think.
>
I am not sure if there will be more builtins coming in the func, so I
use switch case here.
>> + }
>> + else if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_MD)
>> + return targetm.builtin_has_mem_ref_p ((int) DECL_FUNCTION_CODE
>> (fndecl), i);
>> +
>> + return false;
>> +}
>> +
>> /* Finds interesting uses of induction variables in the statement STMT. */
>>
>> static void
>> find_interesting_uses_stmt (struct ivopts_data *data, gimple stmt)
>> {
>> struct iv *iv;
>> - tree op, *lhs, *rhs;
>> + tree op, *lhs, *rhs, callee;
>> ssa_op_iter iter;
>> use_operand_p use_p;
>> enum tree_code code;
>> @@ -1937,6 +1963,74 @@ find_interesting_uses_stmt (struct ivopt
>>
>> call (memory). */
>> }
>> + else if (is_gimple_call (stmt)
>> + && (callee = gimple_call_fndecl (stmt))
>> + && is_builtin_fn (callee)
>> + && builtin_has_mem_ref_p (stmt, -1))
>> + {
>
> I noticed the preceding comments about call(memory), is your change a
> specific case of the mention one?
>
Yes, I will fix it.
>> + size_t i;
>> + for (i = 0; i < gimple_call_num_args (stmt); i++)
>> + {
>> + if (builtin_has_mem_ref_p (stmt, i))
>> + {
>> + gimple def, g;
>> + gimple_seq seq = NULL;
>> + tree type, mem, addr, rhs;
>> + tree *arg = gimple_call_arg_ptr (stmt, i);
>> + location_t loc = gimple_location (stmt);
>> + gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
>> +
>> + if (TREE_CODE (*arg) != SSA_NAME)
>> + continue;
>> +
>> + def = SSA_NAME_DEF_STMT (*arg);
>> + gcc_assert (gimple_code (def) == GIMPLE_PHI
>> + || is_gimple_assign (def));
>> + /* Suppose we have the case:
>> + arg = expr;
>> + call (arg)
>> + If the expr is not like the form: &MEM(...), change it to:
>> + arg = expr;
>> + tmp = &MEM(arg);
>> + call(tmp);
>> + then try to find interesting uses address in MEM(arg). */
>> + if (is_gimple_assign (def)
>> + && (rhs = gimple_assign_rhs1(def))
>> + && TREE_CODE (rhs) == ADDR_EXPR
>> + && REFERENCE_CLASS_P (TREE_OPERAND (rhs, 0)))
>> + {
>> + iv = get_iv (data, *arg);
>> + if (iv && !iv->builtin_mem_param)
>> + iv->builtin_mem_param = true;
>> +
>> + find_interesting_uses_address (data, def,
>> + &TREE_OPERAND (rhs, 0));
>> + }
>> + else
>> + {
>> + mem = build2 (MEM_REF, TREE_TYPE (*arg), *arg,
>> + build_int_cst (TREE_TYPE (*arg), 0));
>> + type = build_pointer_type (TREE_TYPE (*arg));
>> + addr = build1 (ADDR_EXPR, type, mem);
>> + g = gimple_build_assign_with_ops (ADDR_EXPR,
>> + make_ssa_name (type,
>> NULL),
>> + addr, NULL);
>> + gimple_call_set_arg (stmt, i, gimple_assign_lhs (g));
>> + update_stmt (stmt);
>> + gimple_set_location (g, loc);
>> + gimple_seq_add_stmt_without_update (&seq, g);
>> + gsi_insert_seq_before (&gsi, seq, GSI_SAME_STMT);
>> + find_interesting_uses_address (data, g,
>> &TREE_OPERAND (addr, 0));
>
> This would be the only code changes gimple before iv use rewriting and
> seems not a good practice.
Yes. Zdenek mentioned the same. I will think how to fix it.
>
>> + }
>> + }
>> + else
>> + {
>> + tree arg = gimple_call_arg (stmt, i);
>> + find_interesting_uses_op (data, arg);
>> + }
>> + }
>> + return;
>> + }
>>
>> if (gimple_code (stmt) == GIMPLE_PHI
>> && gimple_bb (stmt) == data->current_loop->header)
>> @@ -6507,10 +6601,10 @@ remove_unused_ivs (struct ivopts_data *d
>> && !integer_zerop (info->iv->step)
>> && !info->inv_id
>> && !info->iv->have_use_for
>> + && !info->iv->builtin_mem_param
>> && !info->preserve_biv)
>> {
>
> Thanks,
> bin
>
> --
> Best Regards.
