public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix VRP handling of __builtin_ffs* (PR tree-optimization/61725)
@ 2014-07-08  7:48 Jakub Jelinek
  2014-07-08  8:50 ` Richard Biener
  0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2014-07-08  7:48 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches

Hi!

When writing this code, I've been assuming __builtin_ffs* argument
is unsigned, which is the case of popcount also handled there, and
many other builtins (clz, ctz, ...), except that clrsb has signed argument.
But in reality for some reason ffs has signed argument and some time ago
Marek has even changed the documentation to match the code rather than the
other way around.

So, here is an attempt to deal with it; to return range starting from 1
rather than 0 we need to test for argument range that doesn't include
zero (I'm now including range_includes_zero_p for that), and in case of
maximum of the range if minimum is negative, then one of the negative
values is in the range and therefore the topmost bit is sometimes set.

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

2014-07-08  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/61725
	* tree-vrp.c (extract_range_basic): Don't assume vr0 is unsigned
	range, use range_includes_zerop_p instead of integer_zerop on
	vr0->min, only use log2 of max if min is not negative.

	* gcc.dg/tree-ssa/vrp93.c: New test.
	* gcc.c-torture/execute/pr61725.c: New test.

--- gcc/tree-vrp.c.jj	2014-06-24 09:38:12.000000000 +0200
+++ gcc/tree-vrp.c	2014-07-07 11:26:30.308749523 +0200
@@ -3536,15 +3536,18 @@ extract_range_basic (value_range_t *vr,
 	      /* If arg is non-zero, then ffs or popcount
 		 are non-zero.  */
 	      if (((vr0->type == VR_RANGE
-		    && integer_nonzerop (vr0->min))
+		    && range_includes_zero_p (vr0->min, vr0->max) == 0)
 		   || (vr0->type == VR_ANTI_RANGE
-		       && integer_zerop (vr0->min)))
-		  && !is_overflow_infinity (vr0->min))
+		       && range_includes_zero_p (vr0->min, vr0->max) == 1))
+		  && !is_overflow_infinity (vr0->min)
+		  && !is_overflow_infinity (vr0->max))
 		mini = 1;
 	      /* If some high bits are known to be zero,
 		 we can decrease the maximum.  */
 	      if (vr0->type == VR_RANGE
 		  && TREE_CODE (vr0->max) == INTEGER_CST
+		  && !operand_less_p (vr0->min,
+				      build_zero_cst (TREE_TYPE (vr0->min)))
 		  && !is_overflow_infinity (vr0->max))
 		maxi = tree_floor_log2 (vr0->max) + 1;
 	    }
--- gcc/testsuite/gcc.dg/tree-ssa/vrp93.c.jj	2014-07-07 16:37:39.247344076 +0200
+++ gcc/testsuite/gcc.dg/tree-ssa/vrp93.c	2014-07-07 16:46:33.523847807 +0200
@@ -0,0 +1,36 @@
+/* PR target/29776 */
+/* PR tree-optimization/61725 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-vrp1" } */
+/* { dg-final { scan-tree-dump-not "link_error" "vrp1"} } */
+/* { dg-final { cleanup-tree-dump "vrp1" } } */
+
+#define A(fn, arg, min, max) \
+  if (__builtin_##fn (arg) < min || __builtin_##fn (arg) > max) \
+    link_error ();
+#define B(fn, min, max) \
+  A (fn, a, min, max) A (fn##l, b, min, max) A (fn##ll, c, min, max)
+#define C(fn, min, sub) \
+  A (fn, a, min, ((int) sizeof (a) * __CHAR_BIT__ - sub)) \
+  A (fn##l, b, min, ((int) sizeof (b) * __CHAR_BIT__ - sub)) \
+  A (fn##ll, c, min, ((int) sizeof (c) * __CHAR_BIT__ - sub))
+
+extern void link_error (void);
+
+unsigned int d;
+unsigned long e;
+unsigned long long f;
+
+void
+foo (int a, long b, long long c)
+{
+  C (ffs, 0, 0)
+  a &= 63; b &= 63; c &= 63;
+  B (ffs, 0, 6)
+  a++; b++; c++;
+  B (ffs, 1, 7)
+  a -= 2; b -= 2; c -= 2;
+  C (ffs, 0, 0)
+  a -= 63; b -= 63; c -= 63;
+  C (ffs, 1, 0)
+}
--- gcc/testsuite/gcc.c-torture/execute/pr61725.c.jj	2014-07-07 11:04:07.936218135 +0200
+++ gcc/testsuite/gcc.c-torture/execute/pr61725.c	2014-07-07 11:03:47.000000000 +0200
@@ -0,0 +1,14 @@
+/* PR tree-optimization/61725 */
+
+int
+main ()
+{
+  int x;
+  for (x = -128; x <= 128; x++)
+    {
+      int a = __builtin_ffs (x);
+      if (x == 0 && a != 0)
+        __builtin_abort ();
+    }
+  return 0;
+}

	Jakub

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

* Re: [PATCH] Fix VRP handling of __builtin_ffs* (PR tree-optimization/61725)
  2014-07-08  7:48 [PATCH] Fix VRP handling of __builtin_ffs* (PR tree-optimization/61725) Jakub Jelinek
@ 2014-07-08  8:50 ` Richard Biener
  0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2014-07-08  8:50 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

On Tue, 8 Jul 2014, Jakub Jelinek wrote:

> Hi!
> 
> When writing this code, I've been assuming __builtin_ffs* argument
> is unsigned, which is the case of popcount also handled there, and
> many other builtins (clz, ctz, ...), except that clrsb has signed argument.
> But in reality for some reason ffs has signed argument and some time ago
> Marek has even changed the documentation to match the code rather than the
> other way around.
> 
> So, here is an attempt to deal with it; to return range starting from 1
> rather than 0 we need to test for argument range that doesn't include
> zero (I'm now including range_includes_zero_p for that), and in case of
> maximum of the range if minimum is negative, then one of the negative
> values is in the range and therefore the topmost bit is sometimes set.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/4.9?

Ok.

Thanks,
Richard.

> 2014-07-08  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/61725
> 	* tree-vrp.c (extract_range_basic): Don't assume vr0 is unsigned
> 	range, use range_includes_zerop_p instead of integer_zerop on
> 	vr0->min, only use log2 of max if min is not negative.
> 
> 	* gcc.dg/tree-ssa/vrp93.c: New test.
> 	* gcc.c-torture/execute/pr61725.c: New test.
> 
> --- gcc/tree-vrp.c.jj	2014-06-24 09:38:12.000000000 +0200
> +++ gcc/tree-vrp.c	2014-07-07 11:26:30.308749523 +0200
> @@ -3536,15 +3536,18 @@ extract_range_basic (value_range_t *vr,
>  	      /* If arg is non-zero, then ffs or popcount
>  		 are non-zero.  */
>  	      if (((vr0->type == VR_RANGE
> -		    && integer_nonzerop (vr0->min))
> +		    && range_includes_zero_p (vr0->min, vr0->max) == 0)
>  		   || (vr0->type == VR_ANTI_RANGE
> -		       && integer_zerop (vr0->min)))
> -		  && !is_overflow_infinity (vr0->min))
> +		       && range_includes_zero_p (vr0->min, vr0->max) == 1))
> +		  && !is_overflow_infinity (vr0->min)
> +		  && !is_overflow_infinity (vr0->max))
>  		mini = 1;
>  	      /* If some high bits are known to be zero,
>  		 we can decrease the maximum.  */
>  	      if (vr0->type == VR_RANGE
>  		  && TREE_CODE (vr0->max) == INTEGER_CST
> +		  && !operand_less_p (vr0->min,
> +				      build_zero_cst (TREE_TYPE (vr0->min)))
>  		  && !is_overflow_infinity (vr0->max))
>  		maxi = tree_floor_log2 (vr0->max) + 1;
>  	    }
> --- gcc/testsuite/gcc.dg/tree-ssa/vrp93.c.jj	2014-07-07 16:37:39.247344076 +0200
> +++ gcc/testsuite/gcc.dg/tree-ssa/vrp93.c	2014-07-07 16:46:33.523847807 +0200
> @@ -0,0 +1,36 @@
> +/* PR target/29776 */
> +/* PR tree-optimization/61725 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-vrp1" } */
> +/* { dg-final { scan-tree-dump-not "link_error" "vrp1"} } */
> +/* { dg-final { cleanup-tree-dump "vrp1" } } */
> +
> +#define A(fn, arg, min, max) \
> +  if (__builtin_##fn (arg) < min || __builtin_##fn (arg) > max) \
> +    link_error ();
> +#define B(fn, min, max) \
> +  A (fn, a, min, max) A (fn##l, b, min, max) A (fn##ll, c, min, max)
> +#define C(fn, min, sub) \
> +  A (fn, a, min, ((int) sizeof (a) * __CHAR_BIT__ - sub)) \
> +  A (fn##l, b, min, ((int) sizeof (b) * __CHAR_BIT__ - sub)) \
> +  A (fn##ll, c, min, ((int) sizeof (c) * __CHAR_BIT__ - sub))
> +
> +extern void link_error (void);
> +
> +unsigned int d;
> +unsigned long e;
> +unsigned long long f;
> +
> +void
> +foo (int a, long b, long long c)
> +{
> +  C (ffs, 0, 0)
> +  a &= 63; b &= 63; c &= 63;
> +  B (ffs, 0, 6)
> +  a++; b++; c++;
> +  B (ffs, 1, 7)
> +  a -= 2; b -= 2; c -= 2;
> +  C (ffs, 0, 0)
> +  a -= 63; b -= 63; c -= 63;
> +  C (ffs, 1, 0)
> +}
> --- gcc/testsuite/gcc.c-torture/execute/pr61725.c.jj	2014-07-07 11:04:07.936218135 +0200
> +++ gcc/testsuite/gcc.c-torture/execute/pr61725.c	2014-07-07 11:03:47.000000000 +0200
> @@ -0,0 +1,14 @@
> +/* PR tree-optimization/61725 */
> +
> +int
> +main ()
> +{
> +  int x;
> +  for (x = -128; x <= 128; x++)
> +    {
> +      int a = __builtin_ffs (x);
> +      if (x == 0 && a != 0)
> +        __builtin_abort ();
> +    }
> +  return 0;
> +}
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer

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

end of thread, other threads:[~2014-07-08  8:50 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-08  7:48 [PATCH] Fix VRP handling of __builtin_ffs* (PR tree-optimization/61725) Jakub Jelinek
2014-07-08  8:50 ` 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).