diff mbox

PR 51938: extend ifcombine

Message ID alpine.DEB.2.02.1206101603270.3241@laptop-mg.saclay.inria.fr
State New
Headers show

Commit Message

Marc Glisse June 10, 2012, 2:16 p.m. UTC 
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?


2012-06-10  Marc Glisse  <marc.glisse@inria.fr>

gcc/
         PR tree-optimization/51938
 	* fold-const.c (combine_comparisons): Extra argument. Handle inverted
 	conditions.
         (fold_truth_andor_1): Update call to combine_comparisons.
         * gimple-fold.c (swap12): New function.
         (and_comparisons_1): Extra argument. Handle inverted conditions.
         (and_var_with_comparison_1): Update call to and_comparisons_1.
         (maybe_fold_and_comparisons): Extra argument. Update call to
 	and_comparisons_1.
         (or_comparisons_1): Extra argument. Handle inverted conditions.
         (or_var_with_comparison_1): Update call to or_comparisons_1.
         (maybe_fold_or_comparisons): Extra argument. Update call to
 	or_comparisons_1.
 	* tree-ssa-ifcombine.c (ifcombine_ifnotandif): New function.
 	(ifcombine_ifnotorif): New function.
 	(tree_ssa_ifcombine_bb): Call them.
 	(ifcombine_iforif): Update call to maybe_fold_or_comparisons.
 	(ifcombine_ifandif): Update call to maybe_fold_and_comparisons.
 	* tree-ssa-reassoc.c (eliminate_redundant_comparison): Update calls to
 	maybe_fold_or_comparisons and maybe_fold_and_comparisons.
 	* tree-if-conv.c (fold_or_predicates): Update call to
 	maybe_fold_or_comparisons.
         * gimple.h (maybe_fold_and_comparisons): Match gimple-fold.c prototype.
         (maybe_fold_or_comparisons): Likewise.
         * tree.h (combine_comparisons): Match fold-const.c prototype.

gcc/testsuite/
         PR tree-optimization/51938
 	* gcc.dg/tree-ssa/ssa-ifcombine-8.c: New testcase.
 	* gcc.dg/tree-ssa/ssa-ifcombine-9.c: New testcase.

Comments

Richard Biener June 20, 2012, 10:05 a.m. UTC | #1 
On Sun, Jun 10, 2012 at 4:16 PM, Marc Glisse <marc.glisse@inria.fr> 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.

Is there any reason that would not work?

At least

+  if (inv & 1)
+    lcompcode2 = COMPCODE_TRUE - lcompcode2;

looks as if it were not semantically correct - you cannot simply invert
floating-point comparisons (see the restrictions invert_tree_comparison
has).

Thanks,
Richard.

>
> 2012-06-10  Marc Glisse  <marc.glisse@inria.fr>
>
> gcc/
>        PR tree-optimization/51938
>        * fold-const.c (combine_comparisons): Extra argument. Handle inverted
>        conditions.
>        (fold_truth_andor_1): Update call to combine_comparisons.
>        * gimple-fold.c (swap12): New function.
>        (and_comparisons_1): Extra argument. Handle inverted conditions.
>        (and_var_with_comparison_1): Update call to and_comparisons_1.
>        (maybe_fold_and_comparisons): Extra argument. Update call to
>        and_comparisons_1.
>        (or_comparisons_1): Extra argument. Handle inverted conditions.
>        (or_var_with_comparison_1): Update call to or_comparisons_1.
>        (maybe_fold_or_comparisons): Extra argument. Update call to
>        or_comparisons_1.
>        * tree-ssa-ifcombine.c (ifcombine_ifnotandif): New function.
>        (ifcombine_ifnotorif): New function.
>        (tree_ssa_ifcombine_bb): Call them.
>        (ifcombine_iforif): Update call to maybe_fold_or_comparisons.
>        (ifcombine_ifandif): Update call to maybe_fold_and_comparisons.
>        * tree-ssa-reassoc.c (eliminate_redundant_comparison): Update calls
> to
>        maybe_fold_or_comparisons and maybe_fold_and_comparisons.
>        * tree-if-conv.c (fold_or_predicates): Update call to
>        maybe_fold_or_comparisons.
>        * gimple.h (maybe_fold_and_comparisons): Match gimple-fold.c
> prototype.
>        (maybe_fold_or_comparisons): Likewise.
>        * tree.h (combine_comparisons): Match fold-const.c prototype.
>
> gcc/testsuite/
>        PR tree-optimization/51938
>        * gcc.dg/tree-ssa/ssa-ifcombine-8.c: New testcase.
>        * gcc.dg/tree-ssa/ssa-ifcombine-9.c: New testcase.
>
>
>
> --
> Marc Glisse
Marc Glisse June 20, 2012, 7 p.m. UTC | #2 
On Wed, 20 Jun 2012, Richard Guenther wrote:

> On Sun, Jun 10, 2012 at 4:16 PM, Marc Glisse <marc.glisse@inria.fr> 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.
>
> Is there any reason that would not work?

invert_tree_comparison is useless for floating point (the case I am most 
interested in) unless we specify -fno-trapping-math (writing this patch 
taught me to add this flag to my default flags, but I can't expect 
everyone to do the same). An issue is that gcc mixes the behaviors of qnan 
and snan (it is not really an issue, it just means that !(comparison) 
can't be represented as comparison2).

> At least
>
> +  if (inv & 1)
> +    lcompcode2 = COMPCODE_TRUE - lcompcode2;
>
> looks as if it were not semantically correct - you cannot simply invert
> floating-point comparisons (see the restrictions invert_tree_comparison
> has).

I don't remember all details, but I specifically thought of that, and the 
trapping behavior is handled a few lines below.
Marc Glisse June 23, 2012, 5:18 p.m. UTC | #3 
Hello,

thanks for looking into the patch. A couple more details now that I am 
back from a conference:

On Wed, 20 Jun 2012, Marc Glisse wrote:

> On Wed, 20 Jun 2012, Richard Guenther wrote:
>
>> On Sun, Jun 10, 2012 at 4:16 PM, Marc Glisse <marc.glisse@inria.fr> 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).

The idea of also supporting inv==1 or inv==3 is for uniformity, and 
because we might at some point want to get rid of the 'or' function and 
implement everything in terms of the 'and' version, with suitably inverted 
tests.

>> 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.

I would rather have done that as well, and as a matter of fact that is 
what the first version of the patch did, until I realized that 
-ftrapping-math was the default.

>> Is there any reason that would not work?
>
> invert_tree_comparison is useless for floating point (the case I am most 
> interested in) unless we specify -fno-trapping-math (writing this patch 
> taught me to add this flag to my default flags, but I can't expect everyone 
> to do the same). An issue is that gcc mixes the behaviors of qnan and snan 
> (it is not really an issue, it just means that !(comparison) can't be 
> represented as comparison2).
>
>> At least
>> 
>> +  if (inv & 1)
>> +    lcompcode2 = COMPCODE_TRUE - lcompcode2;
>> 
>> looks as if it were not semantically correct - you cannot simply invert
>> floating-point comparisons (see the restrictions invert_tree_comparison
>> has).
>
> I don't remember all details, but I specifically thought of that, and the 
> trapping behavior is handled a few lines below.

More specifically, it has (was already there, I didn't add it):
    if (!honor_nans)
...
    else if (flag_trapping_math)
      {
         /* Check that the original operation and the optimized ones will trap
            under the same condition.  */
         bool ltrap = (lcompcode & COMPCODE_UNORD) == 0
                      && (lcompcode != COMPCODE_EQ)
                      && (lcompcode != COMPCODE_ORD);
... same for rtrap and trap

         /* In a short-circuited boolean expression the LHS might be
            such that the RHS, if evaluated, will never trap.  For
            example, in ORD (x, y) && (x < y), we evaluate the RHS only
            if neither x nor y is NaN.  (This is a mixed blessing: for
            example, the expression above will never trap, hence
            optimizing it to x < y would be invalid).  */
         if ((code == TRUTH_ORIF_EXPR && (lcompcode & COMPCODE_UNORD))
             || (code == TRUTH_ANDIF_EXPR && !(lcompcode & COMPCODE_UNORD)))
           rtrap = false;

         /* If the comparison was short-circuited, and only the RHS
            trapped, we may now generate a spurious trap.  */
         if (rtrap && !ltrap
             && (code == TRUTH_ANDIF_EXPR || code == TRUTH_ORIF_EXPR))
           return NULL_TREE;

         /* If we changed the conditions that cause a trap, we lose.  */
         if ((ltrap || rtrap) != trap)
...

which presumably ensures that the trapping behavior is preserved (I'll 
have to double-check that I didn't break that logic).

Do you have an idea how this can be handled in a less intrusive way (and 
without duplicating too much code)?
Marc Glisse June 23, 2012, 7:17 p.m. UTC | #4 
On Wed, 20 Jun 2012, Marc Glisse wrote:

> On Wed, 20 Jun 2012, Richard Guenther wrote:
>
>> On Sun, Jun 10, 2012 at 4:16 PM, Marc Glisse <marc.glisse@inria.fr> 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.
>> 
>> Is there any reason that would not work?
>
> invert_tree_comparison is useless for floating point (the case I am most 
> interested in) unless we specify -fno-trapping-math (writing this patch 
> taught me to add this flag to my default flags, but I can't expect everyone 
> to do the same). An issue is that gcc mixes the behaviors of qnan and snan 
> (it is not really an issue, it just means that !(comparison) can't be 
> represented as comparison2).

Actually, what would you think of 
s/flag_trapping_math/flag_signaling_nans/
in invert_tree_comparison? Are there other possible ways a<b can trap than 
having a sNaN for a or b?
Joseph Myers June 24, 2012, 3:10 p.m. UTC | #5 
On Sat, 23 Jun 2012, Marc Glisse wrote:

> Actually, what would you think of s/flag_trapping_math/flag_signaling_nans/
> in invert_tree_comparison? Are there other possible ways a<b can trap than
> having a sNaN for a or b?

Ordered comparisons raise exceptions for both quiet and signaling NaNs.  
It's generally operations with floating-point results that raise 
exceptions only for signaling NaN operands.
Marc Glisse June 24, 2012, 3:19 p.m. UTC | #6 
On Sun, 24 Jun 2012, Joseph S. Myers wrote:

> On Sat, 23 Jun 2012, Marc Glisse wrote:
>
>> Actually, what would you think of s/flag_trapping_math/flag_signaling_nans/
>> in invert_tree_comparison? Are there other possible ways a<b can trap than
>> having a sNaN for a or b?
>
> Ordered comparisons raise exceptions for both quiet and signaling NaNs.
> It's generally operations with floating-point results that raise
> exceptions only for signaling NaN operands.

Thank you for this explanation, it makes more sense now. And it seems to 
mean that writing a non-intrusive patch won't help.
Richard Biener June 26, 2012, 1:32 p.m. UTC | #7 
On Sat, Jun 23, 2012 at 7:18 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> Hello,
>
> thanks for looking into the patch. A couple more details now that I am back
> from a conference:
>
>
> On Wed, 20 Jun 2012, Marc Glisse wrote:
>
>> On Wed, 20 Jun 2012, Richard Guenther wrote:
>>
>>> On Sun, Jun 10, 2012 at 4:16 PM, Marc Glisse <marc.glisse@inria.fr>
>>> 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).
>
>
> The idea of also supporting inv==1 or inv==3 is for uniformity, and because
> we might at some point want to get rid of the 'or' function and implement
> everything in terms of the 'and' version, with suitably inverted tests.
>
>
>>> 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.
>
>
> I would rather have done that as well, and as a matter of fact that is what
> the first version of the patch did, until I realized that -ftrapping-math
> was the default.
>
>
>>> Is there any reason that would not work?
>>
>>
>> invert_tree_comparison is useless for floating point (the case I am most
>> interested in) unless we specify -fno-trapping-math (writing this patch
>> taught me to add this flag to my default flags, but I can't expect everyone
>> to do the same). An issue is that gcc mixes the behaviors of qnan and snan
>> (it is not really an issue, it just means that !(comparison) can't be
>> represented as comparison2).
>>
>>> At least
>>>
>>> +  if (inv & 1)
>>> +    lcompcode2 = COMPCODE_TRUE - lcompcode2;
>>>
>>> looks as if it were not semantically correct - you cannot simply invert
>>> floating-point comparisons (see the restrictions invert_tree_comparison
>>> has).
>>
>>
>> I don't remember all details, but I specifically thought of that, and the
>> trapping behavior is handled a few lines below.
>
>
> More specifically, it has (was already there, I didn't add it):
>   if (!honor_nans)
> ...
>   else if (flag_trapping_math)
>     {
>        /* Check that the original operation and the optimized ones will trap
>           under the same condition.  */
>        bool ltrap = (lcompcode & COMPCODE_UNORD) == 0
>                     && (lcompcode != COMPCODE_EQ)
>                     && (lcompcode != COMPCODE_ORD);
> ... same for rtrap and trap
>
>        /* In a short-circuited boolean expression the LHS might be
>           such that the RHS, if evaluated, will never trap.  For
>           example, in ORD (x, y) && (x < y), we evaluate the RHS only
>           if neither x nor y is NaN.  (This is a mixed blessing: for
>           example, the expression above will never trap, hence
>           optimizing it to x < y would be invalid).  */
>        if ((code == TRUTH_ORIF_EXPR && (lcompcode & COMPCODE_UNORD))
>            || (code == TRUTH_ANDIF_EXPR && !(lcompcode & COMPCODE_UNORD)))
>          rtrap = false;
>
>        /* If the comparison was short-circuited, and only the RHS
>           trapped, we may now generate a spurious trap.  */
>        if (rtrap && !ltrap
>            && (code == TRUTH_ANDIF_EXPR || code == TRUTH_ORIF_EXPR))
>          return NULL_TREE;
>
>        /* If we changed the conditions that cause a trap, we lose.  */
>        if ((ltrap || rtrap) != trap)
> ...
>
> which presumably ensures that the trapping behavior is preserved (I'll have
> to double-check that I didn't break that logic).

I was more concerned about the behavior with NaNs in general where
x < y is not equal to x >= y.  Now combine_comparison handles only
the very specific case of logical and or or of two comparisons with the
same operands, you basically make it handle && ~ and || ~, too
(and ~ && ~ and ~ || ~), so it seems that optionally inverting the result
of the 2nd operand should be enough making the interface prettier
compared to the bitmask.

With the following change this should be installed separately - it's a
functional change already now

	/* If we changed the conditions that cause a trap, we lose.  */
 	if ((ltrap || rtrap) != trap)
+	  {
+	    /* Try the inverse condition.  */
+	    compcode = COMPCODE_TRUE - compcode;
+	    exchg = true;
+	    trap = (compcode & COMPCODE_UNORD) == 0
+		   && (compcode != COMPCODE_EQ)
+		   && (compcode != COMPCODE_ORD);
+	  }
+	if ((ltrap || rtrap) != trap)
 	  return NULL_TREE;
       }
...
+      ret = fold_build2_loc (loc, tcode, truth_type, ll_arg, lr_arg);
+      if (exchg)
+	ret = fold_build1_loc (loc, TRUTH_NOT_EXPR, truth_type, ret);

> Do you have an idea how this can be handled in a less intrusive way (and
> without duplicating too much code)?

Well, for one add an argument to ifcombine_iforif/ifandif whether the 2nd op
is inverted and handle that appropriately instead of copying the functions.

I'd go for a single bool argument for the inversion everywhere, too, both and
and or are commutative and you can handle all cases with that form.

Thanks,
Richard.

> --
> Marc Glisse
Marc Glisse June 26, 2012, 1:48 p.m. UTC | #8 
Thanks for the comments, I'll look into it later in the summer.

On Tue, 26 Jun 2012, Richard Guenther wrote:

> On Sat, Jun 23, 2012 at 7:18 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>> Hello,
>>
>> thanks for looking into the patch. A couple more details now that I am back
>> from a conference:
>>
>>
>> On Wed, 20 Jun 2012, Marc Glisse wrote:
>>
>>> On Wed, 20 Jun 2012, Richard Guenther wrote:
>>>
>>>> On Sun, Jun 10, 2012 at 4:16 PM, Marc Glisse <marc.glisse@inria.fr>
>>>> 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).
>>
>>
>> The idea of also supporting inv==1 or inv==3 is for uniformity, and because
>> we might at some point want to get rid of the 'or' function and implement
>> everything in terms of the 'and' version, with suitably inverted tests.
>>
>>
>>>> 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.
>>
>>
>> I would rather have done that as well, and as a matter of fact that is what
>> the first version of the patch did, until I realized that -ftrapping-math
>> was the default.
>>
>>
>>>> Is there any reason that would not work?
>>>
>>>
>>> invert_tree_comparison is useless for floating point (the case I am most
>>> interested in) unless we specify -fno-trapping-math (writing this patch
>>> taught me to add this flag to my default flags, but I can't expect everyone
>>> to do the same). An issue is that gcc mixes the behaviors of qnan and snan
>>> (it is not really an issue, it just means that !(comparison) can't be
>>> represented as comparison2).
>>>
>>>> At least
>>>>
>>>> +  if (inv & 1)
>>>> +    lcompcode2 = COMPCODE_TRUE - lcompcode2;
>>>>
>>>> looks as if it were not semantically correct - you cannot simply invert
>>>> floating-point comparisons (see the restrictions invert_tree_comparison
>>>> has).
>>>
>>>
>>> I don't remember all details, but I specifically thought of that, and the
>>> trapping behavior is handled a few lines below.
>>
>>
>> More specifically, it has (was already there, I didn't add it):
>>   if (!honor_nans)
>> ...
>>   else if (flag_trapping_math)
>>     {
>>        /* Check that the original operation and the optimized ones will trap
>>           under the same condition.  */
>>        bool ltrap = (lcompcode & COMPCODE_UNORD) == 0
>>                     && (lcompcode != COMPCODE_EQ)
>>                     && (lcompcode != COMPCODE_ORD);
>> ... same for rtrap and trap
>>
>>        /* In a short-circuited boolean expression the LHS might be
>>           such that the RHS, if evaluated, will never trap.  For
>>           example, in ORD (x, y) && (x < y), we evaluate the RHS only
>>           if neither x nor y is NaN.  (This is a mixed blessing: for
>>           example, the expression above will never trap, hence
>>           optimizing it to x < y would be invalid).  */
>>        if ((code == TRUTH_ORIF_EXPR && (lcompcode & COMPCODE_UNORD))
>>            || (code == TRUTH_ANDIF_EXPR && !(lcompcode & COMPCODE_UNORD)))
>>          rtrap = false;
>>
>>        /* If the comparison was short-circuited, and only the RHS
>>           trapped, we may now generate a spurious trap.  */
>>        if (rtrap && !ltrap
>>            && (code == TRUTH_ANDIF_EXPR || code == TRUTH_ORIF_EXPR))
>>          return NULL_TREE;
>>
>>        /* If we changed the conditions that cause a trap, we lose.  */
>>        if ((ltrap || rtrap) != trap)
>> ...
>>
>> which presumably ensures that the trapping behavior is preserved (I'll have
>> to double-check that I didn't break that logic).
>
> I was more concerned about the behavior with NaNs in general where
> x < y is not equal to x >= y.  Now combine_comparison handles only
> the very specific case of logical and or or of two comparisons with the
> same operands, you basically make it handle && ~ and || ~, too
> (and ~ && ~ and ~ || ~), so it seems that optionally inverting the result
> of the 2nd operand should be enough making the interface prettier
> compared to the bitmask.
>
> With the following change this should be installed separately - it's a
> functional change already now
>
> 	/* If we changed the conditions that cause a trap, we lose.  */
> 	if ((ltrap || rtrap) != trap)
> +	  {
> +	    /* Try the inverse condition.  */
> +	    compcode = COMPCODE_TRUE - compcode;
> +	    exchg = true;
> +	    trap = (compcode & COMPCODE_UNORD) == 0
> +		   && (compcode != COMPCODE_EQ)
> +		   && (compcode != COMPCODE_ORD);
> +	  }
> +	if ((ltrap || rtrap) != trap)
> 	  return NULL_TREE;
>       }
> ...
> +      ret = fold_build2_loc (loc, tcode, truth_type, ll_arg, lr_arg);
> +      if (exchg)
> +	ret = fold_build1_loc (loc, TRUTH_NOT_EXPR, truth_type, ret);
>
>> Do you have an idea how this can be handled in a less intrusive way (and
>> without duplicating too much code)?
>
> Well, for one add an argument to ifcombine_iforif/ifandif whether the 2nd op
> is inverted and handle that appropriately instead of copying the functions.
>
> I'd go for a single bool argument for the inversion everywhere, too, both and
> and or are commutative and you can handle all cases with that form.
>
> Thanks,
> Richard.
>
>> --
>> Marc Glisse
>
diff mbox

Patch

Index: fold-const.c
===================================================================
--- fold-const.c	(revision 188331)
+++ fold-const.c	(working copy)
@@ -2247,35 +2247,46 @@  compcode_to_comparison (enum comparison_
       gcc_unreachable ();
     }
 }
 
 /* Return a tree for the comparison which is the combination of
    doing the AND or OR (depending on CODE) of the two operations LCODE
-   and RCODE on the identical operands LL_ARG and LR_ARG.  Take into account
-   the possibility of trapping if the mode has NaNs, and return NULL_TREE
+   and RCODE on the identical operands LL_ARG and LR_ARG.  The bit 0
+   (resp. 1) of inv indicates, when set, that the result of the
+   operation LCODE needs to be inverted.   Take into account the
+   possibility of trapping if the mode has NaNs, and return NULL_TREE
    if this makes the transformation invalid.  */
 
 tree
 combine_comparisons (location_t loc,
 		     enum tree_code code, enum tree_code lcode,
 		     enum tree_code rcode, tree truth_type,
-		     tree ll_arg, tree lr_arg)
+		     tree ll_arg, tree lr_arg, int inv)
 {
   bool honor_nans = HONOR_NANS (TYPE_MODE (TREE_TYPE (ll_arg)));
   enum comparison_code lcompcode = comparison_to_compcode (lcode);
   enum comparison_code rcompcode = comparison_to_compcode (rcode);
+  int lcompcode2 = lcompcode;
+  int rcompcode2 = rcompcode;
   int compcode;
+  bool exchg = false;
+  tree ret;
+
+  if (inv & 1)
+    lcompcode2 = COMPCODE_TRUE - lcompcode2;
+  if (inv & 2)
+    rcompcode2 = COMPCODE_TRUE - rcompcode2;
 
   switch (code)
     {
     case TRUTH_AND_EXPR: case TRUTH_ANDIF_EXPR:
-      compcode = lcompcode & rcompcode;
+      compcode = lcompcode2 & rcompcode2;
       break;
 
     case TRUTH_OR_EXPR: case TRUTH_ORIF_EXPR:
-      compcode = lcompcode | rcompcode;
+      compcode = lcompcode2 | rcompcode2;
       break;
 
     default:
       return NULL_TREE;
     }
 
@@ -2318,25 +2329,38 @@  combine_comparisons (location_t loc,
 	if (rtrap && !ltrap
 	    && (code == TRUTH_ANDIF_EXPR || code == TRUTH_ORIF_EXPR))
 	  return NULL_TREE;
 
 	/* If we changed the conditions that cause a trap, we lose.  */
 	if ((ltrap || rtrap) != trap)
+	  {
+	    /* Try the inverse condition.  */
+	    compcode = COMPCODE_TRUE - compcode;
+	    exchg = true;
+	    trap = (compcode & COMPCODE_UNORD) == 0
+		   && (compcode != COMPCODE_EQ)
+		   && (compcode != COMPCODE_ORD);
+	  }
+	if ((ltrap || rtrap) != trap)
 	  return NULL_TREE;
       }
 
+  /* The inversion can't lead to a constant.  */
   if (compcode == COMPCODE_TRUE)
     return constant_boolean_node (true, truth_type);
   else if (compcode == COMPCODE_FALSE)
     return constant_boolean_node (false, truth_type);
   else
     {
       enum tree_code tcode;
 
       tcode = compcode_to_comparison ((enum comparison_code) compcode);
-      return fold_build2_loc (loc, tcode, truth_type, ll_arg, lr_arg);
+      ret = fold_build2_loc (loc, tcode, truth_type, ll_arg, lr_arg);
+      if (exchg)
+	ret = fold_build1_loc (loc, TRUTH_NOT_EXPR, truth_type, ret);
+      return ret;
     }
 }
 
 /* Return nonzero if two operands (typically of the same tree node)
    are necessarily equal.  If either argument has side-effects this
    function returns zero.  FLAGS modifies behavior as follows:
@@ -5115,22 +5139,22 @@  fold_truth_andor_1 (location_t loc, enum
       && simple_operand_p (lr_arg))
     {
       if (operand_equal_p (ll_arg, rl_arg, 0)
           && operand_equal_p (lr_arg, rr_arg, 0))
 	{
           result = combine_comparisons (loc, code, lcode, rcode,
-					truth_type, ll_arg, lr_arg);
+					truth_type, ll_arg, lr_arg, 0);
 	  if (result)
 	    return result;
 	}
       else if (operand_equal_p (ll_arg, rr_arg, 0)
                && operand_equal_p (lr_arg, rl_arg, 0))
 	{
           result = combine_comparisons (loc, code, lcode,
 					swap_tree_comparison (rcode),
-					truth_type, ll_arg, lr_arg);
+					truth_type, ll_arg, lr_arg, 0);
 	  if (result)
 	    return result;
 	}
     }
 
   code = ((code == TRUTH_AND_EXPR || code == TRUTH_ANDIF_EXPR)
Index: gimple.h
===================================================================
--- gimple.h	(revision 188331)
+++ gimple.h	(working copy)
@@ -5309,12 +5309,12 @@  void gimplify_and_update_call_from_tree
 tree gimple_fold_builtin (gimple);
 bool fold_stmt (gimple_stmt_iterator *);
 bool fold_stmt_inplace (gimple_stmt_iterator *);
 tree get_symbol_constant_value (tree);
 tree canonicalize_constructor_val (tree, tree);
 extern tree maybe_fold_and_comparisons (enum tree_code, tree, tree, 
-					enum tree_code, tree, tree);
+					enum tree_code, tree, tree, int);
 extern tree maybe_fold_or_comparisons (enum tree_code, tree, tree,
-				       enum tree_code, tree, tree);
+				       enum tree_code, tree, tree, int);
 
 bool gimple_val_nonnegative_real_p (tree);
 #endif  /* GCC_GIMPLE_H */
Index: tree.h
===================================================================
--- tree.h	(revision 188331)
+++ tree.h	(working copy)
@@ -5366,13 +5366,13 @@  extern bool tree_invalid_nonnegative_war
 extern bool tree_call_nonnegative_warnv_p (tree, tree, tree, tree, bool *);
 
 extern bool tree_expr_nonzero_warnv_p (tree, bool *);
 
 extern bool fold_real_zero_addition_p (const_tree, const_tree, int);
 extern tree combine_comparisons (location_t, enum tree_code, enum tree_code,
-				 enum tree_code, tree, tree, tree);
+				 enum tree_code, tree, tree, tree, int);
 extern void debug_fold_checksum (const_tree);
 
 /* Return nonzero if CODE is a tree code that represents a truth value.  */
 static inline bool
 truth_value_p (enum tree_code code)
 {
Index: tree-ssa-ifcombine.c
===================================================================
--- tree-ssa-ifcombine.c	(revision 188331)
+++ tree-ssa-ifcombine.c	(working copy)
@@ -372,13 +372,13 @@  ifcombine_ifandif (basic_block inner_con
 
       if (!(t = maybe_fold_and_comparisons (gimple_cond_code (inner_cond),
 					    gimple_cond_lhs (inner_cond),
 					    gimple_cond_rhs (inner_cond),
 					    gimple_cond_code (outer_cond),
 					    gimple_cond_lhs (outer_cond),
-					    gimple_cond_rhs (outer_cond))))
+					    gimple_cond_rhs (outer_cond), 0)))
 	return false;
       t = canonicalize_cond_expr_cond (t);
       if (!t)
 	return false;
       gimple_cond_set_condition_from_tree (inner_cond, t);
       update_stmt (inner_cond);
@@ -520,13 +520,13 @@  ifcombine_iforif (basic_block inner_cond
 
       if (!(t = maybe_fold_or_comparisons (gimple_cond_code (inner_cond),
 					   gimple_cond_lhs (inner_cond),
 					   gimple_cond_rhs (inner_cond),
 					   gimple_cond_code (outer_cond),
 					   gimple_cond_lhs (outer_cond),
-					   gimple_cond_rhs (outer_cond))))
+					   gimple_cond_rhs (outer_cond), 0)))
 	return false;
       t = canonicalize_cond_expr_cond (t);
       if (!t)
 	return false;
       gimple_cond_set_condition_from_tree (inner_cond, t);
       update_stmt (inner_cond);
@@ -545,12 +545,156 @@  ifcombine_iforif (basic_block inner_cond
       return true;
     }
 
   return false;
 }
 
+/* If-convert on a !a||b pattern.  The inner if is specified by its
+   INNER_COND_BB, the outer by OUTER_COND_BB.  Returns true if the
+   edges to the common outer else basic-block / inner then basic-block
+   were merged.  */
+
+static bool
+ifcombine_ifnotorif (basic_block inner_cond_bb, basic_block outer_cond_bb)
+{
+  gimple inner_cond, outer_cond;
+
+  inner_cond = last_stmt (inner_cond_bb);
+  if (!inner_cond
+      || gimple_code (inner_cond) != GIMPLE_COND)
+    return false;
+
+  outer_cond = last_stmt (outer_cond_bb);
+  if (!outer_cond
+      || gimple_code (outer_cond) != GIMPLE_COND)
+    return false;
+
+  /* See if we have two comparisons that we can merge into one.  */
+  if (TREE_CODE_CLASS (gimple_cond_code (inner_cond)) == tcc_comparison
+	   && TREE_CODE_CLASS (gimple_cond_code (outer_cond)) == tcc_comparison)
+    {
+      tree t;
+      bool inv = false;
+
+      if (!(t = maybe_fold_or_comparisons (gimple_cond_code (inner_cond),
+					   gimple_cond_lhs (inner_cond),
+					   gimple_cond_rhs (inner_cond),
+					   gimple_cond_code (outer_cond),
+					   gimple_cond_lhs (outer_cond),
+					   gimple_cond_rhs (outer_cond), 2)))
+	return false;
+
+      if (TREE_CODE (t) == TRUTH_NOT_EXPR)
+	{
+	  inv = true;
+	  t = TREE_OPERAND (t, 0);
+	}
+
+      t = canonicalize_cond_expr_cond (t);
+      if (!t)
+	return false;
+
+      if (inv)
+	{
+	  edge t = EDGE_SUCC (inner_cond_bb, 0);
+	  edge e = EDGE_SUCC (inner_cond_bb, 1);
+	  t->flags ^= (EDGE_TRUE_VALUE | EDGE_FALSE_VALUE);
+	  e->flags ^= (EDGE_TRUE_VALUE | EDGE_FALSE_VALUE);
+	}
+      gimple_cond_set_condition_from_tree (inner_cond, t);
+      update_stmt (inner_cond);
+
+      /* Leave CFG optimization to cfg_cleanup.  */
+      gimple_cond_set_condition_from_tree (outer_cond, boolean_true_node);
+      update_stmt (outer_cond);
+
+      if (dump_file)
+	{
+	  fprintf (dump_file, "optimizing two comparisons to ");
+	  print_generic_expr (dump_file, t, 0);
+	  fprintf (dump_file, "\n");
+	}
+
+      return true;
+    }
+
+  return false;
+}
+
+/* If-convert on a !a&&b pattern.  The inner if is specified by its
+   INNER_COND_BB, the outer by OUTER_COND_BB.  Returns true if the
+   edges to the common outer then basic-block / inner else basic-block
+   were merged.  */
+
+static bool
+ifcombine_ifnotandif (basic_block inner_cond_bb, basic_block outer_cond_bb)
+{
+  gimple inner_cond, outer_cond;
+
+  inner_cond = last_stmt (inner_cond_bb);
+  if (!inner_cond
+      || gimple_code (inner_cond) != GIMPLE_COND)
+    return false;
+
+  outer_cond = last_stmt (outer_cond_bb);
+  if (!outer_cond
+      || gimple_code (outer_cond) != GIMPLE_COND)
+    return false;
+
+  /* See if we have two comparisons that we can merge into one.  */
+  if (TREE_CODE_CLASS (gimple_cond_code (inner_cond)) == tcc_comparison
+	   && TREE_CODE_CLASS (gimple_cond_code (outer_cond)) == tcc_comparison)
+    {
+      tree t;
+      bool inv = false;
+
+      if (!(t = maybe_fold_and_comparisons (gimple_cond_code (inner_cond),
+					    gimple_cond_lhs (inner_cond),
+					    gimple_cond_rhs (inner_cond),
+					    gimple_cond_code (outer_cond),
+					    gimple_cond_lhs (outer_cond),
+					    gimple_cond_rhs (outer_cond), 2)))
+	return false;
+
+      if (TREE_CODE (t) == TRUTH_NOT_EXPR)
+	{
+	  inv = true;
+	  t = TREE_OPERAND (t, 0);
+	}
+
+      t = canonicalize_cond_expr_cond (t);
+      if (!t)
+	return false;
+
+      if (inv)
+	{
+	  edge t = EDGE_SUCC (inner_cond_bb, 0);
+	  edge e = EDGE_SUCC (inner_cond_bb, 1);
+	  t->flags ^= (EDGE_TRUE_VALUE | EDGE_FALSE_VALUE);
+	  e->flags ^= (EDGE_TRUE_VALUE | EDGE_FALSE_VALUE);
+	}
+      gimple_cond_set_condition_from_tree (inner_cond, t);
+      update_stmt (inner_cond);
+
+      /* Leave CFG optimization to cfg_cleanup.  */
+      gimple_cond_set_condition_from_tree (outer_cond, boolean_false_node);
+      update_stmt (outer_cond);
+
+      if (dump_file)
+	{
+	  fprintf (dump_file, "optimizing two comparisons to ");
+	  print_generic_expr (dump_file, t, 0);
+	  fprintf (dump_file, "\n");
+	}
+
+      return true;
+    }
+
+  return false;
+}
+
 /* Recognize a CFG pattern and dispatch to the appropriate
    if-conversion helper.  We start with BB as the innermost
    worker basic-block.  Returns true if a transformation was done.  */
 
 static bool
 tree_ssa_ifcombine_bb (basic_block inner_cond_bb)
@@ -607,12 +751,52 @@  tree_ssa_ifcombine_bb (basic_block inner
 		 if (q) goto then_bb; else goto ...;
 	       <then_bb>
 		 ...
 	   */
 	  return ifcombine_iforif (inner_cond_bb, outer_cond_bb);
 	}
+
+      /* The !|| form is characterized by an outer else_bb that
+	 matches the inner else_bb, with the two edges leading to it
+	 mergable.  The latter is guaranteed by matching PHI arguments
+	 in the else_bb and the inner cond_bb having no side-effects.  */
+      if (recognize_if_then_else (outer_cond_bb, &inner_cond_bb, &then_bb)
+	  && same_phi_args_p (outer_cond_bb, inner_cond_bb, then_bb)
+	  && bb_no_side_effects_p (inner_cond_bb))
+	{
+	  /* We have
+	       <outer_cond_bb>
+		 if (q) goto inner_cond_bb; else goto then_bb;
+	       <inner_cond_bb>
+		 if (p) goto then_bb; else goto ...;
+		 ...
+	       <then_bb>
+		 ...
+	   */
+	  return ifcombine_ifnotorif (inner_cond_bb, outer_cond_bb);
+	}
+
+      /* The !&& form is characterized by an outer then_bb that
+	 matches the inner else_bb, with the two edges leading to it
+	 mergable.  The latter is guaranteed by matching PHI arguments
+	 in the else_bb and the inner cond_bb having no side-effects.  */
+      if (recognize_if_then_else (outer_cond_bb, &else_bb, &inner_cond_bb)
+	  && same_phi_args_p (outer_cond_bb, inner_cond_bb, else_bb)
+	  && bb_no_side_effects_p (inner_cond_bb))
+	{
+	  /* We have
+	       <outer_cond_bb>
+		 if (q) goto else_bb; else goto inner_cond_bb;
+	       <inner_cond_bb>
+		 if (p) goto ...; else goto else_bb;
+		 ...
+	       <else_bb>
+		 ...
+	   */
+	  return ifcombine_ifnotandif (inner_cond_bb, outer_cond_bb);
+	}
     }
 
   return false;
 }
 
 /* Main entry for the tree if-conversion pass.  */
Index: tree-ssa-reassoc.c
===================================================================
--- tree-ssa-reassoc.c	(revision 188331)
+++ tree-ssa-reassoc.c	(working copy)
@@ -1539,17 +1539,17 @@  eliminate_redundant_comparison (enum tre
 
       /* If we got here, we have a match.  See if we can combine the
 	 two comparisons.  */
       if (opcode == BIT_IOR_EXPR)
 	t = maybe_fold_or_comparisons (lcode, op1, op2,
 				       rcode, gimple_assign_rhs1 (def2),
-				       gimple_assign_rhs2 (def2));
+				       gimple_assign_rhs2 (def2), 0);
       else
 	t = maybe_fold_and_comparisons (lcode, op1, op2,
 					rcode, gimple_assign_rhs1 (def2),
-					gimple_assign_rhs2 (def2));
+					gimple_assign_rhs2 (def2), 0);
       if (!t)
 	continue;
 
       /* maybe_fold_and_comparisons and maybe_fold_or_comparisons
 	 always give us a boolean_type_node value back.  If the original
 	 BIT_AND_EXPR or BIT_IOR_EXPR was of a wider integer type,
Index: gimple-fold.c
===================================================================
--- gimple-fold.c	(revision 188331)
+++ gimple-fold.c	(working copy)
@@ -1470,29 +1470,34 @@  same_bool_result_p (const_tree op1, cons
 }
 
 /* Forward declarations for some mutually recursive functions.  */
 
 static tree
 and_comparisons_1 (enum tree_code code1, tree op1a, tree op1b,
-		   enum tree_code code2, tree op2a, tree op2b);
+		   enum tree_code code2, tree op2a, tree op2b, int inv);
 static tree
 and_var_with_comparison (tree var, bool invert,
 			 enum tree_code code2, tree op2a, tree op2b);
 static tree
 and_var_with_comparison_1 (gimple stmt, 
 			   enum tree_code code2, tree op2a, tree op2b);
 static tree
 or_comparisons_1 (enum tree_code code1, tree op1a, tree op1b,
-		  enum tree_code code2, tree op2a, tree op2b);
+		  enum tree_code code2, tree op2a, tree op2b, int inv);
 static tree
 or_var_with_comparison (tree var, bool invert,
 			enum tree_code code2, tree op2a, tree op2b);
 static tree
 or_var_with_comparison_1 (gimple stmt, 
 			  enum tree_code code2, tree op2a, tree op2b);
 
+static int swap12 (int flags)
+{
+  return ((flags & 1) << 1) | (flags >> 1);
+}
+
 /* Helper function for and_comparisons_1:  try to simplify the AND of the
    ssa variable VAR with the comparison specified by (OP2A CODE2 OP2B).
    If INVERT is true, invert the value of the VAR before doing the AND.
    Return NULL_EXPR if we can't simplify this to a single expression.  */
 
 static tree
@@ -1556,13 +1561,14 @@  and_var_with_comparison_1 (gimple stmt,
     {
       tree t = and_comparisons_1 (innercode,
 				  gimple_assign_rhs1 (stmt),
 				  gimple_assign_rhs2 (stmt),
 				  code2,
 				  op2a,
-				  op2b);
+				  op2b,
+				  0);
       if (t)
 	return t;
     }
 
   /* If the definition is an AND or OR expression, we may be able to
      simplify by reassociating.  */
@@ -1601,13 +1607,13 @@  and_var_with_comparison_1 (gimple stmt,
       if (TREE_CODE (inner1) == SSA_NAME
 	  && is_gimple_assign (s = SSA_NAME_DEF_STMT (inner1))
 	  && TREE_CODE_CLASS (gimple_assign_rhs_code (s)) == tcc_comparison
 	  && (t = maybe_fold_and_comparisons (gimple_assign_rhs_code (s),
 					      gimple_assign_rhs1 (s),
 					      gimple_assign_rhs2 (s),
-					      code2, op2a, op2b)))
+					      code2, op2a, op2b, 0)))
 	{
 	  /* Handle the AND case, where we are reassociating:
 	     (inner1 AND inner2) AND (op2a code2 op2b)
 	     => (t AND inner2)
 	     If the partial result t is a constant, we win.  Otherwise
 	     continue on to try reassociating with the other inner test.  */
@@ -1633,13 +1639,13 @@  and_var_with_comparison_1 (gimple stmt,
       if (TREE_CODE (inner2) == SSA_NAME
 	  && is_gimple_assign (s = SSA_NAME_DEF_STMT (inner2))
 	  && TREE_CODE_CLASS (gimple_assign_rhs_code (s)) == tcc_comparison
 	  && (t = maybe_fold_and_comparisons (gimple_assign_rhs_code (s),
 					      gimple_assign_rhs1 (s),
 					      gimple_assign_rhs2 (s),
-					      code2, op2a, op2b)))
+					      code2, op2a, op2b, 0)))
 	{
 	  /* Handle the AND case, where we are reassociating:
 	     (inner1 AND inner2) AND (op2a code2 op2b)
 	     => (inner1 AND t)  */
 	  if (is_and)
 	    {
@@ -1683,43 +1689,50 @@  and_var_with_comparison_1 (gimple stmt,
 
 /* Try to simplify the AND of two comparisons defined by
    (OP1A CODE1 OP1B) and (OP2A CODE2 OP2B), respectively.
    If this can be done without constructing an intermediate value,
    return the resulting tree; otherwise NULL_TREE is returned.
    This function is deliberately asymmetric as it recurses on SSA_DEFs
-   in the first comparison but not the second.  */
+   in the first comparison but not the second.
+   If inv & 1, the first test is actually !(OP1A CODE1 OP1B).
+   If inv & 2, the second test is actually !(OP2A CODE2 OP2B).  */
 
 static tree
 and_comparisons_1 (enum tree_code code1, tree op1a, tree op1b,
-		   enum tree_code code2, tree op2a, tree op2b)
+		   enum tree_code code2, tree op2a, tree op2b, int inv)
 {
   /* First check for ((x CODE1 y) AND (x CODE2 y)).  */
   if (operand_equal_p (op1a, op2a, 0)
       && operand_equal_p (op1b, op2b, 0))
     {
       /* Result will be either NULL_TREE, or a combined comparison.  */
       tree t = combine_comparisons (UNKNOWN_LOCATION,
 				    TRUTH_ANDIF_EXPR, code1, code2,
-				    boolean_type_node, op1a, op1b);
+				    boolean_type_node, op1a, op1b, inv);
       if (t)
 	return t;
     }
 
   /* Likewise the swapped case of the above.  */
   if (operand_equal_p (op1a, op2b, 0)
       && operand_equal_p (op1b, op2a, 0))
     {
       /* Result will be either NULL_TREE, or a combined comparison.  */
       tree t = combine_comparisons (UNKNOWN_LOCATION,
 				    TRUTH_ANDIF_EXPR, code1,
 				    swap_tree_comparison (code2),
-				    boolean_type_node, op1a, op1b);
+				    boolean_type_node, op1a, op1b, inv);
       if (t)
 	return t;
     }
 
+  if (inv & 1) code1 = invert_tree_comparison (code1,
+			 HONOR_NANS (TYPE_MODE (TREE_TYPE (op1a))));
+  if (inv & 2) code2 = invert_tree_comparison (code2,
+			 HONOR_NANS (TYPE_MODE (TREE_TYPE (op2a))));
+
   /* If both comparisons are of the same value against constants, we might
      be able to merge them.  */
   if (operand_equal_p (op1a, op2a, 0)
       && TREE_CODE (op1b) == INTEGER_CST
       && TREE_CODE (op2b) == INTEGER_CST)
     {
@@ -1935,23 +1948,26 @@  and_comparisons_1 (enum tree_code code1,
 
 /* Try to simplify the AND of two comparisons, specified by
    (OP1A CODE1 OP1B) and (OP2B CODE2 OP2B), respectively.
    If this can be simplified to a single expression (without requiring
    introducing more SSA variables to hold intermediate values),
    return the resulting tree.  Otherwise return NULL_TREE.
-   If the result expression is non-null, it has boolean type.  */
+   If the result expression is non-null, it has boolean type.
+   If inv & 1, the first test is actually !(OP1A CODE1 OP1B).
+   If inv & 2, the second test is actually !(OP2A CODE2 OP2B).  */
 
 tree
 maybe_fold_and_comparisons (enum tree_code code1, tree op1a, tree op1b,
-			    enum tree_code code2, tree op2a, tree op2b)
+			    enum tree_code code2, tree op2a, tree op2b, int inv)
 {
-  tree t = and_comparisons_1 (code1, op1a, op1b, code2, op2a, op2b);
+  tree t = and_comparisons_1 (code1, op1a, op1b, code2, op2a, op2b, inv);
   if (t)
     return t;
   else
-    return and_comparisons_1 (code2, op2a, op2b, code1, op1a, op1b);
+    return and_comparisons_1 (code2, op2a, op2b, code1, op1a, op1b,
+			      swap12 (inv));
 }
 
 /* Helper function for or_comparisons_1:  try to simplify the OR of the
    ssa variable VAR with the comparison specified by (OP2A CODE2 OP2B).
    If INVERT is true, invert the value of VAR before doing the OR.
    Return NULL_EXPR if we can't simplify this to a single expression.  */
@@ -2017,13 +2033,14 @@  or_var_with_comparison_1 (gimple stmt,
     {
       tree t = or_comparisons_1 (innercode,
 				 gimple_assign_rhs1 (stmt),
 				 gimple_assign_rhs2 (stmt),
 				 code2,
 				 op2a,
-				 op2b);
+				 op2b,
+				 0);
       if (t)
 	return t;
     }
   
   /* If the definition is an AND or OR expression, we may be able to
      simplify by reassociating.  */
@@ -2062,13 +2079,13 @@  or_var_with_comparison_1 (gimple stmt,
       if (TREE_CODE (inner1) == SSA_NAME
 	  && is_gimple_assign (s = SSA_NAME_DEF_STMT (inner1))
 	  && TREE_CODE_CLASS (gimple_assign_rhs_code (s)) == tcc_comparison
 	  && (t = maybe_fold_or_comparisons (gimple_assign_rhs_code (s),
 					     gimple_assign_rhs1 (s),
 					     gimple_assign_rhs2 (s),
-					     code2, op2a, op2b)))
+					     code2, op2a, op2b, 0)))
 	{
 	  /* Handle the OR case, where we are reassociating:
 	     (inner1 OR inner2) OR (op2a code2 op2b)
 	     => (t OR inner2)
 	     If the partial result t is a constant, we win.  Otherwise
 	     continue on to try reassociating with the other inner test.  */
@@ -2094,13 +2111,13 @@  or_var_with_comparison_1 (gimple stmt,
       if (TREE_CODE (inner2) == SSA_NAME
 	  && is_gimple_assign (s = SSA_NAME_DEF_STMT (inner2))
 	  && TREE_CODE_CLASS (gimple_assign_rhs_code (s)) == tcc_comparison
 	  && (t = maybe_fold_or_comparisons (gimple_assign_rhs_code (s),
 					     gimple_assign_rhs1 (s),
 					     gimple_assign_rhs2 (s),
-					     code2, op2a, op2b)))
+					     code2, op2a, op2b, 0)))
 	{
 	  /* Handle the OR case, where we are reassociating:
 	     (inner1 OR inner2) OR (op2a code2 op2b)
 	     => (inner1 OR t)
 	     => (t OR partial)  */
 	  if (is_or)
@@ -2145,43 +2162,50 @@  or_var_with_comparison_1 (gimple stmt,
 
 /* Try to simplify the OR of two comparisons defined by
    (OP1A CODE1 OP1B) and (OP2A CODE2 OP2B), respectively.
    If this can be done without constructing an intermediate value,
    return the resulting tree; otherwise NULL_TREE is returned.
    This function is deliberately asymmetric as it recurses on SSA_DEFs
-   in the first comparison but not the second.  */
+   in the first comparison but not the second.
+   If inv & 1, the first test is actually !(OP1A CODE1 OP1B).
+   If inv & 2, the second test is actually !(OP2A CODE2 OP2B).  */
 
 static tree
 or_comparisons_1 (enum tree_code code1, tree op1a, tree op1b,
-		  enum tree_code code2, tree op2a, tree op2b)
+		  enum tree_code code2, tree op2a, tree op2b, int inv)
 {
   /* First check for ((x CODE1 y) OR (x CODE2 y)).  */
   if (operand_equal_p (op1a, op2a, 0)
       && operand_equal_p (op1b, op2b, 0))
     {
       /* Result will be either NULL_TREE, or a combined comparison.  */
       tree t = combine_comparisons (UNKNOWN_LOCATION,
 				    TRUTH_ORIF_EXPR, code1, code2,
-				    boolean_type_node, op1a, op1b);
+				    boolean_type_node, op1a, op1b, inv);
       if (t)
 	return t;
     }
 
   /* Likewise the swapped case of the above.  */
   if (operand_equal_p (op1a, op2b, 0)
       && operand_equal_p (op1b, op2a, 0))
     {
       /* Result will be either NULL_TREE, or a combined comparison.  */
       tree t = combine_comparisons (UNKNOWN_LOCATION,
 				    TRUTH_ORIF_EXPR, code1,
 				    swap_tree_comparison (code2),
-				    boolean_type_node, op1a, op1b);
+				    boolean_type_node, op1a, op1b, inv);
       if (t)
 	return t;
     }
 
+  if (inv & 1) code1 = invert_tree_comparison (code1,
+			 HONOR_NANS (TYPE_MODE (TREE_TYPE (op1a))));
+  if (inv & 2) code2 = invert_tree_comparison (code2,
+			 HONOR_NANS (TYPE_MODE (TREE_TYPE (op2a))));
+
   /* If both comparisons are of the same value against constants, we might
      be able to merge them.  */
   if (operand_equal_p (op1a, op2a, 0)
       && TREE_CODE (op1b) == INTEGER_CST
       && TREE_CODE (op2b) == INTEGER_CST)
     {
@@ -2397,23 +2421,26 @@  or_comparisons_1 (enum tree_code code1,
 
 /* Try to simplify the OR of two comparisons, specified by
    (OP1A CODE1 OP1B) and (OP2B CODE2 OP2B), respectively.
    If this can be simplified to a single expression (without requiring
    introducing more SSA variables to hold intermediate values),
    return the resulting tree.  Otherwise return NULL_TREE.
-   If the result expression is non-null, it has boolean type.  */
+   If the result expression is non-null, it has boolean type.
+   If inv & 1, the first test is actually !(OP1A CODE1 OP1B).
+   If inv & 2, the second test is actually !(OP2A CODE2 OP2B).  */
 
 tree
 maybe_fold_or_comparisons (enum tree_code code1, tree op1a, tree op1b,
-			   enum tree_code code2, tree op2a, tree op2b)
+			   enum tree_code code2, tree op2a, tree op2b, int inv)
 {
-  tree t = or_comparisons_1 (code1, op1a, op1b, code2, op2a, op2b);
+  tree t = or_comparisons_1 (code1, op1a, op1b, code2, op2a, op2b, inv);
   if (t)
     return t;
   else
-    return or_comparisons_1 (code2, op2a, op2b, code1, op1a, op1b);
+    return or_comparisons_1 (code2, op2a, op2b, code1, op1a, op1b,
+			     swap12 (inv));
 }
 
 
 /* Fold STMT to a constant using VALUEIZE to valueize SSA names.
 
    Either NULL_TREE, a simplified but non-constant or a constant
Index: tree-if-conv.c
===================================================================
--- tree-if-conv.c	(revision 188331)
+++ tree-if-conv.c	(working copy)
@@ -315,13 +315,13 @@  fold_or_predicates (location_t loc, tree
   enum tree_code code1 = parse_predicate (c1, &op1a, &op1b);
   enum tree_code code2 = parse_predicate (c2, &op2a, &op2b);
 
   if (code1 != ERROR_MARK && code2 != ERROR_MARK)
     {
       tree t = maybe_fold_or_comparisons (code1, op1a, op1b,
-					  code2, op2a, op2b);
+					  code2, op2a, op2b, 0);
       if (t)
 	return t;
     }
 
   return fold_build2_loc (loc, TRUTH_OR_EXPR, boolean_type_node, c1, c2);
 }
Index: testsuite/gcc.dg/tree-ssa/ssa-ifcombine-8.c
===================================================================
--- testsuite/gcc.dg/tree-ssa/ssa-ifcombine-8.c	(revision 0)
+++ testsuite/gcc.dg/tree-ssa/ssa-ifcombine-8.c	(revision 0)
@@ -0,0 +1,24 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O -ftrapping-math -fdump-tree-ifcombine" } */
+
+double test1 (double i, double j)
+{
+  if (i >= j)
+    if (i <= j)
+      goto plif;
+    else
+      goto plouf;
+  else
+    goto plif;
+
+plif:
+  return 0;
+plouf:
+  return -1;
+}
+
+/* The above should be optimized to a i > j test by ifcombine,
+   which also switches plif and plouf.  */
+
+/* { dg-final { scan-tree-dump " > " "ifcombine" } } */
+/* { dg-final { cleanup-tree-dump "ifcombine" } } */

Property changes on: testsuite/gcc.dg/tree-ssa/ssa-ifcombine-8.c
___________________________________________________________________
Added: svn:keywords
   + Author Date Id Revision URL
Added: svn:eol-style
   + native

Index: testsuite/gcc.dg/tree-ssa/ssa-ifcombine-9.c
===================================================================
--- testsuite/gcc.dg/tree-ssa/ssa-ifcombine-9.c	(revision 0)
+++ testsuite/gcc.dg/tree-ssa/ssa-ifcombine-9.c	(revision 0)
@@ -0,0 +1,21 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-ifcombine" } */
+
+void f ();
+enum Sign { NEG=-1, ZERO, POS };
+
+static inline enum Sign sign (double x)
+{
+  if (x > 0) return POS;
+  if (x < 0) return NEG;
+  return ZERO;
+}
+void g (double x)
+{
+  if (sign (x) == NEG) f();
+}
+
+/* The above should be optimized to x < 0 by ifcombine.  */
+
+/* { dg-final { scan-tree-dump "optimizing.* < " "ifcombine" } } */
+/* { dg-final { cleanup-tree-dump "ifcombine" } } */