match.pd: Fix up a ? cst1 : cst2 regression on signed bool [PR111668]

Message ID ZRxRdeneRzguiH9R@tucnak
State Unresolved
Headers
Series match.pd: Fix up a ? cst1 : cst2 regression on signed bool [PR111668] |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

Jakub Jelinek Oct. 3, 2023, 5:37 p.m. UTC
  Hi!

My relatively recent changes to these simplifiers to avoid
doing build_nonstandard_integer_type (primarily for BITINT_TYPE)
broke PR111668, a recurrence of the PR110487 bug.
I thought the build_nonstandard_integer_type isn't ever needed there,
but there is one special case where it is.
For the a ? -1 : 0 and a ? 0 : -1 simplifications there are actually
3 different cases.  One is for signed 1-bit precision types (signed
kind of implied from integer_all_onesp, because otherwise it would
match integer_onep earlier), where the simplifier wierdly was matching
them using the a ? powerof2cst : 0 -> a << (log2(powerof2cst))
simplification and then another simplifier optimizing away the left shift
when log2(powerof2cst) was 0.  Another one is signed BOOLEAN_TYPE with
precision > 1, where indeed we shouldn't be doing the negation in type,
because it isn't well defined in that type, the type only has 2 valid
values, 0 and -1.  As an alternative, we could also e.g. cast to
signed 1-bit precision BOOLEAN_TYPE and then extend to type.
And the last case is what we were doing for types which have both 1 and -1
(all all ones) as valid values (i.e. all signed/unsigned ENUMERAL_TYPEs,
INTEGRAL_TYPEs and BITINT_TYPEs with precision > 1).

The following patch avoids the hops through << 0 for 1-bit precision
and uses build_nonstandard_integer_type solely for the BOOLEAN_TYPE types
(where we have a guarantee the precision is reasonably small, nothing ought
to be created 129+ bit precision BOOLEAN_TYPEs).

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2023-10-03  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/111668
	* match.pd (a ? CST1 : CST2): Handle the a ? -1 : 0 and
	a ? 0 : -1 cases before the powerof2cst cases and differentiate
	between 1-bit precision types, larger precision boolean types
	and other integral types.  Fix comment pastos and formatting.


	Jakub
  

Comments

Richard Biener Oct. 4, 2023, 7:19 a.m. UTC | #1
On Tue, 3 Oct 2023, Jakub Jelinek wrote:

> Hi!
> 
> My relatively recent changes to these simplifiers to avoid
> doing build_nonstandard_integer_type (primarily for BITINT_TYPE)
> broke PR111668, a recurrence of the PR110487 bug.
> I thought the build_nonstandard_integer_type isn't ever needed there,
> but there is one special case where it is.
> For the a ? -1 : 0 and a ? 0 : -1 simplifications there are actually
> 3 different cases.  One is for signed 1-bit precision types (signed
> kind of implied from integer_all_onesp, because otherwise it would
> match integer_onep earlier), where the simplifier wierdly was matching
> them using the a ? powerof2cst : 0 -> a << (log2(powerof2cst))
> simplification and then another simplifier optimizing away the left shift
> when log2(powerof2cst) was 0.  Another one is signed BOOLEAN_TYPE with
> precision > 1, where indeed we shouldn't be doing the negation in type,
> because it isn't well defined in that type, the type only has 2 valid
> values, 0 and -1.  As an alternative, we could also e.g. cast to
> signed 1-bit precision BOOLEAN_TYPE and then extend to type.
> And the last case is what we were doing for types which have both 1 and -1
> (all all ones) as valid values (i.e. all signed/unsigned ENUMERAL_TYPEs,
> INTEGRAL_TYPEs and BITINT_TYPEs with precision > 1).
> 
> The following patch avoids the hops through << 0 for 1-bit precision
> and uses build_nonstandard_integer_type solely for the BOOLEAN_TYPE types
> (where we have a guarantee the precision is reasonably small, nothing ought
> to be created 129+ bit precision BOOLEAN_TYPEs).
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

> 2023-10-03  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/111668
> 	* match.pd (a ? CST1 : CST2): Handle the a ? -1 : 0 and
> 	a ? 0 : -1 cases before the powerof2cst cases and differentiate
> 	between 1-bit precision types, larger precision boolean types
> 	and other integral types.  Fix comment pastos and formatting.
> 
> --- gcc/match.pd.jj	2023-10-02 09:42:01.657836005 +0200
> +++ gcc/match.pd	2023-10-03 10:33:30.817614648 +0200
> @@ -5100,36 +5100,53 @@ (define_operator_list SYNC_FETCH_AND_AND
>   (switch
>    (if (integer_zerop (@2))
>     (switch
> -    /* a ? 1 : 0 -> a if 0 and 1 are integral types. */
> +    /* a ? 1 : 0 -> a if 0 and 1 are integral types.  */
>      (if (integer_onep (@1))
>       (convert (convert:boolean_type_node @0)))
> +    /* a ? -1 : 0 -> -a.  */
> +    (if (INTEGRAL_TYPE_P (type) && integer_all_onesp (@1))
> +     (if (TYPE_PRECISION (type) == 1)
> +      /* For signed 1-bit precision just cast bool to the type.  */
> +      (convert (convert:boolean_type_node @0))
> +      (if (TREE_CODE (type) == BOOLEAN_TYPE)
> +       (with {
> +	  tree intt = build_nonstandard_integer_type (TYPE_PRECISION (type),
> +						      TYPE_UNSIGNED (type));
> +	}
> +	(convert (negate (convert:intt (convert:boolean_type_node @0)))))
> +       (negate (convert:type (convert:boolean_type_node @0))))))
>      /* a ? powerof2cst : 0 -> a << (log2(powerof2cst)) */
>      (if (INTEGRAL_TYPE_P (type) && integer_pow2p (@1))
>       (with {
>         tree shift = build_int_cst (integer_type_node, tree_log2 (@1));
>        }
> -      (lshift (convert (convert:boolean_type_node @0)) { shift; })))
> -    /* a ? -1 : 0 -> -a.  No need to check the TYPE_PRECISION not being 1
> -       here as the powerof2cst case above will handle that case correctly.  */
> -    (if (INTEGRAL_TYPE_P (type) && integer_all_onesp (@1))
> -     (negate (convert:type (convert:boolean_type_node @0))))))
> +      (lshift (convert (convert:boolean_type_node @0)) { shift; })))))
>    (if (integer_zerop (@1))
>     (switch
> -    /* a ? 0 : 1 -> !a. */
> +    /* a ? 0 : 1 -> !a.  */
>      (if (integer_onep (@2))
>       (convert (bit_xor (convert:boolean_type_node @0) { boolean_true_node; })))
> -    /* a ? powerof2cst : 0 -> (!a) << (log2(powerof2cst)) */
> +    /* a ? 0 : -1 -> -(!a).  */
> +    (if (INTEGRAL_TYPE_P (type) && integer_all_onesp (@2))
> +     (if (TYPE_PRECISION (type) == 1)
> +      /* For signed 1-bit precision just cast bool to the type.  */
> +      (convert (bit_xor (convert:boolean_type_node @0) { boolean_true_node; }))
> +      (if (TREE_CODE (type) == BOOLEAN_TYPE)
> +       (with {
> +	  tree intt = build_nonstandard_integer_type (TYPE_PRECISION (type),
> +						      TYPE_UNSIGNED (type));
> +	}
> +	(convert (negate (convert:intt (bit_xor (convert:boolean_type_node @0)
> +						{ boolean_true_node; })))))
> +       (negate (convert:type (bit_xor (convert:boolean_type_node @0)
> +				      { boolean_true_node; }))))))
> +    /* a ? 0 : powerof2cst -> (!a) << (log2(powerof2cst)) */
>      (if (INTEGRAL_TYPE_P (type) && integer_pow2p (@2))
>       (with {
>         tree shift = build_int_cst (integer_type_node, tree_log2 (@2));
>        }
>        (lshift (convert (bit_xor (convert:boolean_type_node @0)
> -				{ boolean_true_node; })) { shift; })))
> -    /* a ? -1 : 0 -> -(!a).  No need to check the TYPE_PRECISION not being 1
> -       here as the powerof2cst case above will handle that case correctly.  */
> -    (if (INTEGRAL_TYPE_P (type) && integer_all_onesp (@2))
> -     (negate (convert:type (bit_xor (convert:boolean_type_node @0)
> -				    { boolean_true_node; }))))))))
> +				{ boolean_true_node; })) { shift; })))))))
>  
>  /* (a > 1) ? 0 : (cast)a is the same as (cast)(a == 1)
>     for unsigned types. */
> 
> 	Jakub
> 
>
  

Patch

--- gcc/match.pd.jj	2023-10-02 09:42:01.657836005 +0200
+++ gcc/match.pd	2023-10-03 10:33:30.817614648 +0200
@@ -5100,36 +5100,53 @@  (define_operator_list SYNC_FETCH_AND_AND
  (switch
   (if (integer_zerop (@2))
    (switch
-    /* a ? 1 : 0 -> a if 0 and 1 are integral types. */
+    /* a ? 1 : 0 -> a if 0 and 1 are integral types.  */
     (if (integer_onep (@1))
      (convert (convert:boolean_type_node @0)))
+    /* a ? -1 : 0 -> -a.  */
+    (if (INTEGRAL_TYPE_P (type) && integer_all_onesp (@1))
+     (if (TYPE_PRECISION (type) == 1)
+      /* For signed 1-bit precision just cast bool to the type.  */
+      (convert (convert:boolean_type_node @0))
+      (if (TREE_CODE (type) == BOOLEAN_TYPE)
+       (with {
+	  tree intt = build_nonstandard_integer_type (TYPE_PRECISION (type),
+						      TYPE_UNSIGNED (type));
+	}
+	(convert (negate (convert:intt (convert:boolean_type_node @0)))))
+       (negate (convert:type (convert:boolean_type_node @0))))))
     /* a ? powerof2cst : 0 -> a << (log2(powerof2cst)) */
     (if (INTEGRAL_TYPE_P (type) && integer_pow2p (@1))
      (with {
        tree shift = build_int_cst (integer_type_node, tree_log2 (@1));
       }
-      (lshift (convert (convert:boolean_type_node @0)) { shift; })))
-    /* a ? -1 : 0 -> -a.  No need to check the TYPE_PRECISION not being 1
-       here as the powerof2cst case above will handle that case correctly.  */
-    (if (INTEGRAL_TYPE_P (type) && integer_all_onesp (@1))
-     (negate (convert:type (convert:boolean_type_node @0))))))
+      (lshift (convert (convert:boolean_type_node @0)) { shift; })))))
   (if (integer_zerop (@1))
    (switch
-    /* a ? 0 : 1 -> !a. */
+    /* a ? 0 : 1 -> !a.  */
     (if (integer_onep (@2))
      (convert (bit_xor (convert:boolean_type_node @0) { boolean_true_node; })))
-    /* a ? powerof2cst : 0 -> (!a) << (log2(powerof2cst)) */
+    /* a ? 0 : -1 -> -(!a).  */
+    (if (INTEGRAL_TYPE_P (type) && integer_all_onesp (@2))
+     (if (TYPE_PRECISION (type) == 1)
+      /* For signed 1-bit precision just cast bool to the type.  */
+      (convert (bit_xor (convert:boolean_type_node @0) { boolean_true_node; }))
+      (if (TREE_CODE (type) == BOOLEAN_TYPE)
+       (with {
+	  tree intt = build_nonstandard_integer_type (TYPE_PRECISION (type),
+						      TYPE_UNSIGNED (type));
+	}
+	(convert (negate (convert:intt (bit_xor (convert:boolean_type_node @0)
+						{ boolean_true_node; })))))
+       (negate (convert:type (bit_xor (convert:boolean_type_node @0)
+				      { boolean_true_node; }))))))
+    /* a ? 0 : powerof2cst -> (!a) << (log2(powerof2cst)) */
     (if (INTEGRAL_TYPE_P (type) && integer_pow2p (@2))
      (with {
        tree shift = build_int_cst (integer_type_node, tree_log2 (@2));
       }
       (lshift (convert (bit_xor (convert:boolean_type_node @0)
-				{ boolean_true_node; })) { shift; })))
-    /* a ? -1 : 0 -> -(!a).  No need to check the TYPE_PRECISION not being 1
-       here as the powerof2cst case above will handle that case correctly.  */
-    (if (INTEGRAL_TYPE_P (type) && integer_all_onesp (@2))
-     (negate (convert:type (bit_xor (convert:boolean_type_node @0)
-				    { boolean_true_node; }))))))))
+				{ boolean_true_node; })) { shift; })))))))
 
 /* (a > 1) ? 0 : (cast)a is the same as (cast)(a == 1)
    for unsigned types. */