public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] phiopt: Improve conditional_replacement for x ? 0 : -1 [PR796232]
@ 2020-12-05  9:10 Jakub Jelinek
  2020-12-05 10:20 ` Richard Biener
  0 siblings, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2020-12-05  9:10 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches

Hi!

As mentioned in the PR, for boolean x we currently optimize
in phiopt x ? 0 : -1 into -(int)!x but it can be optimized as
(int) x - 1 which is one less operation both in GIMPLE and in x86 assembly.

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

And/or, shall we have a match.pd optimization to turn that -(type)!x
for BOOLEAN_TYPE (or other 1 bit unsigned precision values) into
(type) - 1.

2020-12-05  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/96232
	* tree-ssa-phiopt.c (conditional_replacement): Optimize
	x ? 0 : -1 as (int) x - 1 rather than -(int)!x.

	* gcc.dg/tree-ssa/pr96232-1.c: New test.

--- gcc/tree-ssa-phiopt.c.jj	2020-11-04 11:58:58.670252748 +0100
+++ gcc/tree-ssa-phiopt.c	2020-12-04 17:27:53.472837921 +0100
@@ -827,10 +827,24 @@ conditional_replacement (basic_block con
 
   if (neg)
     {
-      cond = fold_convert_loc (gimple_location (stmt),
-                               TREE_TYPE (result), cond);
-      cond = fold_build1_loc (gimple_location (stmt),
-                              NEGATE_EXPR, TREE_TYPE (cond), cond);
+      if (TREE_CODE (cond) == TRUTH_NOT_EXPR
+	  && INTEGRAL_TYPE_P (TREE_TYPE (nonzero_arg)))
+	{
+	  /* x ? 0 : -1 is better optimized as (int) x - 1 than
+	     -(int)!x.  */
+	  cond = fold_convert_loc (gimple_location (stmt),
+				   TREE_TYPE (result),
+				   TREE_OPERAND (cond, 0));
+	  cond = fold_build2_loc (gimple_location (stmt), PLUS_EXPR,
+				  TREE_TYPE (result), cond, nonzero_arg);
+	}
+      else
+	{
+	  cond = fold_convert_loc (gimple_location (stmt),
+				   TREE_TYPE (result), cond);
+	  cond = fold_build1_loc (gimple_location (stmt),
+				  NEGATE_EXPR, TREE_TYPE (cond), cond);
+	}
     }
   else if (shift)
     {
--- gcc/testsuite/gcc.dg/tree-ssa/pr96232-1.c.jj	2020-12-04 17:32:40.607615276 +0100
+++ gcc/testsuite/gcc.dg/tree-ssa/pr96232-1.c	2020-12-04 17:33:09.914286354 +0100
@@ -0,0 +1,11 @@
+/* PR tree-optimization/96232 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+/* { dg-final { scan-tree-dump " \\+ -1;" "optimized" } } */
+/* { dg-final { scan-tree-dump-not "~x_\[0-9]*\\\(D\\\)" "optimized" } } */
+
+int
+foo (_Bool x)
+{
+  return x ? 0 : -1;
+}

	Jakub


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

* Re: [PATCH] phiopt: Improve conditional_replacement for x ? 0 : -1 [PR796232]
  2020-12-05  9:10 [PATCH] phiopt: Improve conditional_replacement for x ? 0 : -1 [PR796232] Jakub Jelinek
@ 2020-12-05 10:20 ` Richard Biener
  2020-12-05 10:57   ` [PATCH] match.pd: " Jakub Jelinek
  0 siblings, 1 reply; 4+ messages in thread
From: Richard Biener @ 2020-12-05 10:20 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

On December 5, 2020 10:10:25 AM GMT+01:00, Jakub Jelinek <jakub@redhat.com> wrote:
>Hi!
>
>As mentioned in the PR, for boolean x we currently optimize
>in phiopt x ? 0 : -1 into -(int)!x but it can be optimized as
>(int) x - 1 which is one less operation both in GIMPLE and in x86
>assembly.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
>And/or, shall we have a match.pd optimization to turn that -(type)!x
>for BOOLEAN_TYPE (or other 1 bit unsigned precision values) into
>(type) - 1.

I think that would make sense. Does that then cover the phiopt case directly? 

>2020-12-05  Jakub Jelinek  <jakub@redhat.com>
>
>	PR tree-optimization/96232
>	* tree-ssa-phiopt.c (conditional_replacement): Optimize
>	x ? 0 : -1 as (int) x - 1 rather than -(int)!x.
>
>	* gcc.dg/tree-ssa/pr96232-1.c: New test.
>
>--- gcc/tree-ssa-phiopt.c.jj	2020-11-04 11:58:58.670252748 +0100
>+++ gcc/tree-ssa-phiopt.c	2020-12-04 17:27:53.472837921 +0100
>@@ -827,10 +827,24 @@ conditional_replacement (basic_block con
> 
>   if (neg)
>     {
>-      cond = fold_convert_loc (gimple_location (stmt),
>-                               TREE_TYPE (result), cond);
>-      cond = fold_build1_loc (gimple_location (stmt),
>-                              NEGATE_EXPR, TREE_TYPE (cond), cond);
>+      if (TREE_CODE (cond) == TRUTH_NOT_EXPR
>+	  && INTEGRAL_TYPE_P (TREE_TYPE (nonzero_arg)))
>+	{
>+	  /* x ? 0 : -1 is better optimized as (int) x - 1 than
>+	     -(int)!x.  */
>+	  cond = fold_convert_loc (gimple_location (stmt),
>+				   TREE_TYPE (result),
>+				   TREE_OPERAND (cond, 0));
>+	  cond = fold_build2_loc (gimple_location (stmt), PLUS_EXPR,
>+				  TREE_TYPE (result), cond, nonzero_arg);
>+	}
>+      else
>+	{
>+	  cond = fold_convert_loc (gimple_location (stmt),
>+				   TREE_TYPE (result), cond);
>+	  cond = fold_build1_loc (gimple_location (stmt),
>+				  NEGATE_EXPR, TREE_TYPE (cond), cond);
>+	}
>     }
>   else if (shift)
>     {
>--- gcc/testsuite/gcc.dg/tree-ssa/pr96232-1.c.jj	2020-12-04
>17:32:40.607615276 +0100
>+++ gcc/testsuite/gcc.dg/tree-ssa/pr96232-1.c	2020-12-04
>17:33:09.914286354 +0100
>@@ -0,0 +1,11 @@
>+/* PR tree-optimization/96232 */
>+/* { dg-do compile } */
>+/* { dg-options "-O2 -fdump-tree-optimized" } */
>+/* { dg-final { scan-tree-dump " \\+ -1;" "optimized" } } */
>+/* { dg-final { scan-tree-dump-not "~x_\[0-9]*\\\(D\\\)" "optimized" }
>} */
>+
>+int
>+foo (_Bool x)
>+{
>+  return x ? 0 : -1;
>+}
>
>	Jakub


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

* [PATCH] match.pd: Improve conditional_replacement for x ? 0 : -1 [PR796232]
  2020-12-05 10:20 ` Richard Biener
@ 2020-12-05 10:57   ` Jakub Jelinek
  2020-12-05 12:51     ` Richard Biener
  0 siblings, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2020-12-05 10:57 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches

On Sat, Dec 05, 2020 at 11:20:11AM +0100, Richard Biener wrote:
> >As mentioned in the PR, for boolean x we currently optimize
> >in phiopt x ? 0 : -1 into -(int)!x but it can be optimized as
> >(int) x - 1 which is one less operation both in GIMPLE and in x86
> >assembly.
> >
> >Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> >
> >And/or, shall we have a match.pd optimization to turn that -(type)!x
> >for BOOLEAN_TYPE (or other 1 bit unsigned precision values) into
> >(type) - 1.
> 
> I think that would make sense. Does that then cover the phiopt case directly? 

That would be the following then.  Seems it works for that case.
Ok for trunk if it passes bootstrap/regtest?

2020-12-05  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/96232
	* match.pd (-(type)!A -> (type)A - 1): New optimization.

	* gcc.dg/tree-ssa/pr96232-1.c: New test.

--- gcc/match.pd.jj	2020-12-02 11:20:24.765486816 +0100
+++ gcc/match.pd	2020-12-05 11:46:00.554518927 +0100
@@ -3812,6 +3812,16 @@ (define_operator_list COND_TERNARY
   (cnd (logical_inverted_value truth_valued_p@0) @1 @2)
   (cnd @0 @2 @1)))
 
+/* -(type)!A -> (type)A - 1.  */
+(simplify
+ (negate (convert?:s (logical_inverted_value:s @0)))
+ (if (INTEGRAL_TYPE_P (type)
+      && TREE_CODE (type) != BOOLEAN_TYPE
+      && TYPE_PRECISION (type) > 1
+      && TREE_CODE (@0) == SSA_NAME
+      && ssa_name_has_boolean_range (@0))
+  (plus (convert:type @0) { build_all_ones_cst (type); })))
+
 /* A + (B vcmp C ? 1 : 0) -> A - (B vcmp C ? -1 : 0), since vector comparisons
    return all -1 or all 0 results.  */
 /* ??? We could instead convert all instances of the vec_cond to negate,
--- gcc/testsuite/gcc.dg/tree-ssa/pr96232-1.c.jj	2020-12-05 11:37:27.804332875 +0100
+++ gcc/testsuite/gcc.dg/tree-ssa/pr96232-1.c	2020-12-05 11:37:27.804332875 +0100
@@ -0,0 +1,11 @@
+/* PR tree-optimization/96232 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+/* { dg-final { scan-tree-dump " \\+ -1;" "optimized" } } */
+/* { dg-final { scan-tree-dump-not "~x_\[0-9]*\\\(D\\\)" "optimized" } } */
+
+int
+foo (_Bool x)
+{
+  return x ? 0 : -1;
+}


	Jakub


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

* Re: [PATCH] match.pd: Improve conditional_replacement for x ? 0 : -1 [PR796232]
  2020-12-05 10:57   ` [PATCH] match.pd: " Jakub Jelinek
@ 2020-12-05 12:51     ` Richard Biener
  0 siblings, 0 replies; 4+ messages in thread
From: Richard Biener @ 2020-12-05 12:51 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

On December 5, 2020 11:57:46 AM GMT+01:00, Jakub Jelinek <jakub@redhat.com> wrote:
>On Sat, Dec 05, 2020 at 11:20:11AM +0100, Richard Biener wrote:
>> >As mentioned in the PR, for boolean x we currently optimize
>> >in phiopt x ? 0 : -1 into -(int)!x but it can be optimized as
>> >(int) x - 1 which is one less operation both in GIMPLE and in x86
>> >assembly.
>> >
>> >Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>> >
>> >And/or, shall we have a match.pd optimization to turn that -(type)!x
>> >for BOOLEAN_TYPE (or other 1 bit unsigned precision values) into
>> >(type) - 1.
>> 
>> I think that would make sense. Does that then cover the phiopt case
>directly? 
>
>That would be the following then.  Seems it works for that case.
>Ok for trunk if it passes bootstrap/regtest?

Ok. 

Richard. 

>2020-12-05  Jakub Jelinek  <jakub@redhat.com>
>
>	PR tree-optimization/96232
>	* match.pd (-(type)!A -> (type)A - 1): New optimization.
>
>	* gcc.dg/tree-ssa/pr96232-1.c: New test.
>
>--- gcc/match.pd.jj	2020-12-02 11:20:24.765486816 +0100
>+++ gcc/match.pd	2020-12-05 11:46:00.554518927 +0100
>@@ -3812,6 +3812,16 @@ (define_operator_list COND_TERNARY
>   (cnd (logical_inverted_value truth_valued_p@0) @1 @2)
>   (cnd @0 @2 @1)))
> 
>+/* -(type)!A -> (type)A - 1.  */
>+(simplify
>+ (negate (convert?:s (logical_inverted_value:s @0)))
>+ (if (INTEGRAL_TYPE_P (type)
>+      && TREE_CODE (type) != BOOLEAN_TYPE
>+      && TYPE_PRECISION (type) > 1
>+      && TREE_CODE (@0) == SSA_NAME
>+      && ssa_name_has_boolean_range (@0))
>+  (plus (convert:type @0) { build_all_ones_cst (type); })))
>+
>/* A + (B vcmp C ? 1 : 0) -> A - (B vcmp C ? -1 : 0), since vector
>comparisons
>    return all -1 or all 0 results.  */
>/* ??? We could instead convert all instances of the vec_cond to
>negate,
>--- gcc/testsuite/gcc.dg/tree-ssa/pr96232-1.c.jj	2020-12-05
>11:37:27.804332875 +0100
>+++ gcc/testsuite/gcc.dg/tree-ssa/pr96232-1.c	2020-12-05
>11:37:27.804332875 +0100
>@@ -0,0 +1,11 @@
>+/* PR tree-optimization/96232 */
>+/* { dg-do compile } */
>+/* { dg-options "-O2 -fdump-tree-optimized" } */
>+/* { dg-final { scan-tree-dump " \\+ -1;" "optimized" } } */
>+/* { dg-final { scan-tree-dump-not "~x_\[0-9]*\\\(D\\\)" "optimized" }
>} */
>+
>+int
>+foo (_Bool x)
>+{
>+  return x ? 0 : -1;
>+}
>
>
>	Jakub


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

end of thread, other threads:[~2020-12-05 12:51 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-05  9:10 [PATCH] phiopt: Improve conditional_replacement for x ? 0 : -1 [PR796232] Jakub Jelinek
2020-12-05 10:20 ` Richard Biener
2020-12-05 10:57   ` [PATCH] match.pd: " Jakub Jelinek
2020-12-05 12:51     ` 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).