match.pd handling of three-constant bitops

natch.pd tries to reassociate two bit operations if both of them have
constant operands.  However, with the polynomial integers added later,
there's no guarantee that a bit operation on two integers can be folded
at compile time.  This means that the pattern can trigger for operations
on three constants, and as things stood could endlessly oscillate
between the two associations.

This patch keeps the existing pattern for the normal case of a
non-constant first operand.  When all three operands are constant it
tries to find a pair of constants that do fold.  If none do, it keeps
the original expression as-was.

Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64le-linus-gnu.
OK to install?

Richard


2017-09-20  Richard Sandiford  <richard.sandiford@linaro.org>
	    Alan Hayward  <alan.hayward@arm.com>
	    David Sherwood  <david.sherwood@arm.com>

gcc/
	* match.pd: Handle bit operations involving three constants
	and try to fold one pair.

On Wed, Sep 20, 2017 at 2:18 PM, Richard Sandiford
<richard.sandiford@linaro.org> wrote:
> natch.pd tries to reassociate two bit operations if both of them have
> constant operands.  However, with the polynomial integers added later,
> there's no guarantee that a bit operation on two integers can be folded
> at compile time.  This means that the pattern can trigger for operations
> on three constants, and as things stood could endlessly oscillate
> between the two associations.
>
> This patch keeps the existing pattern for the normal case of a
> non-constant first operand.  When all three operands are constant it
> tries to find a pair of constants that do fold.  If none do, it keeps
> the original expression as-was.
>
> Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64le-linus-gnu.
> OK to install?

Hmm, is the complication with trying variants necessary or could we simply do

 if (!CONSTANT_CLASS_P (@0))
  (bitop @0 (bitop @1 @2))

and be done with it?

> Richard
>
>
> 2017-09-20  Richard Sandiford  <richard.sandiford@linaro.org>
>             Alan Hayward  <alan.hayward@arm.com>
>             David Sherwood  <david.sherwood@arm.com>
>
> gcc/
>         * match.pd: Handle bit operations involving three constants
>         and try to fold one pair.
>
> Index: gcc/match.pd
> ===================================================================
> --- gcc/match.pd        2017-09-16 21:38:21.106513157 +0100
> +++ gcc/match.pd        2017-09-20 13:17:10.552389270 +0100
> @@ -1017,7 +1017,14 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>  (for bitop (bit_and bit_ior bit_xor)
>   (simplify
>    (bitop (bitop @0 CONSTANT_CLASS_P@1) CONSTANT_CLASS_P@2)
> -  (bitop @0 (bitop @1 @2))))
> +  (if (!CONSTANT_CLASS_P (@0))
> +   (bitop @0 (bitop @1 @2))
> +   (with { tree cst1 = const_binop (bitop, type, @0, @2); }
> +    (if (cst1)
> +     (bitop @1 { cst1; })
> +     (with { tree cst2 = const_binop (bitop, type, @1, @2); }
> +      (if (cst2)
> +       (bitop @0 { cst2; }))))))))
>
>  /* Try simple folding for X op !X, and X op X with the help
>     of the truth_valued_p and logical_inverted_value predicates.  */

Richard Biener <richard.guenther@gmail.com> writes:
> On Wed, Sep 20, 2017 at 2:18 PM, Richard Sandiford
> <richard.sandiford@linaro.org> wrote:
>> natch.pd tries to reassociate two bit operations if both of them have
>> constant operands.  However, with the polynomial integers added later,
>> there's no guarantee that a bit operation on two integers can be folded
>> at compile time.  This means that the pattern can trigger for operations
>> on three constants, and as things stood could endlessly oscillate
>> between the two associations.
>>
>> This patch keeps the existing pattern for the normal case of a
>> non-constant first operand.  When all three operands are constant it
>> tries to find a pair of constants that do fold.  If none do, it keeps
>> the original expression as-was.
>>
>> Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64le-linus-gnu.
>> OK to install?
>
> Hmm, is the complication with trying variants necessary or could we simply do
>
>  if (!CONSTANT_CLASS_P (@0))
>   (bitop @0 (bitop @1 @2))
>
> and be done with it?

That's enough to fix the oscillation, but there are cases that benefit
from trying the other combinations.  E.g. if @1 and @2 are both
INTEGER_CSTs, it's still worth folding them.  POLY_CST | INTEGER_CST
is supported for some values, so if @0 | @1 doesn't fold, it's worth
trying @0 | @2 instead.

Thanks,
Richard

On Thu, Sep 21, 2017 at 1:17 PM, Richard Sandiford
<richard.sandiford@linaro.org> wrote:
> Richard Biener <richard.guenther@gmail.com> writes:
>> On Wed, Sep 20, 2017 at 2:18 PM, Richard Sandiford
>> <richard.sandiford@linaro.org> wrote:
>>> natch.pd tries to reassociate two bit operations if both of them have
>>> constant operands.  However, with the polynomial integers added later,
>>> there's no guarantee that a bit operation on two integers can be folded
>>> at compile time.  This means that the pattern can trigger for operations
>>> on three constants, and as things stood could endlessly oscillate
>>> between the two associations.
>>>
>>> This patch keeps the existing pattern for the normal case of a
>>> non-constant first operand.  When all three operands are constant it
>>> tries to find a pair of constants that do fold.  If none do, it keeps
>>> the original expression as-was.
>>>
>>> Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64le-linus-gnu.
>>> OK to install?
>>
>> Hmm, is the complication with trying variants necessary or could we simply do
>>
>>  if (!CONSTANT_CLASS_P (@0))
>>   (bitop @0 (bitop @1 @2))
>>
>> and be done with it?
>
> That's enough to fix the oscillation, but there are cases that benefit
> from trying the other combinations.  E.g. if @1 and @2 are both
> INTEGER_CSTs, it's still worth folding them.  POLY_CST | INTEGER_CST
> is supported for some values, so if @0 | @1 doesn't fold, it's worth
> trying @0 | @2 instead.

Hmm, ok.

The patch is ok then if you add a comment reflecting the above.

Thanks,
Richard.

>
> Thanks,
> Richard

Richard Biener <richard.guenther@gmail.com> writes:
> On Thu, Sep 21, 2017 at 1:17 PM, Richard Sandiford
> <richard.sandiford@linaro.org> wrote:
>> Richard Biener <richard.guenther@gmail.com> writes:
>>> On Wed, Sep 20, 2017 at 2:18 PM, Richard Sandiford
>>> <richard.sandiford@linaro.org> wrote:
>>>> natch.pd tries to reassociate two bit operations if both of them have
>>>> constant operands.  However, with the polynomial integers added later,
>>>> there's no guarantee that a bit operation on two integers can be folded
>>>> at compile time.  This means that the pattern can trigger for operations
>>>> on three constants, and as things stood could endlessly oscillate
>>>> between the two associations.
>>>>
>>>> This patch keeps the existing pattern for the normal case of a
>>>> non-constant first operand.  When all three operands are constant it
>>>> tries to find a pair of constants that do fold.  If none do, it keeps
>>>> the original expression as-was.
>>>>
>>>> Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64le-linus-gnu.
>>>> OK to install?
>>>
>>> Hmm, is the complication with trying variants necessary or could we simply do
>>>
>>>  if (!CONSTANT_CLASS_P (@0))
>>>   (bitop @0 (bitop @1 @2))
>>>
>>> and be done with it?
>>
>> That's enough to fix the oscillation, but there are cases that benefit
>> from trying the other combinations.  E.g. if @1 and @2 are both
>> INTEGER_CSTs, it's still worth folding them.  POLY_CST | INTEGER_CST
>> is supported for some values, so if @0 | @1 doesn't fold, it's worth
>> trying @0 | @2 instead.
>
> Hmm, ok.
>
> The patch is ok then if you add a comment reflecting the above.

Thanks.  For the record, here's what I installed after retesting.

Richard


2018-01-03  Richard Sandiford  <richard.sandiford@linaro.org>
	    Alan Hayward  <alan.hayward@arm.com>
	    David Sherwood  <david.sherwood@arm.com>

gcc/
	* match.pd: Handle bit operations involving three constants
	and try to fold one pair.
------------------------------------------------------------------------------

Index: gcc/match.pd
===================================================================
--- gcc/match.pd	2018-01-02 16:55:03.498204314 +0000
+++ gcc/match.pd	2018-01-03 07:10:06.993953631 +0000
@@ -1111,7 +1111,24 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
 (for bitop (bit_and bit_ior bit_xor)
  (simplify
   (bitop (bitop @0 CONSTANT_CLASS_P@1) CONSTANT_CLASS_P@2)
-  (bitop @0 (bitop @1 @2))))
+  (if (!CONSTANT_CLASS_P (@0))
+   /* This is the canonical form regardless of whether (bitop @1 @2) can be
+      folded to a constant.  */
+   (bitop @0 (bitop @1 @2))
+   /* In this case we have three constants and (bitop @0 @1) doesn't fold
+      to a constant.  This can happen if @0 or @1 is a POLY_INT_CST and if
+      the values involved are such that the operation can't be decided at
+      compile time.  Try folding one of @0 or @1 with @2 to see whether
+      that combination can be decided at compile time.
+
+      Keep the existing form if both folds fail, to avoid endless
+      oscillation.  */
+   (with { tree cst1 = const_binop (bitop, type, @0, @2); }
+    (if (cst1)
+     (bitop @1 { cst1; })
+     (with { tree cst2 = const_binop (bitop, type, @1, @2); }
+      (if (cst2)
+       (bitop @0 { cst2; }))))))))
 
 /* Try simple folding for X op !X, and X op X with the help
    of the truth_valued_p and logical_inverted_value predicates.  */