Re: [PATCH] Optimize x ? bswap(x) : 0 in tree-ssa-phiopt

2021-08-05 Thread Martin Liška

Hello.

I noticed the patch caused new Clang warnings:

/home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-clang/build/gcc/tree-ssa-phiopt.c:2586:10:
 warning: comparison of different enumeration types in switch statement 
('combined_fn' and 'built_in_function') [-Wenum-compare-switch]
/home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-clang/build/gcc/tree-ssa-phiopt.c:2589:10:
 warning: comparison of different enumeration types in switch statement 
('combined_fn' and 'built_in_function') [-Wenum-compare-switch]
/home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-clang/build/gcc/tree-ssa-phiopt.c:2592:10:
 warning: comparison of different enumeration types in switch statement 
('combined_fn' and 'built_in_function') [-Wenum-compare-switch]

Can you please take a look?
Thanks,
Martin


Re: [PATCH] Optimize x ? bswap(x) : 0 in tree-ssa-phiopt

2021-08-02 Thread Richard Biener via Gcc-patches
On Sat, Jul 31, 2021 at 9:56 AM Roger Sayle  wrote:
>
>
> Many thanks again to Jakub Jelinek for a speedy fix for PR 101642.
> Interestingly, that test case "bswap16(x) ? : x" also reveals a
> missed optimization opportunity.  The resulting "x ? bswap(x) : 0"
> can be further simplified to just bswap(x).
>
> Conveniently, tree-ssa-phiopt.c already recognizes/optimizes the
> related "x ? popcount(x) : 0", so this patch simply makes that
> transformation make general, additionally handling bswap, parity,
> ffs and clrsb.  All of the required infrastructure is already
> present thanks to Jakub previously adding support for clz/ctz.
> To reflect this generalization, the name of the function is changed
> from cond_removal_in_popcount_clz_ctz_pattern to the hopefully
> equally descriptive cond_removal_in_builtin_zero_pattern.
>
> The following patch has been tested on x86_64-pc-linux-gnu with a
> "make bootstrap" and "make -k check" with no new failures.
>
> Ok for mainline?

OK.

Thanks,
Richard.

>
> 2021-07-31  Roger Sayle  
>
> gcc/ChangeLog
> * tree-ssa-phiopt.c (cond_removal_in_builtin_zero_pattern):
> Renamed from cond_removal_in_popcount_clz_ctz_pattern.
> Add support for BSWAP, FFS, PARITY and CLRSB builtins.
> (tree_ssa_phiop_worker): Update call to function above.
>
> gcc/testuite/ChangeLog
> * gcc.dg/tree-ssa/phi-opt-25.c: New test case.
>
>
> Roger
> --
> Roger Sayle
> NextMove Software
> Cambridge, UK
>


[PATCH] Optimize x ? bswap(x) : 0 in tree-ssa-phiopt

2021-07-31 Thread Roger Sayle

Many thanks again to Jakub Jelinek for a speedy fix for PR 101642.
Interestingly, that test case "bswap16(x) ? : x" also reveals a
missed optimization opportunity.  The resulting "x ? bswap(x) : 0"
can be further simplified to just bswap(x).

Conveniently, tree-ssa-phiopt.c already recognizes/optimizes the
related "x ? popcount(x) : 0", so this patch simply makes that
transformation make general, additionally handling bswap, parity,
ffs and clrsb.  All of the required infrastructure is already
present thanks to Jakub previously adding support for clz/ctz.
To reflect this generalization, the name of the function is changed
from cond_removal_in_popcount_clz_ctz_pattern to the hopefully
equally descriptive cond_removal_in_builtin_zero_pattern.

The following patch has been tested on x86_64-pc-linux-gnu with a
"make bootstrap" and "make -k check" with no new failures.

Ok for mainline?


2021-07-31  Roger Sayle  

gcc/ChangeLog
* tree-ssa-phiopt.c (cond_removal_in_builtin_zero_pattern):
Renamed from cond_removal_in_popcount_clz_ctz_pattern.
Add support for BSWAP, FFS, PARITY and CLRSB builtins.
(tree_ssa_phiop_worker): Update call to function above.

gcc/testuite/ChangeLog
* gcc.dg/tree-ssa/phi-opt-25.c: New test case.


Roger
--
Roger Sayle
NextMove Software
Cambridge, UK

diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c
index c6adbbd..66af902 100644
--- a/gcc/tree-ssa-phiopt.c
+++ b/gcc/tree-ssa-phiopt.c
@@ -66,9 +66,9 @@ static bool minmax_replacement (basic_block, basic_block,
edge, edge, gphi *, tree, tree);
 static bool spaceship_replacement (basic_block, basic_block,
   edge, edge, gphi *, tree, tree);
-static bool cond_removal_in_popcount_clz_ctz_pattern (basic_block, basic_block,
- edge, edge, gphi *,
- tree, tree);
+static bool cond_removal_in_builtin_zero_pattern (basic_block, basic_block,
+ edge, edge, gphi *,
+ tree, tree);
 static bool cond_store_replacement (basic_block, basic_block, edge, edge,
hash_set *);
 static bool cond_if_else_store_replacement (basic_block, basic_block, 
basic_block);
@@ -350,9 +350,8 @@ tree_ssa_phiopt_worker (bool do_store_elim, bool 
do_hoist_loads, bool early_p)
   early_p))
cfgchanged = true;
  else if (!early_p
-  && cond_removal_in_popcount_clz_ctz_pattern (bb, bb1, e1,
-   e2, phi, arg0,
-   arg1))
+  && cond_removal_in_builtin_zero_pattern (bb, bb1, e1, e2,
+   phi, arg0, arg1))
cfgchanged = true;
  else if (minmax_replacement (bb, bb1, e1, e2, phi, arg0, arg1))
cfgchanged = true;
@@ -2466,7 +2465,8 @@ spaceship_replacement (basic_block cond_bb, basic_block 
middle_bb,
   return true;
 }
 
-/* Convert
+/* Optimize x ? __builtin_fun (x) : C, where C is __builtin_fun (0).
+   Convert
 

if (b_4(D) != 0)
@@ -2498,10 +2498,10 @@ spaceship_replacement (basic_block cond_bb, basic_block 
middle_bb,
instead of 0 above it uses the value from that macro.  */
 
 static bool
-cond_removal_in_popcount_clz_ctz_pattern (basic_block cond_bb,
- basic_block middle_bb,
- edge e1, edge e2, gphi *phi,
- tree arg0, tree arg1)
+cond_removal_in_builtin_zero_pattern (basic_block cond_bb,
+ basic_block middle_bb,
+ edge e1, edge e2, gphi *phi,
+ tree arg0, tree arg1)
 {
   gimple *cond;
   gimple_stmt_iterator gsi, gsi_from;
@@ -2549,6 +2549,12 @@ cond_removal_in_popcount_clz_ctz_pattern (basic_block 
cond_bb,
   int val = 0;
   switch (cfn)
 {
+case CFN_BUILT_IN_BSWAP16:
+case CFN_BUILT_IN_BSWAP32:
+case CFN_BUILT_IN_BSWAP64:
+case CFN_BUILT_IN_BSWAP128:
+CASE_CFN_FFS:
+CASE_CFN_PARITY:
 CASE_CFN_POPCOUNT:
   break;
 CASE_CFN_CLZ:
@@ -2577,6 +2583,15 @@ cond_removal_in_popcount_clz_ctz_pattern (basic_block 
cond_bb,
}
}
   return false;
+case BUILT_IN_CLRSB:
+  val = TYPE_PRECISION (integer_type_node) - 1;
+  break;
+case BUILT_IN_CLRSBL:
+  val = TYPE_PRECISION (long_integer_type_node) - 1;
+  break;
+case BUILT_IN_CLRSBLL:
+  val = TYPE_PRECISION (long_long_integer_type_node) - 1;
+  break;
 default:
   return false;
 }
/* { dg-do compile } */
/* { dg-options "-O2 -fdump-tree-optimized" } */

unsi