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

Patrick Palka patrick@parcs.ath.cx
Sun Nov 16 05:05:00 GMT 2014


On Wed, Nov 12, 2014 at 3:38 AM, Richard Biener
<richard.guenther@gmail.com> wrote:
> 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.

Sadly changing the patch to propagate all kinds of conversions, not
only just those from booleans, introduces regressions that I don't
know how to adequately fix.



More information about the Gcc-patches mailing list