Handle non-SSA arguments in ipa-inline-analysis better

Hi,
this patch extends handling of non-SSA arguments to bultin_constant_p and
execution predicates.

Bootstrapped/regtested x86_64-linux, will commit it shortly.

Honza

	* ipa-inline-analysis.c (set_cond_stmt_execution_predicate): Allow
	handled components in parameter of builtin_constant_p.
	(will_be_nonconstant_predicate): Allow loads of non-SSA parameters.

On Thu, Sep 22, 2011 at 12:51 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
> Hi,
> this patch extends handling of non-SSA arguments to bultin_constant_p and
> execution predicates.
>
> Bootstrapped/regtested x86_64-linux, will commit it shortly.
>
> Honza
>
>        * ipa-inline-analysis.c (set_cond_stmt_execution_predicate): Allow
>        handled components in parameter of builtin_constant_p.
>        (will_be_nonconstant_predicate): Allow loads of non-SSA parameters.
> Index: ipa-inline-analysis.c
> ===================================================================
> --- ipa-inline-analysis.c       (revision 179046)
> +++ ipa-inline-analysis.c       (working copy)
> @@ -1202,6 +1202,7 @@ set_cond_stmt_execution_predicate (struc
>   gimple set_stmt;
>   tree op2;
>   tree parm;
> +  tree base;
>
>   last = last_stmt (bb);
>   if (!last
> @@ -1252,7 +1253,8 @@ set_cond_stmt_execution_predicate (struc
>       || gimple_call_num_args (set_stmt) != 1)
>     return;
>   op2 = gimple_call_arg (set_stmt, 0);
> -  parm = unmodified_parm (set_stmt, op2);
> +  base = get_base_address (op2);
> +  parm = unmodified_parm (set_stmt, base ? base : op2);

You don't care for a NULL get_base_address return in the other
place you added it.  I think you should instead bail out on a
NULL return from said.

Note that get_base_address will not strip things down to an
SSA name pointer but will return a MEM[ptr, offset] for that.
But I suppose unmodified_parm / ipa_get_parm_decl_index
already cares for that (dereference of argument)?

>   if (!parm)
>     return;
>   index = ipa_get_param_decl_index (info, parm);
> @@ -1433,6 +1435,7 @@ will_be_nonconstant_predicate (struct ip
>   ssa_op_iter iter;
>   tree use;
>   struct predicate op_non_const;
> +  bool is_load;
>
>   /* What statments might be optimized away
>      when their arguments are constant
> @@ -1443,11 +1446,29 @@ will_be_nonconstant_predicate (struct ip
>       && gimple_code (stmt) != GIMPLE_SWITCH)
>     return p;
>
> -  /* Stores and loads will stay anyway.
> -     TODO: Constant memory accesses could be handled here, too.  */
> -  if (gimple_vuse (stmt))
> +  /* Stores will stay anyway.  */
> +  if (gimple_vdef (stmt))
>     return p;
>
> +  is_load = gimple_vuse (stmt) != NULL;
> +
> +  /* Loads can be optimized when the value is known.  */
> +  if (is_load)
> +    {
> +      tree op = gimple_assign_rhs1 (stmt);
> +      tree base = get_base_address (op);
> +      tree parm;
> +
> +      gcc_assert (gimple_assign_single_p (stmt));
> +      if (!base)
> +       return p;
> +      parm = unmodified_parm (stmt, base);
> +      if (!parm )
> +       return p;
> +      if (ipa_get_param_decl_index (info, parm) < 0)
> +       return p;
> +    }
> +
>   /* See if we understand all operands before we start
>      adding conditionals.  */
>   FOR_EACH_SSA_TREE_OPERAND (use, stmt, iter, SSA_OP_USE)
> @@ -1466,6 +1487,15 @@ will_be_nonconstant_predicate (struct ip
>       return p;
>     }
>   op_non_const = false_predicate ();
> +  if (is_load)
> +    {
> +      tree parm = unmodified_parm
> +                   (stmt, get_base_address (gimple_assign_rhs1 (stmt)));
> +      p = add_condition (summary,
> +                        ipa_get_param_decl_index (info, parm),
> +                        IS_NOT_CONSTANT, NULL);
> +      op_non_const = or_predicates (summary->conds, &p, &op_non_const);
> +    }
>   FOR_EACH_SSA_TREE_OPERAND (use, stmt, iter, SSA_OP_USE)
>     {
>       tree parm = unmodified_parm (stmt, use);
>

> 
> You don't care for a NULL get_base_address return in the other
> place you added it.  I think you should instead bail out on a
> NULL return from said.
> 
> Note that get_base_address will not strip things down to an
> SSA name pointer but will return a MEM[ptr, offset] for that.
> But I suppose unmodified_parm / ipa_get_parm_decl_index
> already cares for that (dereference of argument)?

I am not (yet) tracking values passed by reference.  We do not have jump
functions for these, so there is limited use for it until Martin adds them.

So basically I need to track the SSA names, the cases where scalar variable is
not in SSA because its address is taken. (i.e. parm_decl alone) and the
accesses into aggregates that are parameters. I.e. param.field or param[0].

Except for first case where I already have SSA name, I should end
up with get_base_address returning the actual PARM_DECL, right?

The reason why I am not checking for NULL return is that the code first
verifies that it understand all the parameters and then it starts building the
predicates (to avoid adding contions that won't be used at the end).
So we know get_base_address will return non-NULL.

Thanks for comments!
Honza

On Sun, Sep 25, 2011 at 12:25 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
>>
>> You don't care for a NULL get_base_address return in the other
>> place you added it.  I think you should instead bail out on a
>> NULL return from said.
>>
>> Note that get_base_address will not strip things down to an
>> SSA name pointer but will return a MEM[ptr, offset] for that.
>> But I suppose unmodified_parm / ipa_get_parm_decl_index
>> already cares for that (dereference of argument)?
>
> I am not (yet) tracking values passed by reference.  We do not have jump
> functions for these, so there is limited use for it until Martin adds them.
>
> So basically I need to track the SSA names, the cases where scalar variable is
> not in SSA because its address is taken. (i.e. parm_decl alone) and the
> accesses into aggregates that are parameters. I.e. param.field or param[0].
>
> Except for first case where I already have SSA name, I should end
> up with get_base_address returning the actual PARM_DECL, right?

Yes.

> The reason why I am not checking for NULL return is that the code first
> verifies that it understand all the parameters and then it starts building the
> predicates (to avoid adding contions that won't be used at the end).
> So we know get_base_address will return non-NULL.

So why check in the 2nd case?

> Thanks for comments!
> Honza
>