Re: [PATCH/AARCH64] Improve aarch64 conditional compare usage

[Jeff Law <law at redhat dot com>]

On 05/01/2017 11:16 AM, Steve Ellcey wrote:
> This is a resubmittal of an earlier patch
> (https://gcc.gnu.org/ml/gcc-patches/2017-02/msg00203.html) to improve the
> use of ccmp (conditional compare) on aarch64.  I made a couple of tweaks
> after the first submittal and retested now that we are back in stage 1.
> 
> Most of the changes are restructuring the code to allow the change and do
> not affect the actual output.  The actual behavour change is in
> ccmp_tree_comparison_p where we recoginize a boolean variable as well
> as a compare expression as code that can be done with a conditionial
> compare and in get_compare_parts where we treat a boolean variable X
> as 'X != 0' and generate that comparision.
> 
> Since the code in ccmp.c is ony used when TARGET_GEN_CCMP_FIRST is set
> and TARGET_GEN_CCMP_FIRST is only set for aarch64 this change will only
> affect aarch64.
> 
> Tested with no regressions and a new test is added to verify that we
> generate a ccmp instruction with the change.  I ran the SPEC2006 int
> tests and got a .02 increase in the SPECmark on a ThunderX box. The
> biggest increases were in mcf and astar.  One test, xlancbmk, did slow
> down but the overall SPEC result was a speedup.
> 
> OK for checkin?
> 
> Steve Ellcey
> sellcey@cavium.com
> 
> 
> GCC ChangeLog:
> 
> 2017-05-01  Steve Ellcey  <sellcey@cavium.com>
> 
> 	* ccmp.c (ccmp_tree_comparison_p): New function.
> 	(ccmp_candidate_p): Update to use above function.
> 	(get_compare_parts): New function.
> 	(expand_ccmp_next): Update to use new functions.
> 	(expand_ccmp_expr_1): Take tree arg instead of gimple, update to use
> 	new functions.
> 	(expand_ccmp_expr): Pass tree instead of gimple to expand_ccmp_expr_1,
> 	take mode as argument.
> 	* ccmp.h (expand_ccmp_expr): Add mode as argument.
> 	* expr.c (expand_expr_real_1): Pass mode as argument.
> 
> GCC Testsuite ChangeLog:
> 
> 
> 2017-05-01  Steve Ellcey  <sellcey@cavium.com>
> 
> 	* gcc.target/aarch64/ccmp_2.c: New test.
> 
> 
> gcc.ccmp.patch
> 
> 
> diff --git a/gcc/ccmp.c b/gcc/ccmp.c
> index 92ca133..4fa3ebd 100644
> --- a/gcc/ccmp.c
> +++ b/gcc/ccmp.c
> @@ -38,6 +38,29 @@ along with GCC; see the file COPYING3.  If not see
>  #include "ccmp.h"
>  #include "predict.h"
>  
> +/* Check whether T is a simple boolean variable or a SSA name
> +   set by a comparison operator in the same basic block.  */
> +static bool
> +ccmp_tree_comparison_p (tree t, basic_block bb)
> +{
> +  gimple *g = get_gimple_for_ssa_name (t);
> +  tree_code tcode;
> +
> +  /* If we have a boolean variable allow it and generate a compare
> +     to zero reg when expanding.  */
> +  if (!g)
> +    return (TREE_CODE (TREE_TYPE (t)) == BOOLEAN_TYPE);
Depending on how you use T, you might be better off checking T's range
and considering anything with the [0,1] range as a boolean.  That would
also pick up the case where T was set via a comparison, or the output of
a PHI with arguments that are all [0,1], etc.  I've found that to be a
useful improvement in a couple places.

See ssa_name_has_boolean_range.  I don't consider it a requirement for
this patch to go forward, but more something you might want to
investigate as a future improvement.

OK for the trunk.  Sorry about the delay.

jeff