diff mbox series

[1/2] PowerPC: Rename min/max/cmove functions.

Message ID 20200811162206.GA6583@ibm-toto.the-meissners.org
State New
Headers show
Series [1/2] PowerPC: Rename min/max/cmove functions. | expand

Commit Message

Michael Meissner Aug. 11, 2020, 4:22 p.m. UTC 
PowerPC: Rename min/max/cmove functions.

This patch renames some of the functions in rs6000.c that are used to generate
floating point scalar minimum, maximum, and conditional move sequences to use a
more generic name then _p9.

gcc/
2020-08-07  Michael Meissner  <meissner@linux.ibm.com>

	* config/rs6000/rs6000.c
	(rs6000_emit_p9_fp_minmax,generate_fp_min_max): Rename.
	(rs6000_emit_p9_fp_cmove,generate_fp_cmove): Rename.
	(rs6000_emit_cmove): Update to use renamed functions.
---
 gcc/config/rs6000/rs6000.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

Comments

Segher Boessenkool Aug. 11, 2020, 6:17 p.m. UTC | #1 
Hi!

On Tue, Aug 11, 2020 at 12:22:06PM -0400, Michael Meissner wrote:
> 	(rs6000_emit_p9_fp_minmax,generate_fp_min_max): Rename.

Space after comma.  "Rename." is never useful (and you should show what
is renamed to what).  The patch does *not* do the same or similar to the
two names inside the parens here, so you cannot do this in one entry.

> -/* 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 an appropriate minimum or maximum instruction for floating
> +   point 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 we can't generate the appropriate minimum or maximum, and 1 if
> +   we can did the minimum or maximum.  */

It should say these are "C" variants, not the proper min/max ones.
"Appropriate" is very misleading here.

> -/* 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.
> +
> +   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 operation cannot be generated, and 1 if we could generate
> +   the instruction.  */

... and 1 if we *did* generate it.

Change this to a bool, and rename to "maybe_emit" etc.?

"generate_" is a horrible name...  We already have "gen_" things, with
different semantics, and this emits the insn, doesn't just generate it.

Thanks,


Segher
diff mbox series

Patch

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index d26a18f..1c042b0 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -15032,13 +15032,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 an appropriate minimum or maximum instruction for floating
+   point 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 we can't generate the appropriate minimum or maximum, and 1 if
+   we can did the minimum or maximum.  */
 
 static int
-rs6000_emit_p9_fp_minmax (rtx dest, rtx op, rtx true_cond, rtx false_cond)
+generate_fp_min_max (rtx dest, rtx op, rtx true_cond, rtx false_cond)
 {
   enum rtx_code code = GET_CODE (op);
   rtx op0 = XEXP (op, 0);
@@ -15074,13 +15078,17 @@  rs6000_emit_p9_fp_minmax (rtx dest, rtx op, rtx true_cond, rtx false_cond)
   return 1;
 }
 
-/* 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.
+
+   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 operation cannot be generated, and 1 if we could generate
+   the instruction.  */
 
 static int
-rs6000_emit_p9_fp_cmove (rtx dest, rtx op, rtx true_cond, rtx false_cond)
+generate_fp_cmove (rtx dest, rtx op, rtx true_cond, rtx false_cond)
 {
   enum rtx_code code = GET_CODE (op);
   rtx op0 = XEXP (op, 0);
@@ -15162,10 +15170,10 @@  rs6000_emit_cmove (rtx dest, rtx op, rtx true_cond, rtx false_cond)
       && (compare_mode == SFmode || compare_mode == DFmode)
       && (result_mode == SFmode || result_mode == DFmode))
     {
-      if (rs6000_emit_p9_fp_minmax (dest, op, true_cond, false_cond))
+      if (generate_fp_min_max (dest, op, true_cond, false_cond))
 	return 1;
 
-      if (rs6000_emit_p9_fp_cmove (dest, op, true_cond, false_cond))
+      if (generate_fp_cmove (dest, op, true_cond, false_cond))
 	return 1;
     }