Re: [PATCH] Fix PR78515

[Martin Jambor <mjambor at suse dot cz>]

Hi,

On Fri, Nov 25, 2016 at 12:01:38PM +0100, Richard Biener wrote:
> 
> I am testing the following to beat some sanity into 
> compute_complex_assign_jump_func.

That the function does not handle ternary operations (did we have them
since the beginning?) is clearly my fault and the patch is fine.
Please commit it.

> There's still that odd 'stmt2'
> hanging around that gets set to sth else than stmt with
> 
>   op1 = gimple_assign_rhs1 (stmt);
> 
>   if (TREE_CODE (op1) == SSA_NAME)
>     {
>       if (SSA_NAME_IS_DEFAULT_DEF (op1))
>         index = ipa_get_param_decl_index (info, SSA_NAME_VAR (op1));
>       else
>         {
>           index = load_from_param (fbi, info->descriptors,
>                                    SSA_NAME_DEF_STMT (op1));
>           stmt2 = SSA_NAME_DEF_STMT (op1);
> 
> I assume that the original code wanted to restrict its processing
> to unary RHS of 'stmt' but still this "skips" arbitrary unary
> operations in 'stmt'?  But maybe I'm not understanding jump functions
> here.  If we have
> 
>   _2 = -param_1(D);
>   _3 = ~_2;
> 
> and stmt is _3 then we create a unary pass through JF with - (and the ~
> gets lost?).
>

It definitely looks like that.  Unary arithmetic jump functions have
been added only recently with the IPA VRP propagation and I remember
staring at the stmt2 thingy for a while during review but then
apparently I forgot about it.

It seems to me that the check should refer to stmt, I will do that and
see what breaks and how the IL looks at that point and then decide
where to go from there.

Thanks,

Martin