[2/4] PowerPC: Rename functions for min, max, cmove

PowerPC: Rename functions for min, max, cmove.

This patch renames the functions that generate the ISA 3.0 C minimum, C
maximum, and conditional move instructions to use a better name than just using
a _p9 suffix.  Because the functions can fail, the names use "maybe_emit"
instead of "generate_" in the name.

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.c (rs6000_emit_p9_fp_minmax): Rename to
	maybe_emit_fp_c_minmax.
	(maybe_emit_fp_c_minmax): Rename rs6000_emit_p9_fp_minmax.  Return
	bool instead of int.
	(rs6000_emit_p9_fp_cmove): Rename to maybe_emit_fp_cmove.
	(maybe_emit_fp_cmove): Rename rs6000_emit_p9_fp_cmove.  Return
	bool instead of int.
	(have_compare_and_set_mask): New helper function.
	(rs6000_emit_cmove): Rework support of ISA 3.0 functions to
	generate "C" minimum, "C" maximum, and conditional move
	instructions for scalar floating point.

---
 gcc/config/rs6000/rs6000.c | 77 ++++++++++++++++++++++++++------------
 1 file changed, 53 insertions(+), 24 deletions(-)

On Wed, 2020-08-26 at 22:44 -0400, Michael Meissner via Gcc-patches wrote:
> PowerPC: Rename functions for min, max, cmove.
> 
> This patch renames the functions that generate the ISA 3.0 C minimum, C
> maximum, and conditional move instructions to use a better name than just using
> a _p9 suffix.  Because the functions can fail, the names use "maybe_emit"
> instead of "generate_" in the name.
> 
> 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.c (rs6000_emit_p9_fp_minmax): Rename to
> 	maybe_emit_fp_c_minmax.
ok

> 	(maybe_emit_fp_c_minmax): Rename rs6000_emit_p9_fp_minmax.  Return
> 	bool instead of int.

function rename is redundant between the two entries?


> 	(rs6000_emit_p9_fp_cmove): Rename to maybe_emit_fp_cmove.
> 	(maybe_emit_fp_cmove): Rename rs6000_emit_p9_fp_cmove.  Return
> 	bool instead of int.

Function rename comment is redundant ?

> 	(have_compare_and_set_mask): New helper function.
> 	(rs6000_emit_cmove): Rework support of ISA 3.0 functions to
> 	generate "C" minimum, "C" maximum, and conditional move
> 	instructions for scalar floating point.
> 

Nothing else jumped out at me below. 
lgtm,  thanks, 
-Will

> ---
>  gcc/config/rs6000/rs6000.c | 77 ++++++++++++++++++++++++++------------
>  1 file changed, 53 insertions(+), 24 deletions(-)
> 
> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index bac50c2bcf6..6324f930628 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -15056,13 +15056,17 @@ rs6000_emit_vector_cond_expr (rtx dest, rtx op_true, rtx op_false,
>    return 1;
>  }
> 
> -/* ISA 3.0 (power9) minmax subcase to emit a XSMAXCDP or XSMINCDP instruction
> -   for SF/DF scalars.  Move TRUE_COND to DEST if OP of the operands of the last
> -   comparison is nonzero/true, FALSE_COND if it is zero/false.  Return 0 if the
> -   hardware has no such operation.  */
> +/* Possibly emit the C variant of the minimum or maximum instruction for
> +   floating point scalars (xsmincdp, xsmaxcdp, etc.).
> 
> -static int
> -rs6000_emit_p9_fp_minmax (rtx dest, rtx op, rtx true_cond, rtx false_cond)
> +   Move TRUE_COND to DEST if OP of the operands of the last comparison is
> +   nonzero/true, FALSE_COND if it is zero/false.
> +
> +   Return false if we can't generate the appropriate minimum or maximum, and
> +   true if we can did the minimum or maximum.  */
> +
> +static bool
> +maybe_emit_fp_c_minmax (rtx dest, rtx op, rtx true_cond, rtx false_cond)
>  {
>    enum rtx_code code = GET_CODE (op);
>    rtx op0 = XEXP (op, 0);
> @@ -15072,14 +15076,14 @@ rs6000_emit_p9_fp_minmax (rtx dest, rtx op, rtx true_cond, rtx false_cond)
>    bool max_p = false;
> 
>    if (result_mode != compare_mode)
> -    return 0;
> +    return false;
> 
>    if (code == GE || code == GT)
>      max_p = true;
>    else if (code == LE || code == LT)
>      max_p = false;
>    else
> -    return 0;
> +    return false;
> 
>    if (rtx_equal_p (op0, true_cond) && rtx_equal_p (op1, false_cond))
>      ;
> @@ -15092,19 +15096,23 @@ rs6000_emit_p9_fp_minmax (rtx dest, rtx op, rtx true_cond, rtx false_cond)
>      max_p = !max_p;
> 
>    else
> -    return 0;
> +    return false;
> 
>    rs6000_emit_minmax (dest, max_p ? SMAX : SMIN, op0, op1);
> -  return 1;
> +  return true;
>  }
> 
> -/* ISA 3.0 (power9) conditional move subcase to emit XSCMP{EQ,GE,GT,NE}DP and
> -   XXSEL instructions for SF/DF scalars.  Move TRUE_COND to DEST if OP of the
> -   operands of the last comparison is nonzero/true, FALSE_COND if it is
> -   zero/false.  Return 0 if the hardware has no such operation.  */
> +/* Possibly emit a floating point conditional move by generating a compare that
> +   sets a mask instruction and a XXSEL select instruction.
> 
> -static int
> -rs6000_emit_p9_fp_cmove (rtx dest, rtx op, rtx true_cond, rtx false_cond)
> +   Move TRUE_COND to DEST if OP of the operands of the last comparison is
> +   nonzero/true, FALSE_COND if it is zero/false.
> +
> +   Return false if the operation cannot be generated, and true if we could
> +   generate the instruction.  */
> +
> +static bool
> +maybe_emit_fp_cmove (rtx dest, rtx op, rtx true_cond, rtx false_cond)
>  {
>    enum rtx_code code = GET_CODE (op);
>    rtx op0 = XEXP (op, 0);
> @@ -15132,7 +15140,7 @@ rs6000_emit_p9_fp_cmove (rtx dest, rtx op, rtx true_cond, rtx false_cond)
>        break;
> 
>      default:
> -      return 0;
> +      return false;
>      }
> 
>    /* Generate:	[(parallel [(set (dest)
> @@ -15152,7 +15160,28 @@ rs6000_emit_p9_fp_cmove (rtx dest, rtx op, rtx true_cond, rtx false_cond)
>    emit_insn (gen_rtx_PARALLEL (VOIDmode,
>  			       gen_rtvec (2, cmove_rtx, clobber_rtx)));
> 
> -  return 1;
> +  return true;
> +}
> +
> +/* Helper function to return true if the target has instructions to do a
> +   compare and set mask instruction that can be used with XXSEL to implement a
> +   conditional move.  It is also assumed that such a target also supports the
> +   "C" minimum and maximum instructions. */
> +
> +static bool
> +have_compare_and_set_mask (machine_mode mode)
> +{
> +  switch (mode)
> +    {
> +    case SFmode:
> +    case DFmode:
> +      return TARGET_P9_MINMAX;
> +
> +    default:
> +      break;
> +    }
> +
> +  return false;
>  }
> 
>  /* Emit a conditional move: move TRUE_COND to DEST if OP of the
> @@ -15181,15 +15210,15 @@ rs6000_emit_cmove (rtx dest, rtx op, rtx true_cond, rtx false_cond)
>    if (GET_MODE (false_cond) != result_mode)
>      return false;
> 
> -  /* See if we can use the ISA 3.0 (power9) min/max/compare functions.  */
> -  if (TARGET_P9_MINMAX
> -      && (compare_mode == SFmode || compare_mode == DFmode)
> -      && (result_mode == SFmode || result_mode == DFmode))
> +  /* See if we can use the "C" minimum, "C" maximum, and compare and set mask
> +     instructions.  */
> +  if (have_compare_and_set_mask (compare_mode)
> +      && have_compare_and_set_mask (result_mode))
>      {
> -      if (rs6000_emit_p9_fp_minmax (dest, op, true_cond, false_cond))
> +      if (maybe_emit_fp_c_minmax (dest, op, true_cond, false_cond))
>  	return true;
> 
> -      if (rs6000_emit_p9_fp_cmove (dest, op, true_cond, false_cond))
> +      if (maybe_emit_fp_cmove (dest, op, true_cond, false_cond))
>  	return true;
>      }
> 
> -- 
> 2.22.0
> 
>

On Thu, Aug 27, 2020 at 03:47:15PM -0500, will schmidt wrote:
> On Wed, 2020-08-26 at 22:44 -0400, Michael Meissner via Gcc-patches wrote:
> > 	* config/rs6000/rs6000.c (rs6000_emit_p9_fp_minmax): Rename to
> > 	maybe_emit_fp_c_minmax.
> ok
> 
> > 	(maybe_emit_fp_c_minmax): Rename rs6000_emit_p9_fp_minmax.  Return
> > 	bool instead of int.
> 
> function rename is redundant between the two entries?

Yes, there are cleaner ways to write this, while still keeping both the
old and new names on the lhs of the colon (where they should be!)
Without naming each thing twice, even.


Segher

On Wed, Aug 26, 2020 at 10:44:22PM -0400, Michael Meissner wrote:
> PowerPC: Rename functions for min, max, cmove.
> 
> This patch renames the functions that generate the ISA 3.0 C minimum, C
> maximum, and conditional move instructions to use a better name than just using
> a _p9 suffix.  Because the functions can fail, the names use "maybe_emit"
> instead of "generate_" in the name.

Please keep the rs6000_ prefix though.

If we want to drop that, we should do it *everywhere*.

> -/* ISA 3.0 (power9) minmax subcase to emit a XSMAXCDP or XSMINCDP instruction
> -   for SF/DF scalars.  Move TRUE_COND to DEST if OP of the operands of the last
> -   comparison is nonzero/true, FALSE_COND if it is zero/false.  Return 0 if the
> -   hardware has no such operation.  */
> +/* Possibly emit the C variant of the minimum or maximum instruction for
> +   floating point scalars (xsmincdp, xsmaxcdp, etc.).

Please explain that this is asymmetric.

This isn't a proper min/max, except with fastmath :-(

> +  /* See if we can use the "C" minimum, "C" maximum, and compare and set mask
> +     instructions.  */

Just write the actual mnemonics please.

Okay for trunk with those things fixed.  Thanks!


Segher