[2/3] Change the `zero_one ==/!= 0) ? y : z <op> y` patterns to use multiply rather than `(-zero_one) & z`
Checks
Context |
Check |
Description |
linaro-tcwg-bot/tcwg_gcc_build--master-arm |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 |
success
|
Testing passed
|
Commit Message
Since there is a pattern to convert `(-zero_one) & z` into `zero_one * z` already,
it is better if we don't do a secondary transformation. This reduces the extra
statements produced by match-and-simplify on the gimple level too.
gcc/ChangeLog:
* match.pd (`zero_one ==/!= 0) ? y : z <op> y`): Use
multiply rather than negation/bit_and.
---
gcc/match.pd | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
Comments
On 6/7/23 15:32, Andrew Pinski via Gcc-patches wrote:
> Since there is a pattern to convert `(-zero_one) & z` into `zero_one * z` already,
> it is better if we don't do a secondary transformation. This reduces the extra
> statements produced by match-and-simplify on the gimple level too.
>
> gcc/ChangeLog:
>
> * match.pd (`zero_one ==/!= 0) ? y : z <op> y`): Use
> multiply rather than negation/bit_and.
Don't you need to check the types in a manner similar to what the A & -Y
-> X * Y pattern does before you make this transformation?
jeff
On Wed, Jun 7, 2023 at 3:57 PM Jeff Law via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
>
>
> On 6/7/23 15:32, Andrew Pinski via Gcc-patches wrote:
> > Since there is a pattern to convert `(-zero_one) & z` into `zero_one * z` already,
> > it is better if we don't do a secondary transformation. This reduces the extra
> > statements produced by match-and-simplify on the gimple level too.
> >
> > gcc/ChangeLog:
> >
> > * match.pd (`zero_one ==/!= 0) ? y : z <op> y`): Use
> > multiply rather than negation/bit_and.
> Don't you need to check the types in a manner similar to what the A & -Y
> -> X * Y pattern does before you make this transformation?
No, because the convert is in a different order than in that
transformation; a very subtle difference which makes it work.
In A & -Y it was matching:
(bit_and (convert? (negate
But here we have:
(bit_and (negate (convert
Notice the convert is in a different location, in the `A & -Y` case,
the convert needs to be a sign extending (or a truncation) of the
negative value. Here we are converting the one_zero_value to the new
type so we get zero_one in the new type and then doing the negation
getting us 0 or -1 value.
Thanks,
Andrew
>
> jeff
>
On 6/7/23 17:05, Andrew Pinski wrote:
> On Wed, Jun 7, 2023 at 3:57 PM Jeff Law via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>>
>>
>>
>> On 6/7/23 15:32, Andrew Pinski via Gcc-patches wrote:
>>> Since there is a pattern to convert `(-zero_one) & z` into `zero_one * z` already,
>>> it is better if we don't do a secondary transformation. This reduces the extra
>>> statements produced by match-and-simplify on the gimple level too.
>>>
>>> gcc/ChangeLog:
>>>
>>> * match.pd (`zero_one ==/!= 0) ? y : z <op> y`): Use
>>> multiply rather than negation/bit_and.
>> Don't you need to check the types in a manner similar to what the A & -Y
>> -> X * Y pattern does before you make this transformation?
>
> No, because the convert is in a different order than in that
> transformation; a very subtle difference which makes it work.
>
> In A & -Y it was matching:
> (bit_and (convert? (negate
> But here we have:
> (bit_and (negate (convert
> Notice the convert is in a different location, in the `A & -Y` case,
> the convert needs to be a sign extending (or a truncation) of the
> negative value. Here we are converting the one_zero_value to the new
> type so we get zero_one in the new type and then doing the negation
> getting us 0 or -1 value.
THanks for the clarification. OK for the trunk.
jeff
On Wed, Jun 7, 2023 at 4:11 PM Jeff Law <jeffreyalaw@gmail.com> wrote:
>
>
>
> On 6/7/23 17:05, Andrew Pinski wrote:
> > On Wed, Jun 7, 2023 at 3:57 PM Jeff Law via Gcc-patches
> > <gcc-patches@gcc.gnu.org> wrote:
> >>
> >>
> >>
> >> On 6/7/23 15:32, Andrew Pinski via Gcc-patches wrote:
> >>> Since there is a pattern to convert `(-zero_one) & z` into `zero_one * z` already,
> >>> it is better if we don't do a secondary transformation. This reduces the extra
> >>> statements produced by match-and-simplify on the gimple level too.
> >>>
> >>> gcc/ChangeLog:
> >>>
> >>> * match.pd (`zero_one ==/!= 0) ? y : z <op> y`): Use
> >>> multiply rather than negation/bit_and.
> >> Don't you need to check the types in a manner similar to what the A & -Y
> >> -> X * Y pattern does before you make this transformation?
> >
> > No, because the convert is in a different order than in that
> > transformation; a very subtle difference which makes it work.
> >
> > In A & -Y it was matching:
> > (bit_and (convert? (negate
> > But here we have:
> > (bit_and (negate (convert
> > Notice the convert is in a different location, in the `A & -Y` case,
> > the convert needs to be a sign extending (or a truncation) of the
> > negative value. Here we are converting the one_zero_value to the new
> > type so we get zero_one in the new type and then doing the negation
> > getting us 0 or -1 value.
> THanks for the clarification. OK for the trunk.
So even though my transformation is correct based on what was done in
match.pd but that was broken already for signed one bit integers:
```
struct s
{
int t : 1;
};
int f(struct s t, int a, int b)
{
int bd = t.t;
if (bd) a|=b;
return a;
}
```
I am going to withdraw this patch and fix that up first.
Thanks,
Andrew
>
> jeff
@@ -3688,7 +3688,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(cond (le @0 integer_zerop@1) (negate@2 @0) integer_zerop@1)
(max @2 @1))
-/* (zero_one == 0) ? y : z <op> y -> (-(typeof(y))zero_one & z) <op> y */
+/* (zero_one == 0) ? y : z <op> y -> ((typeof(y))zero_one * z) <op> y */
(for op (bit_xor bit_ior)
(simplify
(cond (eq zero_one_valued_p@0
@@ -3698,9 +3698,9 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(if (INTEGRAL_TYPE_P (type)
&& TYPE_PRECISION (type) > 1
&& (INTEGRAL_TYPE_P (TREE_TYPE (@0))))
- (op (bit_and (negate (convert:type @0)) @2) @1))))
+ (op (mult (convert:type @0) @2) @1))))
-/* (zero_one != 0) ? z <op> y : y -> (-(typeof(y))zero_one & z) <op> y */
+/* (zero_one != 0) ? z <op> y : y -> ((typeof(y))zero_one * z) <op> y */
(for op (bit_xor bit_ior)
(simplify
(cond (ne zero_one_valued_p@0
@@ -3710,7 +3710,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(if (INTEGRAL_TYPE_P (type)
&& TYPE_PRECISION (type) > 1
&& (INTEGRAL_TYPE_P (TREE_TYPE (@0))))
- (op (bit_and (negate (convert:type @0)) @2) @1))))
+ (op (mult (convert:type @0) @2) @1))))
/* Simplifications of shift and rotates. */