Richard Biener <richard.guent...@gmail.com> writes:
> On Wed, Jul 11, 2018 at 8:48 AM Aldy Hernandez <al...@redhat.com> wrote:
>>
>> Hmmm, I think we can do better, and since this hasn't been reviewed yet,
>> I don't think anyone will mind the adjustment to the patch ;-).
>>
>> I really hate int_const_binop_SOME_RANDOM_NUMBER. We should abstract
>> them into properly named poly_int_binop, wide_int_binop, and tree_binop,
>> and then use a default argument for int_const_binop() to get things going.
>>
>> Sorry for more changes in flight, but I thought we could benefit from
>> more cleanups :).
>>
>> OK for trunk pending tests?
>
> Much of GCC pre-dates function overloading / default args ;)
>
> Looks OK but can you please rename your tree_binop to int_cst_binop?
> Or maybe inline it into int_const_binop, also sharing the force_fit_type ()
> tail with poly_int_binop?
>
> What about mixed INTEGER_CST / poly_int constants? Shouldn't it
> be
>
> if (neither-poly-nor-integer-cst (arg1 || arg2))
> return NULL_TREE;
> if (poly_int_tree (arg1) || poly_int_tree (arg2))
> poly-int-stuff
> else if (INTEGER_CST && INTEGER_CST)
> wide-int-stuff
>
> ? I see that is a pre-existing issue but if you are at refactoring...
> wi::to_poly_wide should handle INTEGER_CST operands just fine
> I hope.
Don't think it's a preexisting issue. poly_int_tree_p returns true
for anything that can be represented as a poly_int, i.e. both
INTEGER_CST and POLY_INT_CST. (It wouldn't really make sense to
ask whether something could *only* be represented as a POLY_INT_CST.)
So:
if (poly_int_tree_p (arg1) && poly_int_tree_p (arg2))
{
poly_wide_int res;
bool overflow;
tree type = TREE_TYPE (arg1);
signop sign = TYPE_SIGN (type);
switch (code)
{
case PLUS_EXPR:
res = wi::add (wi::to_poly_wide (arg1),
wi::to_poly_wide (arg2), sign, &overflow);
break;
handles POLY_INT_CST + POLY_INT_CST, POLY_INT_CST + INTEGER_CST and
INTEGER_CST + POLY_INT_CST.
Thanks,
Richard
