public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] ranger: Fix up wi_fold_in_parts for small precision types [PR104334]
@ 2022-02-03  7:56 Jakub Jelinek
  2022-02-03  8:34 ` Richard Biener
  0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2022-02-03  7:56 UTC (permalink / raw)
  To: Richard Biener, Andrew MacLeod; +Cc: gcc-patches

Hi!

The wide-int.h templates expect that when an int/long etc. operand is used
it will be sign-extended based on the types precision.
wi_fold_in_parts passes 3 such non-zero constants to wi::lt_p, wi::gt_p
and wi::eq_p - 1, 3 and 4, which means it was doing weird things if either
some of 1, 3 or 4 weren't representable in type, or if type was unsigned 3 bit
type 4 should be written as -4.
The following patch promotes the subtraction operands to widest_int and
uses that as the type for ?h_range variables and compares them as such.
We don't need the overflow handling because there is never an overflow.

Bootstrapped/regtested on x86_64-linux and i686-linux (on the former also
with lto bootstrap), ok for trunk?

2022-02-02  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/104334
	* range-op.cc (range_operator::wi_fold_in_parts): Change lh_range
	and rh_range type to widest_int and subtract in widest_int.  Remove
	ov_rh, ov_lh and sign vars, always perform comparisons as signed
	and use >, < and == operators for it.

	* g++.dg/opt/pr104334.C: New test.

--- gcc/range-op.cc.jj	2022-01-13 22:29:15.345831749 +0100
+++ gcc/range-op.cc	2022-02-02 20:20:22.020000000 +0100
@@ -144,22 +144,21 @@ range_operator::wi_fold_in_parts (irange
 				  const wide_int &rh_lb,
 				  const wide_int &rh_ub) const
 {
-  wi::overflow_type ov_rh, ov_lh;
   int_range_max tmp;
-  wide_int rh_range = wi::sub (rh_ub, rh_lb, TYPE_SIGN (type), &ov_rh);
-  wide_int lh_range = wi::sub (lh_ub, lh_lb, TYPE_SIGN (type), &ov_lh);
-  signop sign = TYPE_SIGN (type);;
+  widest_int rh_range = wi::sub (widest_int::from (rh_ub, TYPE_SIGN (type)),
+				 widest_int::from (rh_lb, TYPE_SIGN (type)));
+  widest_int lh_range = wi::sub (widest_int::from (lh_ub, TYPE_SIGN (type)),
+				 widest_int::from (lh_lb, TYPE_SIGN (type)));
   // If there are 2, 3, or 4 values in the RH range, do them separately.
   // Call wi_fold_in_parts to check the RH side.
-  if (wi::gt_p (rh_range, 0, sign) && wi::lt_p (rh_range, 4, sign)
-      && ov_rh == wi::OVF_NONE)
+  if (rh_range > 0 && rh_range < 4)
     {
       wi_fold_in_parts (r, type, lh_lb, lh_ub, rh_lb, rh_lb);
-      if (wi::gt_p (rh_range, 1, sign))
+      if (rh_range > 1)
 	{
 	  wi_fold_in_parts (tmp, type, lh_lb, lh_ub, rh_lb + 1, rh_lb + 1);
 	  r.union_ (tmp);
-	  if (wi::eq_p (rh_range, 3))
+	  if (rh_range == 3)
 	    {
 	      wi_fold_in_parts (tmp, type, lh_lb, lh_ub, rh_lb + 2, rh_lb + 2);
 	      r.union_ (tmp);
@@ -170,15 +169,14 @@ range_operator::wi_fold_in_parts (irange
     }
   // Otherise check for 2, 3, or 4 values in the LH range and split them up.
   // The RH side has been checked, so no recursion needed.
-  else if (wi::gt_p (lh_range, 0, sign) && wi::lt_p (lh_range, 4, sign)
-	   && ov_lh == wi::OVF_NONE)
+  else if (lh_range > 0 && lh_range < 4)
     {
       wi_fold (r, type, lh_lb, lh_lb, rh_lb, rh_ub);
-      if (wi::gt_p (lh_range, 1, sign))
+      if (lh_range > 1)
 	{
 	  wi_fold (tmp, type, lh_lb + 1, lh_lb + 1, rh_lb, rh_ub);
 	  r.union_ (tmp);
-	  if (wi::eq_p (lh_range, 3))
+	  if (lh_range == 3)
 	    {
 	      wi_fold (tmp, type, lh_lb + 2, lh_lb + 2, rh_lb, rh_ub);
 	      r.union_ (tmp);
--- gcc/testsuite/g++.dg/opt/pr104334.C.jj	2022-02-02 14:35:51.184657968 +0100
+++ gcc/testsuite/g++.dg/opt/pr104334.C	2022-02-02 14:37:14.888478594 +0100
@@ -0,0 +1,40 @@
+// PR tree-optimization/104334
+// { dg-do run { target c++11 } }
+// { dg-options "-O2 --param logical-op-non-short-circuit=0" }
+
+enum class A { A0, A1, A2, A3 };
+int x;
+
+__attribute__((noipa)) void
+baz ()
+{
+  x = 1;
+}
+
+struct B {
+  unsigned b : 2;
+
+  A
+  foo () const
+  {
+    return static_cast<A> (b);
+  }
+
+  __attribute__((noinline)) void
+  bar ()
+  {
+    if (foo () == A::A2 || foo () == A::A3)
+      baz ();
+  }
+};
+
+int
+main ()
+{
+  B c;
+  c.b = 2;
+  c.bar ();
+  if (x != 1)
+    __builtin_abort ();
+  return 0;
+}

	Jakub


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] ranger: Fix up wi_fold_in_parts for small precision types [PR104334]
  2022-02-03  7:56 [PATCH] ranger: Fix up wi_fold_in_parts for small precision types [PR104334] Jakub Jelinek
@ 2022-02-03  8:34 ` Richard Biener
  0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2022-02-03  8:34 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Andrew MacLeod, gcc-patches

On Thu, 3 Feb 2022, Jakub Jelinek wrote:

> Hi!
> 
> The wide-int.h templates expect that when an int/long etc. operand is used
> it will be sign-extended based on the types precision.
> wi_fold_in_parts passes 3 such non-zero constants to wi::lt_p, wi::gt_p
> and wi::eq_p - 1, 3 and 4, which means it was doing weird things if either
> some of 1, 3 or 4 weren't representable in type, or if type was unsigned 3 bit
> type 4 should be written as -4.
> The following patch promotes the subtraction operands to widest_int and
> uses that as the type for ?h_range variables and compares them as such.
> We don't need the overflow handling because there is never an overflow.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux (on the former also
> with lto bootstrap), ok for trunk?

OK.

Thanks,
Richard.

> 2022-02-02  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/104334
> 	* range-op.cc (range_operator::wi_fold_in_parts): Change lh_range
> 	and rh_range type to widest_int and subtract in widest_int.  Remove
> 	ov_rh, ov_lh and sign vars, always perform comparisons as signed
> 	and use >, < and == operators for it.
> 
> 	* g++.dg/opt/pr104334.C: New test.
> 
> --- gcc/range-op.cc.jj	2022-01-13 22:29:15.345831749 +0100
> +++ gcc/range-op.cc	2022-02-02 20:20:22.020000000 +0100
> @@ -144,22 +144,21 @@ range_operator::wi_fold_in_parts (irange
>  				  const wide_int &rh_lb,
>  				  const wide_int &rh_ub) const
>  {
> -  wi::overflow_type ov_rh, ov_lh;
>    int_range_max tmp;
> -  wide_int rh_range = wi::sub (rh_ub, rh_lb, TYPE_SIGN (type), &ov_rh);
> -  wide_int lh_range = wi::sub (lh_ub, lh_lb, TYPE_SIGN (type), &ov_lh);
> -  signop sign = TYPE_SIGN (type);;
> +  widest_int rh_range = wi::sub (widest_int::from (rh_ub, TYPE_SIGN (type)),
> +				 widest_int::from (rh_lb, TYPE_SIGN (type)));
> +  widest_int lh_range = wi::sub (widest_int::from (lh_ub, TYPE_SIGN (type)),
> +				 widest_int::from (lh_lb, TYPE_SIGN (type)));
>    // If there are 2, 3, or 4 values in the RH range, do them separately.
>    // Call wi_fold_in_parts to check the RH side.
> -  if (wi::gt_p (rh_range, 0, sign) && wi::lt_p (rh_range, 4, sign)
> -      && ov_rh == wi::OVF_NONE)
> +  if (rh_range > 0 && rh_range < 4)
>      {
>        wi_fold_in_parts (r, type, lh_lb, lh_ub, rh_lb, rh_lb);
> -      if (wi::gt_p (rh_range, 1, sign))
> +      if (rh_range > 1)
>  	{
>  	  wi_fold_in_parts (tmp, type, lh_lb, lh_ub, rh_lb + 1, rh_lb + 1);
>  	  r.union_ (tmp);
> -	  if (wi::eq_p (rh_range, 3))
> +	  if (rh_range == 3)
>  	    {
>  	      wi_fold_in_parts (tmp, type, lh_lb, lh_ub, rh_lb + 2, rh_lb + 2);
>  	      r.union_ (tmp);
> @@ -170,15 +169,14 @@ range_operator::wi_fold_in_parts (irange
>      }
>    // Otherise check for 2, 3, or 4 values in the LH range and split them up.
>    // The RH side has been checked, so no recursion needed.
> -  else if (wi::gt_p (lh_range, 0, sign) && wi::lt_p (lh_range, 4, sign)
> -	   && ov_lh == wi::OVF_NONE)
> +  else if (lh_range > 0 && lh_range < 4)
>      {
>        wi_fold (r, type, lh_lb, lh_lb, rh_lb, rh_ub);
> -      if (wi::gt_p (lh_range, 1, sign))
> +      if (lh_range > 1)
>  	{
>  	  wi_fold (tmp, type, lh_lb + 1, lh_lb + 1, rh_lb, rh_ub);
>  	  r.union_ (tmp);
> -	  if (wi::eq_p (lh_range, 3))
> +	  if (lh_range == 3)
>  	    {
>  	      wi_fold (tmp, type, lh_lb + 2, lh_lb + 2, rh_lb, rh_ub);
>  	      r.union_ (tmp);
> --- gcc/testsuite/g++.dg/opt/pr104334.C.jj	2022-02-02 14:35:51.184657968 +0100
> +++ gcc/testsuite/g++.dg/opt/pr104334.C	2022-02-02 14:37:14.888478594 +0100
> @@ -0,0 +1,40 @@
> +// PR tree-optimization/104334
> +// { dg-do run { target c++11 } }
> +// { dg-options "-O2 --param logical-op-non-short-circuit=0" }
> +
> +enum class A { A0, A1, A2, A3 };
> +int x;
> +
> +__attribute__((noipa)) void
> +baz ()
> +{
> +  x = 1;
> +}
> +
> +struct B {
> +  unsigned b : 2;
> +
> +  A
> +  foo () const
> +  {
> +    return static_cast<A> (b);
> +  }
> +
> +  __attribute__((noinline)) void
> +  bar ()
> +  {
> +    if (foo () == A::A2 || foo () == A::A3)
> +      baz ();
> +  }
> +};
> +
> +int
> +main ()
> +{
> +  B c;
> +  c.b = 2;
> +  c.bar ();
> +  if (x != 1)
> +    __builtin_abort ();
> +  return 0;
> +}
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-02-03  8:34 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-03  7:56 [PATCH] ranger: Fix up wi_fold_in_parts for small precision types [PR104334] Jakub Jelinek
2022-02-03  8:34 ` Richard Biener

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).