Re: [PATCH] Look through widening type conversions for possible edge assertions

[Richard Biener <richard dot guenther at gmail dot com>]

On Wed, Nov 12, 2014 at 5:17 AM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> On Tue, Nov 11, 2014 at 8:48 AM, Richard Biener
> <richard.guenther@gmail.com> wrote:
>> On Tue, Nov 11, 2014 at 1:10 PM, Patrick Palka <patrick@parcs.ath.cx> wrote:
>>> This patch is a replacement for the 2nd VRP refactoring patch.  It
>>> simply teaches VRP to look through widening type conversions when
>>> finding suitable edge assertions, e.g.
>>>
>>> bool p = x != y;
>>> int q = (int) p;
>>> if (q == 0) // new edge assert: p == 0 and therefore x == y
>>
>> I think the proper fix is to forward x != y to q == 0 instead of this one.
>> That said - the tree-ssa-forwprop.c restriction on only forwarding
>> single-uses into conditions is clearly bogus here.  I suggest to
>> relax it for conversions and compares.  Like with
>>
>> Index: tree-ssa-forwprop.c
>> ===================================================================
>> --- tree-ssa-forwprop.c (revision 217349)
>> +++ tree-ssa-forwprop.c (working copy)
>> @@ -476,7 +476,7 @@ forward_propagate_into_comparison_1 (gim
>>         {
>>           rhs0 = rhs_to_tree (TREE_TYPE (op1), def_stmt);
>>           tmp = combine_cond_expr_cond (stmt, code, type,
>> -                                       rhs0, op1, !single_use0_p);
>> +                                       rhs0, op1, false);
>>           if (tmp)
>>             return tmp;
>>         }
>>
>>
>> Thanks,
>> Richard.
>
> That makes sense.  Attached is what I have so far.  I relaxed the
> forwprop restriction in the case of comparing an integer constant with
> a comparison or with a conversion from a boolean value.  (If I allow
> all conversions, not just those from a boolean value, then a couple of
> -Wstrict-overflow faillures trigger..)  Does the change look sensible?
>  Should the logic be duplicated for the case when TREE_CODE (op1) ==
> SSA_NAME? Thanks for your help so far!

It looks good though I'd have allowed all kinds of conversions, not only
those from booleans.

If the patch tests ok with that change it is ok.

Thanks,
Richard.