diff mbox series

[1/2] Match: zero_one_valued_p should match 0 constants too

Message ID 20230607001706.3000011-1-apinski@marvell.com
State Accepted
Headers
Series [1/2] Match: zero_one_valued_p should match 0 constants too |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Andrew Pinski June 7, 2023, 12:17 a.m. UTC 
  While working on `bool0 ? bool1 : bool2` I noticed that
zero_one_valued_p does not match on the constant zero
as in that case tree_nonzero_bits will return 0 and
that is different from 1.

OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.

gcc/ChangeLog:

	* match.pd (zero_one_valued_p): Match 0 integer constant
	too.
---
 gcc/match.pd | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Jeff Law June 7, 2023, 2:28 a.m. UTC | #1 
On 6/6/23 18:17, Andrew Pinski via Gcc-patches wrote:
> While working on `bool0 ? bool1 : bool2` I noticed that
> zero_one_valued_p does not match on the constant zero
> as in that case tree_nonzero_bits will return 0 and
> that is different from 1.
> 
> OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.
> 
> gcc/ChangeLog:
> 
> 	* match.pd (zero_one_valued_p): Match 0 integer constant
> 	too.
Presumably "1" is matched by the tree_nonzero_bits (@0) == 1.  So OK.

jeff
Jakub Jelinek June 7, 2023, 7:12 a.m. UTC | #2 
On Tue, Jun 06, 2023 at 05:17:05PM -0700, Andrew Pinski via Gcc-patches wrote:
> While working on `bool0 ? bool1 : bool2` I noticed that
> zero_one_valued_p does not match on the constant zero
> as in that case tree_nonzero_bits will return 0 and
> that is different from 1.
> 
> OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.
> 
> gcc/ChangeLog:
> 
> 	* match.pd (zero_one_valued_p): Match 0 integer constant
> 	too.
> ---
>  gcc/match.pd | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/gcc/match.pd b/gcc/match.pd
> index f9cbd757752..f97ff7ef760 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -1983,11 +1983,16 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>    (bit_not (bit_not @0))
>    @0)
>  
> +/* zero_one_valued_p will match when a value is known to be either
> +   0 or 1 including the constant 0. */
>  (match zero_one_valued_p
>   @0
>   (if (INTEGRAL_TYPE_P (type) && tree_nonzero_bits (@0) == 1)))

So perhaps instead change this to
&& wi::leu_p (tree_nonzero_bits (@0), 1)
?

>  (match zero_one_valued_p
>   truth_valued_p@0)
> +(match zero_one_valued_p
> + integer_zerop@0
> + (if (INTEGRAL_TYPE_P (type))))
>  
>  /* Transform { 0 or 1 } * { 0 or 1 } into { 0 or 1 } & { 0 or 1 }.  */
>  (simplify

	Jakub
Jeff Law June 7, 2023, 1:03 p.m. UTC | #3 
On 6/7/23 01:12, Jakub Jelinek via Gcc-patches wrote:

>>   
>> +/* zero_one_valued_p will match when a value is known to be either
>> +   0 or 1 including the constant 0. */
>>   (match zero_one_valued_p
>>    @0
>>    (if (INTEGRAL_TYPE_P (type) && tree_nonzero_bits (@0) == 1)))
> 
> So perhaps instead change this to
> && wi::leu_p (tree_nonzero_bits (@0), 1)
I guess that would cover both cases without the extra conditional.  I'm 
fine with that approach too.  Consider it pre-approved if someone wants 
to make that change.

jeff
diff mbox series

Patch

diff --git a/gcc/match.pd b/gcc/match.pd
index f9cbd757752..f97ff7ef760 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -1983,11 +1983,16 @@  DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
   (bit_not (bit_not @0))
   @0)
 
+/* zero_one_valued_p will match when a value is known to be either
+   0 or 1 including the constant 0. */
 (match zero_one_valued_p
  @0
  (if (INTEGRAL_TYPE_P (type) && tree_nonzero_bits (@0) == 1)))
 (match zero_one_valued_p
  truth_valued_p@0)
+(match zero_one_valued_p
+ integer_zerop@0
+ (if (INTEGRAL_TYPE_P (type))))
 
 /* Transform { 0 or 1 } * { 0 or 1 } into { 0 or 1 } & { 0 or 1 }.  */
 (simplify