On 07/19/2018 03:06 AM, Aldy Hernandez wrote:
>
>
> On 07/19/2018 04:18 AM, Richard Biener wrote:
>> On Wed, Jul 18, 2018 at 2:05 PM Aldy Hernandez <al...@redhat.com> wrote:
>>>
>>> Hi again!
>>>
>>> Well, since this hasn't been reviewed and I'm about to overhaul the
>>> TYPE_OVERFLOW_WRAPS code anyhow, might as well lump it all in one patch.
>>>
>>> On 07/16/2018 09:19 AM, Aldy Hernandez wrote:
>>>> Howdy!
>>>>
>>>> I've abstracted out the cross product calculations into its own
>>>> function, and have adapted it to deal with wide ints so it's more
>>>> reusable. It required some shuffling around, and implementing things a
>>>> bit different, but things should be behave as before.
>>>>
>>>> I also renamed vrp_int_const_binop to make its intent clearer,
>>>> especially now that it's really just a wrapper to wide_int_binop that
>>>> deals with overflow.
>>>>
>>>> (If wide_int_binop_overflow is generally useful, perhaps we could merge
>>>> it with wide_int_overflow.)
>>>
>>> This is the same as the previous patch, plus I'm abstracting the
>>> TYPE_OVERFLOW_WRAPS code as well. With this, the code dealing with
>>> MULT_EXPR in vrp gets reduced to handling value_range specific stuff.
>>> Yay code re-use!
>>>
>>> A few notes:
>>>
>>> This is dead code. I've removed it:
>>>
>>> - /* If we have an unsigned MULT_EXPR with two VR_ANTI_RANGEs,
>>> - drop to VR_VARYING. It would take more effort to compute a
>>> - precise range for such a case. For example, if we have
>>> - op0 == 65536 and op1 == 65536 with their ranges both being
>>> - ~[0,0] on a 32-bit machine, we would have op0 * op1 == 0, so
>>> - we cannot claim that the product is in ~[0,0]. Note that we
>>> - are guaranteed to have vr0.type == vr1.type at this
>>> - point. */
>>> - if (vr0.type == VR_ANTI_RANGE
>>> - && !TYPE_OVERFLOW_UNDEFINED (expr_type))
>>> - {
>>> - set_value_range_to_varying (vr);
>>> - return;
>>> - }
>>>
>>> Also, the vrp_int typedef has a weird name, especially when we have
>>> widest2_int in gimple-fold.c that does the exact thing. I've moved the
>>> common code to wide-int.h and tree.h so we can all share :).
>>>
>>> At some point we could move the wide_int_range* and wide_int_binop* code
>>> into its own file.
>>
>> Yes.
>
> Sometime within the next couple rounds I'll come up with a file name
> that doesn't hurt my eyes. It seems that the hardest part of
> programming is actually coming up with sensible file and variable names
> :-/.
I recall reading somewhere that once you have the right
function/method/variable names you're 90% of the way to having the
problem solved. I don't totally agree, but I can see the point the
original author of the statement was trying to get across.
Internally when refactoring I usually start with calling stuff "foo",
"bar", "doit" and friends until I'm fairly happy with what got carved
out then I try to figure out reasonable names.
Jeff
