, Add PowerPC ISA 3.0 min/max support

On Mon, May 09, 2016 at 09:31:43AM -0500, Segher Boessenkool wrote:
> On Thu, May 05, 2016 at 03:18:39PM -0400, Michael Meissner wrote:
> > At the present time, the code does not support comparisons involving >= and <=
> > unless the -ffast-math option is used. I hope eventually to support generating
> > these instructions without having -ffast-math used.
> > 
> > The underlying reason is when fast math is not used, we change the condition
> > from:
> > 
> > 	(ge:SI (reg:CCFP <reg>) (const_int 0))
> > 
> > to:
> > 
> > 	(ior:SI (gt:SI (reg:CCFP <reg>) (const_int 0))
> > 		(eq:SI (reg:CCFP <reg>) (const_int 0)))
> > 
> > The machine independent portion of the compiler does not recognize this when
> > trying to generate conditional moves.
> > 
> > I would imagine the 'fix' is to generate GE/LE all of the time, and then have a
> > splitter that converts it to IOR of GT/EQ if it is not a conditional move with
> > ISA 3.0 instructions.
> 
> That sounds like a plan :-)

Well in the list of my priorities, it is low on the list.  Hopefully I or
somebody else will be able to get to it by the time GCC 7 freezes.

> 
> > -;; Return true if operand is MIN or MAX operator.
> > +;; Return true if operand is MIN or MAX operator.  Since this is only used to
> > +;; convert floating point MIN/MAX operations into FSEL on pre-vsx systems,
> > +;; don't include UMIN or UMAX.
> >  (define_predicate "min_max_operator"
> > -  (match_code "smin,smax,umin,umax"))
> > +  (match_code "smin,smax"))
> 
> Please name it signed_min_max_operator instead?

In this set of patches, I rewrote the define_split that called it to use
SMIN/SMAX code iterators, so I deleted the min_max_operator predicate.

> > --- gcc/config/rs6000/rs6000.c	(.../svn+ssh://meissner@gcc.gnu.org/svn/gcc/trunk/gcc/config/rs6000)	(revision 235831)
> > +++ gcc/config/rs6000/rs6000.c	(.../gcc/config/rs6000)	(working copy)
> > @@ -20534,6 +20534,12 @@ print_operand (FILE *file, rtx x, int co
> >  				"local dynamic TLS references");
> >        return;
> >  
> > +    case '@':
> > +      /* If -mpower9-minmax, use xsmaxcpdp instead of xsmaxdp.  */
> > +      if (TARGET_P9_MINMAX)
> > +	putc ('c', file);
> > +      return;
> 
> I don't think @ is very mnemonic, nor is this special enough for such
> a nice letter.

I just remove the %@ and instead did a C++ test for the appropriate string to
return.

> Form looking at how it is used, it seems you can make it part of code_attr
> minmax (and give that a better name, minmax_fp or such)?

No, you can't use code attributes, because it is based on the target switches,
not on the insn (i.e. VSX with -ffast-math uses the same insn as p9 min/max
without -ffast-math).

> > +  rs6000_emit_minmax (dest, (max_p) ? SMAX : SMIN, op0, op1);
> 
> Superfluous parentheses.

Ok.

> > +rs6000_emit_power9_cmove (rtx dest, rtx op, rtx true_cond, rtx false_cond)
> 
> Maybe put some "fp" in the name?  For "minmax" as well.
> 
> > +  if (swap_p)
> > +    compare_rtx = gen_rtx_fmt_ee (code, CCFPmode, op1, op0);
> > +  else
> > +    compare_rtx = gen_rtx_fmt_ee (code, CCFPmode, op0, op1);
> 
> if (swap_p)
>   std::swap (op0, op1);
> 
> and then just generate the one form?

I renamed the functions, and used std::swap earlier.

These patches have been bootstrap on a big endian power7 system (both 32-bit
and 64-bit available) and little endian power8 system with no regressions.  Are
these patches ok to install in the trunk?  After a burn-in period, are they ok
to install on the GCC 6.2 branch?

[gcc]
2016-05-26  Michael Meissner  <meissner@linux.vnet.ibm.com>

	* config/rs6000/rs6000.c (rs6000_emit_p9_fp_minmax): New function
	for ISA 3.0 min/max support.
	(rs6000_emit_p9_fp_cmove): New function for ISA 3.0 floating point
	conditional move support.
	(rs6000_emit_cmove): Call rs6000_emit_p9_fp_minmax and
	rs6000_emit_p9_fp_cmove if the ISA 3.0 instructions are
	available.
	* config/rs6000/rs6000.md (SFDF2): New iterator to allow doing
	conditional moves there the comparison type is different from move
	type.
	(fp_minmax): New code iterator for smin/smax.
	(minmax): New code attributes for min/max.
	(SMINMAX): Likewise.
	(smax<mode>3): Combine min, max insns into one insn using the
	fp_minmax code iterator.  Add support for ISA 3.0 min/max
	instructions that don't need -ffast-math.
	(s<minmax><mode>3): Likewise.
	(smax<mode>3_vsx): Likewise.
	(smin<mode>3): Likewise.
	(s<minmax><mode>3_vsx): Likewise.
	(smin<mode>3_vsx): Likewise.
	(pre-VSX min/max splitters): Likewise.
	(s<minmax><mode>3_fpr): Likewise.
	(movsfcc): Rewrite floating point conditional moves to combine
	SFmode/DFmode into a single insn.
	(mov<mode>cc): Likewise.
	(movdfcc): Likewise.
	(fselsfsf4): Combine FSEL cases into a single insn, using SFDF and
	SFDF2 iterators to handle all combinations.
	(fseldfsf4): Likewise.
	(fsel<SFDF:mode><SFDF2:mode>4): Likewise.
	(fseldfdf4): Likewise.
	(fselsfdf4): Likewise.
	(mov<SFDF:mode><SFDF2:mode>cc_p9): Add support for the ISA 3.0
	comparison instructions that set a 0/-1 mask, and use it for
	floating point conditional move via XXSEL.
	(fpmask<mode>): Likewise.
	(xxsel<mode>): Likewise.
	* config/rs6000/predicates.md (min_max_operator): Delete, no
	longer used.
	(fpmask_comparison_operaton): New insn for ISA 3.0 comparison
	instructions that generate a 0/-1 mask for use with XXSEL.
	* config/rs6000/rs6000.h (TARGET_MINMAX_SF): New helper macros to
	say whether floating point min/max is available, either through
	FSEL, ISA 2.06 min/max, and ISA 3.0 min/max instrucitons.
	(TARGET_MINMAX_DF): Likewise.

[gcc/testsuite]
2016-05-26  Michael Meissner  <meissner@linux.vnet.ibm.com>

	<fsf submission>
	* gcc.target/powerpc/p9-minmax-1.c: New tests for ISA 3.0
	floating point min/max/comparison instructions.
	* gcc.target/powerpc/p9-minmax-2.c: Likewise.

, Add PowerPC ISA 3.0 min/max support

Commit Message

Comments

Patch