Re: [PATCH 1/4] PowerPC: Change cmove function return to bool

[will schmidt <will_schmidt@vnet.ibm.com>]

On Wed, 2020-08-26 at 22:43 -0400, Michael Meissner via Gcc-patches wrote:
> PowerPC: Change cmove function return to bool.
> 
> In doing the other work for adding ISA 3.1 128-bit minimum, maximum, and
> conditional move support, I noticed the two functions that process conditional
> moves return 'int' instead of 'bool'.  This patch changes these functions to
> return 'bool'.
> 
> I have built compilers on a little endian power9 Linux system with all 4
> patches applied.  I did bootstrap builds and ran the testsuite, with no
> regressions.  Previous versions of the patch was also tested on a little endian
> power8 Linux system.  I would like to check this patch into the master
> branch for GCC 11.  At this time, I do not anticipate needing to backport these
> changes to GCC 10.3.
> 
> gcc/
> 2020-08-26  Michael Meissner  <meissner@linux.ibm.com>
> 
> 	* config/rs6000/rs6000-protos.h (rs6000_emit_cmove): Change return
> 	type to bool.
> 	(rs6000_emit_int_cmove): Change return type to bool.
> 	* config/rs6000/rs6000.c (rs6000_emit_cmove): Change return type
> 	to bool.
> 	(rs6000_emit_int_cmove): Change return type to bool.
> 


lgtm,
thanks
-Will


> ---
>  gcc/config/rs6000/rs6000-protos.h |  4 ++--
>  gcc/config/rs6000/rs6000.c        | 32 +++++++++++++++----------------
>  2 files changed, 18 insertions(+), 18 deletions(-)
> 
> diff --git a/gcc/config/rs6000/rs6000-protos.h b/gcc/config/rs6000/rs6000-protos.h
> index 28e859f4381..02e4d71922f 100644
> --- a/gcc/config/rs6000/rs6000-protos.h
> +++ b/gcc/config/rs6000/rs6000-protos.h
> @@ -119,8 +119,8 @@ extern char * output_cbranch (rtx, const char *, int, rtx_insn *);
>  extern const char * output_probe_stack_range (rtx, rtx, rtx);
>  extern void rs6000_emit_dot_insn (rtx dst, rtx src, int dot, rtx ccreg);
>  extern bool rs6000_emit_set_const (rtx, rtx);
> -extern int rs6000_emit_cmove (rtx, rtx, rtx, rtx);
> -extern int rs6000_emit_int_cmove (rtx, rtx, rtx, rtx);
> +extern bool rs6000_emit_cmove (rtx, rtx, rtx, rtx);
> +extern bool rs6000_emit_int_cmove (rtx, rtx, rtx, rtx);
>  extern int rs6000_emit_vector_cond_expr (rtx, rtx, rtx, rtx, rtx, rtx);
>  extern void rs6000_emit_minmax (rtx, enum rtx_code, rtx, rtx);
>  extern void rs6000_expand_atomic_compare_and_swap (rtx op[]);
> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index 1c1caa90ede..bac50c2bcf6 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -15159,7 +15159,7 @@ rs6000_emit_p9_fp_cmove (rtx dest, rtx op, rtx true_cond, rtx false_cond)
>     operands of the last comparison is nonzero/true, FALSE_COND if it
>     is zero/false.  Return 0 if the hardware has no such operation.  */
> 
> -int
> +bool
>  rs6000_emit_cmove (rtx dest, rtx op, rtx true_cond, rtx false_cond)
>  {
>    enum rtx_code code = GET_CODE (op);
> @@ -15175,11 +15175,11 @@ rs6000_emit_cmove (rtx dest, rtx op, rtx true_cond, rtx false_cond)
>        /* In the isel case however, we can use a compare immediate, so
>  	 op1 may be a small constant.  */
>        && (!TARGET_ISEL || !short_cint_operand (op1, VOIDmode)))
> -    return 0;
> +    return false;
>    if (GET_MODE (true_cond) != result_mode)
> -    return 0;
> +    return false;
>    if (GET_MODE (false_cond) != result_mode)
> -    return 0;
> +    return false;
> 
>    /* See if we can use the ISA 3.0 (power9) min/max/compare functions.  */
>    if (TARGET_P9_MINMAX
> @@ -15187,16 +15187,16 @@ rs6000_emit_cmove (rtx dest, rtx op, rtx true_cond, rtx false_cond)
>        && (result_mode == SFmode || result_mode == DFmode))
>      {
>        if (rs6000_emit_p9_fp_minmax (dest, op, true_cond, false_cond))
> -	return 1;
> +	return true;
> 
>        if (rs6000_emit_p9_fp_cmove (dest, op, true_cond, false_cond))
> -	return 1;
> +	return true;
>      }
> 
>    /* Don't allow using floating point comparisons for integer results for
>       now.  */
>    if (FLOAT_MODE_P (compare_mode) && !FLOAT_MODE_P (result_mode))
> -    return 0;
> +    return false;
> 
>    /* First, work out if the hardware can do this at all, or
>       if it's too slow....  */
> @@ -15204,7 +15204,7 @@ rs6000_emit_cmove (rtx dest, rtx op, rtx true_cond, rtx false_cond)
>      {
>        if (TARGET_ISEL)
>  	return rs6000_emit_int_cmove (dest, op, true_cond, false_cond);
> -      return 0;
> +      return false;
>      }
> 
>    is_against_zero = op1 == CONST0_RTX (compare_mode);
> @@ -15216,7 +15216,7 @@ rs6000_emit_cmove (rtx dest, rtx op, rtx true_cond, rtx false_cond)
>       generated.  */
>    if (SCALAR_FLOAT_MODE_P (compare_mode)
>        && flag_trapping_math && ! is_against_zero)
> -    return 0;
> +    return false;
> 
>    /* Eliminate half of the comparisons by switching operands, this
>       makes the remaining code simpler.  */
> @@ -15232,7 +15232,7 @@ rs6000_emit_cmove (rtx dest, rtx op, rtx true_cond, rtx false_cond)
>    /* UNEQ and LTGT take four instructions for a comparison with zero,
>       it'll probably be faster to use a branch here too.  */
>    if (code == UNEQ && HONOR_NANS (compare_mode))
> -    return 0;
> +    return false;
> 
>    /* We're going to try to implement comparisons by performing
>       a subtract, then comparing against zero.  Unfortunately,
> @@ -15247,14 +15247,14 @@ rs6000_emit_cmove (rtx dest, rtx op, rtx true_cond, rtx false_cond)
>        && ((! rtx_equal_p (op0, false_cond) && ! rtx_equal_p (op1, false_cond))
>  	  || (! rtx_equal_p (op0, true_cond)
>  	      && ! rtx_equal_p (op1, true_cond))))
> -    return 0;
> +    return false;
> 
>    /* At this point we know we can use fsel.  */
> 
>    /* Don't allow compare_mode other than SFmode or DFmode, for others there
>       is no fsel instruction.  */
>    if (compare_mode != SFmode && compare_mode != DFmode)
> -    return 0;
> +    return false;
> 
>    /* Reduce the comparison to a comparison against zero.  */
>    if (! is_against_zero)
> @@ -15353,12 +15353,12 @@ rs6000_emit_cmove (rtx dest, rtx op, rtx true_cond, rtx false_cond)
>  						gen_rtx_GE (VOIDmode,
>  							    op0, op1),
>  						true_cond, false_cond)));
> -  return 1;
> +  return true;
>  }
> 
>  /* Same as above, but for ints (isel).  */
> 
> -int
> +bool
>  rs6000_emit_int_cmove (rtx dest, rtx op, rtx true_cond, rtx false_cond)
>  {
>    rtx condition_rtx, cr;
> @@ -15368,7 +15368,7 @@ rs6000_emit_int_cmove (rtx dest, rtx op, rtx true_cond, rtx false_cond)
>    bool signedp;
> 
>    if (mode != SImode && (!TARGET_POWERPC64 || mode != DImode))
> -    return 0;
> +    return false;
> 
>    /* We still have to do the compare, because isel doesn't do a
>       compare, it just looks at the CRx bits set by a previous compare
> @@ -15403,7 +15403,7 @@ rs6000_emit_int_cmove (rtx dest, rtx op, rtx true_cond, rtx false_cond)
> 
>    emit_insn (isel_func (dest, condition_rtx, true_cond, false_cond, cr));
> 
> -  return 1;
> +  return true;
>  }
> 
>  void
> -- 
> 2.22.0
> 
>