From: Richard Guenther <richard.guenther@gmail.com>
To: Kai Tietz <ktietz70@googlemail.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>, Andrew Pinski <pinskia@gmail.com>
Subject: Re: [patch tree-optimization]: Fix for PR 45397 part 2 of 2
Date: Wed, 21 Mar 2012 10:06:00 -0000 [thread overview]
Message-ID: <CAFiYyc0LtHXFD+Ut6zMvhq5OA29ixVV0hCg0oWe1nosRH6rRHQ@mail.gmail.com> (raw)
In-Reply-To: <CAEwic4YN46J2rSU7obYgVAhfhBVH7ofWiMi+k-ST7m84_ZnQ1w@mail.gmail.com>
On Wed, Mar 21, 2012 at 10:56 AM, Kai Tietz <ktietz70@googlemail.com> wrote:
> 2012/3/21 Richard Guenther <richard.guenther@gmail.com>:
>> On Thu, Mar 15, 2012 at 3:45 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>>> 2012/3/15 Richard Guenther <richard.guenther@gmail.com>:
>>>> On Thu, Mar 15, 2012 at 2:46 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>>>>> 2012/3/15 Richard Guenther <richard.guenther@gmail.com>:
>>>>>> On Thu, Mar 15, 2012 at 2:09 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> this is the second part of the patch for this problem. It adds some
>>>>>>> basic simplifications for ==/!=
>>>>>>> comparisons for eliminating redudant operands.
>>>>>>>
>>>>>>> It adds the following patterns:
>>>>>>> -X ==/!= Z - X -> Z ==/!= 0.
>>>>>>> ~X ==/!= Z ^ X -> Z ==/!= ~0
>>>>>>> X ==/!= X - Y -> Y == 0
>>>>>>> X ==/!= X + Y -> Y == 0
>>>>>>> X ==/!= X ^ Y -> Y == 0
>>>>>>> (X - Y) ==/!= (Z - Y) -> X ==/!= Z
>>>>>>> (Y - X) ==/!= (Y - Z) -> X ==/!= Z
>>>>>>> (X + Y) ==/!= (X + Z) -> Y ==/!= Z
>>>>>>> (X + Y) ==/!= (Z + X) -> Y ==/!= Z
>>>>>>> (X ^ Y) ==/!= (Z ^ X) -> Y ==/!= Z
>>>>>>
>>>>>> Can you re-base this patch to work without the previous one? Also
>>>>>> please coordinate with Andrew. Note that all of these(?) simplifications
>>>>>> are already done by fold_comparison which we could share if you'd split
>>>>>> out the EXPR_P op0/op1 cases with separated operands/code.
>>>>>>
>>>>>> Richard.
>>>>>
>>>>> Hmm, fold_comparison doesn't do the same thing as it checks for
>>>>> possible overflow. This is true for comparisons not being ==/!= or
>>>>> having operands of none-integral-type. But for ==/!= with integral
>>>>> typed arguments the overflow doesn't matter at all. And exactly this
>>>>> is what patch implements here.
>>>>
>>>> fold_comparison does not check for overflow for ==/!=.
>>>>
>>>>> This optimization of course is just desired in non-AST form, as we
>>>>> otherwise loose information in FE. Therefore I didn't added it to
>>>>> fold_const.
>>>>
>>>> Which pieces are not already in fold-const btw? forwprop already
>>>> re-constructs trees for the defs of the lhs/rhs of a comparison.
>>>>
>>>> Richard.
>>>
>>> I have tried to use here instead a call to fold_build2 instead, and I
>>> had to notice that it didn't optimized a single case (beside the - and
>>> ~ case on both sides).
>>>
>>> I see in fold const for example in the pattern 'X +- C1 CMP Y +- C2'
>>> to 'X CMP Y +- C2 +- C1' explicit the check for it.
>>>
>>> ...
>>> /* Transform comparisons of the form X +- C1 CMP Y +- C2 to
>>> X CMP Y +- C2 +- C1 for signed X, Y. This is valid if
>>> the resulting offset is smaller in absolute value than the
>>> original one. */
>>> if (TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (arg0))
>>> && (TREE_CODE (arg0) == PLUS_EXPR || TREE_CODE (arg0) == MINUS_EXPR)
>>> ...
>>
>> Because the transform is not valid if Y +- C2 +- C1 overflows. It is not valid
>> because overflow is undefined, not because the comparison would do the
>> wrong thing. You'd have to change the addition to unsigned.
>>
>>> The same for pattern X +- C1 CMP C2 to X CMP C2 +- C1.
>>
>> Well, this is obviously just a missed optimization in fold-const.c then. Mind
>> conditionalizing the overflow check to codes not NE_EXPR or EQ_EXPR?
>>
>>> The cases for '(X + Y) ==/!= (Z + X)' and co have the same issue or
>>> are simply not present.
>>
>> That's true. I suppose they were considered too special to worry about.
>> Did you see these cases in real code?
>>
>>> Sorry fold_const doesn't cover this at all.
>>
>> It covers part of it.
>>
>>> Kai
>
> Sure, the test code shown in this patch isn't that unusual.
> Especially in gimple (by using different statements) such construct
> are happening.
>
> Eg.:
>
> int f1 (int a, int b, int c)
> {
> if ((a + b) == (c + a))
> return 1;
> return 0;
> }
>
> int f2 (int a, int b, int c)
> {
> if ((a ^ b) == (a ^ c))
> return 1;
> return 0;
> }
>
>
> int f2 (int a, int b)
> {
> if (-a == (b - a))
> return 1;
> return 0;
> }
>
> In all those cases the use of variable should be optimized out.
> Instead we are producing pretty weak code for those cases.
True, I agree we should try to handle these. Did you talk to Andrew
with respect to the gimple-combining thing he is working on?
Richard.
> Kai
prev parent reply other threads:[~2012-03-21 10:06 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-03-15 13:09 Kai Tietz
2012-03-15 13:36 ` Richard Guenther
2012-03-15 13:47 ` Kai Tietz
2012-03-15 14:16 ` Richard Guenther
2012-03-15 14:45 ` Kai Tietz
2012-03-21 9:46 ` Richard Guenther
2012-03-21 9:57 ` Kai Tietz
2012-03-21 10:06 ` Richard Guenther [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAFiYyc0LtHXFD+Ut6zMvhq5OA29ixVV0hCg0oWe1nosRH6rRHQ@mail.gmail.com \
--to=richard.guenther@gmail.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=ktietz70@googlemail.com \
--cc=pinskia@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).