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

[Segher Boessenkool]

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