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 <meiss...@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 > >