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
