diff mbox

Fix A < 0 ? <sign bit of A> : 0 optimization (PR middle-end/58564)

Message ID 20130930132908.GU30970@tucnak.zalov.cz
State New
Headers show

Commit Message

Jakub Jelinek Sept. 30, 2013, 1:29 p.m. UTC
Hi!

Apparently sign_bit_p looks through both sign and zero extensions.
That is just fine for the single bit test optimization, where we know the
constant is a power of two, and it is (A & power_of_two) == 0 (or != 0)
test.  sign_bit_p is also used to check for minimum value of some integral
type, because it is called with the same argument twice, it must be
INTEGER_CST if non-NULL is returned and thus there is no zero extensions.
But in the last spot where sign_bit_p is used, sign extensions would be
fine, ((int) x) < 0 for say signed char or signed short x iff x < 0,
but if there is a zero extension, it is just a possible missed optimization
on the comparison.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk/4.8?

2013-09-30  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/58564
	* fold-const.c (fold_ternary_loc): For A < 0 : <sign bit of A> : 0
	optimization, punt if sign_bit_p looked through any zero extension.

	* gcc.c-torture/execute/pr58564.c: New test.


	Jakub

Comments

Richard Biener Sept. 30, 2013, 1:38 p.m. UTC | #1
On Mon, 30 Sep 2013, Jakub Jelinek wrote:

> Hi!
> 
> Apparently sign_bit_p looks through both sign and zero extensions.
> That is just fine for the single bit test optimization, where we know the
> constant is a power of two, and it is (A & power_of_two) == 0 (or != 0)
> test.  sign_bit_p is also used to check for minimum value of some integral
> type, because it is called with the same argument twice, it must be
> INTEGER_CST if non-NULL is returned and thus there is no zero extensions.
> But in the last spot where sign_bit_p is used, sign extensions would be
> fine, ((int) x) < 0 for say signed char or signed short x iff x < 0,
> but if there is a zero extension, it is just a possible missed optimization
> on the comparison.
> 
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk/4.8?

Ok.

Thanks,
Richard.

> 2013-09-30  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR middle-end/58564
> 	* fold-const.c (fold_ternary_loc): For A < 0 : <sign bit of A> : 0
> 	optimization, punt if sign_bit_p looked through any zero extension.
> 
> 	* gcc.c-torture/execute/pr58564.c: New test.
> 
> --- gcc/fold-const.c.jj	2013-09-27 15:42:37.000000000 +0200
> +++ gcc/fold-const.c	2013-09-30 11:19:06.333978484 +0200
> @@ -14196,14 +14196,29 @@ fold_ternary_loc (location_t loc, enum t
>  	  && integer_zerop (op2)
>  	  && (tem = sign_bit_p (TREE_OPERAND (arg0, 0), arg1)))
>  	{
> +	  /* sign_bit_p looks through both zero and sign extensions,
> +	     but for this optimization only sign extensions are
> +	     usable.  */
> +	  tree tem2 = TREE_OPERAND (arg0, 0);
> +	  while (tem != tem2)
> +	    {
> +	      if (TREE_CODE (tem2) != NOP_EXPR
> +		  || TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (tem2, 0))))
> +		{
> +		  tem = NULL_TREE;
> +		  break;
> +		}
> +	      tem2 = TREE_OPERAND (tem2, 0);
> +	    }
>  	  /* sign_bit_p only checks ARG1 bits within A's precision.
>  	     If <sign bit of A> has wider type than A, bits outside
>  	     of A's precision in <sign bit of A> need to be checked.
>  	     If they are all 0, this optimization needs to be done
>  	     in unsigned A's type, if they are all 1 in signed A's type,
>  	     otherwise this can't be done.  */
> -	  if (TYPE_PRECISION (TREE_TYPE (tem))
> -	      < TYPE_PRECISION (TREE_TYPE (arg1))
> +	  if (tem
> +	      && TYPE_PRECISION (TREE_TYPE (tem))
> +		 < TYPE_PRECISION (TREE_TYPE (arg1))
>  	      && TYPE_PRECISION (TREE_TYPE (tem))
>  		 < TYPE_PRECISION (type))
>  	    {
> --- gcc/testsuite/gcc.c-torture/execute/pr58564.c.jj	2013-09-30 11:09:38.691122488 +0200
> +++ gcc/testsuite/gcc.c-torture/execute/pr58564.c	2013-09-30 11:09:14.000000000 +0200
> @@ -0,0 +1,14 @@
> +/* PR middle-end/58564 */
> +
> +extern void abort (void);
> +int a, b;
> +short *c, **d = &c;
> +
> +int
> +main ()
> +{
> +  b = (0, 0 > ((&c == d) & (1 && (a ^ 1)))) | 0U;
> +  if (b != 0)
> +    abort ();
> +  return 0;
> +}
> 
> 	Jakub
> 
>
diff mbox

Patch

--- gcc/fold-const.c.jj	2013-09-27 15:42:37.000000000 +0200
+++ gcc/fold-const.c	2013-09-30 11:19:06.333978484 +0200
@@ -14196,14 +14196,29 @@  fold_ternary_loc (location_t loc, enum t
 	  && integer_zerop (op2)
 	  && (tem = sign_bit_p (TREE_OPERAND (arg0, 0), arg1)))
 	{
+	  /* sign_bit_p looks through both zero and sign extensions,
+	     but for this optimization only sign extensions are
+	     usable.  */
+	  tree tem2 = TREE_OPERAND (arg0, 0);
+	  while (tem != tem2)
+	    {
+	      if (TREE_CODE (tem2) != NOP_EXPR
+		  || TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (tem2, 0))))
+		{
+		  tem = NULL_TREE;
+		  break;
+		}
+	      tem2 = TREE_OPERAND (tem2, 0);
+	    }
 	  /* sign_bit_p only checks ARG1 bits within A's precision.
 	     If <sign bit of A> has wider type than A, bits outside
 	     of A's precision in <sign bit of A> need to be checked.
 	     If they are all 0, this optimization needs to be done
 	     in unsigned A's type, if they are all 1 in signed A's type,
 	     otherwise this can't be done.  */
-	  if (TYPE_PRECISION (TREE_TYPE (tem))
-	      < TYPE_PRECISION (TREE_TYPE (arg1))
+	  if (tem
+	      && TYPE_PRECISION (TREE_TYPE (tem))
+		 < TYPE_PRECISION (TREE_TYPE (arg1))
 	      && TYPE_PRECISION (TREE_TYPE (tem))
 		 < TYPE_PRECISION (type))
 	    {
--- gcc/testsuite/gcc.c-torture/execute/pr58564.c.jj	2013-09-30 11:09:38.691122488 +0200
+++ gcc/testsuite/gcc.c-torture/execute/pr58564.c	2013-09-30 11:09:14.000000000 +0200
@@ -0,0 +1,14 @@ 
+/* PR middle-end/58564 */
+
+extern void abort (void);
+int a, b;
+short *c, **d = &c;
+
+int
+main ()
+{
+  b = (0, 0 > ((&c == d) & (1 && (a ^ 1)))) | 0U;
+  if (b != 0)
+    abort ();
+  return 0;
+}