On Fri, 25 Nov 2016, Martin Jambor wrote:
> 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.
For quite some time indeed ;)
> Please commit it.
Will do after testing finished.
> > 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.
it's the only use of stmt2 though, so it must have been added for some
reason... (maybe somebody wanted to handle simple copy-propagation?!).
I'd say rip it out and thus keep stmt2 == stmt. There must be
a testcase added for this...
Richard.
