This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Look through widening type conversions for possible edge assertions
- From: Richard Biener <richard dot guenther at gmail dot com>
- To: Patrick Palka <patrick at parcs dot ath dot cx>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Sun, 16 Nov 2014 14:52:27 +0100
- Subject: Re: [PATCH] Look through widening type conversions for possible edge assertions
- Authentication-results: sourceware.org; auth=none
- References: <CA+C-WL-fbKDbNmMr0ah-72QQmYw7O_eY3uKpf5Z7UtMw8-7D4g at mail dot gmail dot com> <1415707831-652-1-git-send-email-patrick at parcs dot ath dot cx> <CAFiYyc0Ved1EyHPA4HXzkrrHeo=JhYJg4d6JZ7AasktoVn=VxA at mail dot gmail dot com> <CA+C-WL928gZXwjR0mXiDOsUWis8AVQ3UNi85TjGieJ8p39HsFw at mail dot gmail dot com> <CAFiYyc2WdT79QZqwpR=fceARqCiYEouBrKf_Lm_dFmCwWpCCsw at mail dot gmail dot com> <CA+C-WL9dE9OEu3-_zRQ8mV_Ly=rmfbkZoffi7mKfiusW-sQm-w at mail dot gmail dot com>
On November 16, 2014 5:22:26 AM CET, Patrick Palka <patrick@parcs.ath.cx> wrote:
>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.
OK. The original patch propagating only bool conversions is ok then. Can you list the failures you have seen when propagating more?
Thanks,
Richard.