WARNING: This server is unstable and will be retired in the next days. If you want to keep this forum available, please request immediately a migration on the Nabble Support forum. Forums that don't receive any migration request will be deleted forever.

 « Return to Thread: [Patch] PR 51938: extend ifcombine

Re: [Patch] PR 51938: extend ifcombine

by Marc Glisse-6 :: Rate this Message:

| View in Thread

On Tue, 24 Jul 2012, Richard Guenther wrote:

> On Mon, Jul 23, 2012 at 10:58 PM, Marc Glisse <marc.glisse@...> wrote:
>> On Wed, 20 Jun 2012, Richard Guenther wrote:
>>
>>> On Sun, Jun 10, 2012 at 4:16 PM, Marc Glisse <marc.glisse@...> wrote:
>>>>
>>>> Hello,
>>>>
>>>> currently, tree-ssa-ifcombine handles pairs of imbricated "if"s that
>>>> share
>>>> the same then branch, or the same else branch. There is no particular
>>>> reason
>>>> why it couldn't also handle the case where the then branch of one is the
>>>> else branch of the other, which is what I do here.
>>>>
>>>> Any comments?
>>>
>>>
>>> The general idea looks good, but I think the patch is too invasive.  As
>>> far
>>> as I can see the only callers with a non-zero 'inv' argument come from
>>> ifcombine_ifnotorif and ifcombine_ifnotandif (and both with inv == 2).
>>> I would rather see a more localized patch that makes use of
>>> invert_tree_comparison to perform the inversion on the call arguments
>>> of maybe_fold_and/or_comparisons.
>>
>>
>> Hello,
>>
>> I finally went back to this version (which is where I started from, as shown
>> in the PR). It is not very satisfying because:
>>
>> * some bit tests could also be optimized (more generally, grouping a&&b and
>> !a&&b on one side and a||b and !a||b on the other side is rather arbitrary),
>>
>> * -ftrapping-math makes it useless for floating point,
>>
>> but I guess it is better than nothing. Handling traps correctly is
>> complicated because the current code is already a bit bogus (see
>> http://gcc.gnu.org/ml/gcc-patches/2012-07/msg00924.html for an example), and
>> even the definition of -ftrapping-math is not clear (
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53805 ).
>>
>> If defaults are ever reconsidered, my default flags include -frounding-math
>> -fno-trapping-math.
>>
>
> This patch is ok if bootstrapped / regtested properly.

Thanks. And sorry for forgetting to mention it was bootstrapped and
regtested.

Note that Andrew Pinski posted a patch:
http://gcc.gnu.org/ml/gcc-patches/2012-07/msg01167.html

which does something quite similar, so I will wait a bit before committing
anything.

--
Marc Glisse

 « Return to Thread: [Patch] PR 51938: extend ifcombine