On Mon, 29 Feb 2016, Jakub Jelinek wrote:
> On Mon, Feb 29, 2016 at 04:26:12PM +0100, Richard Biener wrote:
> > *************** get_unary_op (tree name, enum tree_code
> > *** 621,626 ****
> > --- 641,680 ----
> > return NULL_TREE;
> > }
> >
> > + /* Return true if OP1 and OP2 have the same value if casted to either
> > type. */
> > +
> > + static bool
> > + ops_equal_values_p (tree op1, tree op2)
> > + {
> > + if (op1 == op2)
> > + return true;
> > +
> > + if (TREE_CODE (op1) == SSA_NAME)
> > + {
> > + gimple *stmt = SSA_NAME_DEF_STMT (op1);
> > + if (gimple_nop_conversion_p (stmt))
> > + {
> > + op1 = gimple_assign_rhs1 (stmt);
> > + if (op1 == op2)
> > + return true;
> > + }
> > + }
> > +
> > + if (TREE_CODE (op2) == SSA_NAME)
> > + {
> > + gimple *stmt = SSA_NAME_DEF_STMT (op2);
> > + if (gimple_nop_conversion_p (stmt))
> > + {
> > + op2 = gimple_assign_rhs1 (stmt);
> > + if (op1 == op2)
> > + return true;
> > + }
> > + }
>
> This will not work if you have:
> x_1 = (nop) x_0;
> x_2 = (nop) x_1;
> and op1 is x_1 and op2 is x_2 (but will work
> if op1 is x_2 and op1 is x_1).
Hmm, I expected CSE / nop combining to always simplify the above
but yes, given we want to catch IVOPTs introduced expressions here
and no CSE / forwprop between it and the reassoc it makes sense
to catch that as well.
> Wouldn't it be better to also remember the original
> tree orig_op1 = op1;
> at the beginning and in the last comparison do
> if (op1 == op2 || orig_op1 == op2)
> ?
Will do as followup now.
Thanks,
Richard.
