Re: [PATCH #2], PR 71201, Fix xxperm fusion on PowerPC ISA 3.0

[Michael Meissner <meissner at linux dot vnet dot ibm dot com>]

On Thu, May 19, 2016 at 10:33:41AM -0500, Segher Boessenkool wrote:
> On Thu, May 19, 2016 at 10:53:41AM -0400, Michael Meissner wrote:
> > GCC 6.1 added support for the XXPERM instruction for the PowerPC ISA 3.0.  The
> > XXPERM instruction is essentially a 4 operand instruction, with only 3 operands
> > in the instruction (the target register overlaps with the first input
> > register).  The Power9 hardware has fusion support where if the instruction
> > that precedes the XXPERM is a XXLOR move instruction to set the first input
> > argument, it is fused with the XXPERM.  I added code to support this fusion.
> > 
> > Unfortunately, in running the testsuite on the power9 simulator, we discovered
> > that the test gcc.c-torture/execute/pr56866.c would fail because the fusion
> > alternatives confused the register allocator and/or the passes after the
> > register allocator.  This patch removes the explicit fusion support from
> > XXPERM.
> 
> Okay.  Please keep the PR open until that problem is fixed.  It also
> shouldn't be "target" category, if the problem is RA.
> 
> > In addition, ISA 3.0 added XXPERMR and VPERMR instructions for little endian
> > support where the permute vector reverses the bytes.  This patch adds support
> > for XXPERMR/VPERMR.
> 
> Please send that as a separate patch, it has nothing to do with the PR.
> 
> > +	x = gen_rtx_UNSPEC (mode,
> > +			    gen_rtvec (3, target, reg, 
> 
> Trailing space.
> 
> > +  if (TARGET_P9_VECTOR)
> > +    {
> > +      unspec = gen_rtx_UNSPEC (mode, gen_rtvec (3, op0, op1, sel), 
> 
> And another.
> 
> > +	 The VNAND is preferred for future fusion opportunities.  */
> > +      notx = gen_rtx_NOT (V16QImode, sel);
> > +      iorx = (TARGET_P8_VECTOR
> > +	      ? gen_rtx_IOR (V16QImode, notx, notx)
> > +	      : gen_rtx_AND (V16QImode, notx, notx));
> > +      emit_insn (gen_rtx_SET (norreg, iorx));
> > +      
> 
> Some more.
> 
> > +/* { dg-final { scan-assembler	   "vpermr\|xxpermr" } } */
> 
> Tab in the middle of the line.

This patch just fixes the xxperm fusion problem, and I will submit the
vpermr/xxpermr support in another patch.

Note, if you believe the register allocator and the post reload RTL passes need
to be fixed to allow the fusion of the move to the xxperm, that is fine.
However, take it on yourself.  As the person who wrote the code to add fusion
support for xxperm, I now think it was a bad idea, and I want to remove that
support.  It would probably be better done by modifying the scheduler to keep
the move and xxperm together, rather than including it in the insn.

I have bootstrapped the compiler on a little endian power9 system and there
were no regressions in the test suite.  Is it ok to check into the trunk and on
the 6.1 branch?

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

	PR target/71201
	* config/rs6000/altivec.md (altivec_vperm_<mode>_internal): Drop
	ISA 3.0 xxperm fusion alternative.
	(altivec_vperm_v8hiv16qi): Likewise.
	(altivec_vperm_<mode>_uns_internal): Likewise.
	(vperm_v8hiv4si): Likewise.
	(vperm_v16qiv8hi): Likewise.

[gcc/testsuite]
2016-05-23  Michael Meissner  <meissner@linux.vnet.ibm.com>
	    Kelvin Nilsen  <kelvin@gcc.gnu.org>

	* gcc.target/powerpc/p9-permute.c: Run test on big endian as well
	as little endian.

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.vnet.ibm.com, phone: +1 (978) 899-4797

Attachment: gcc-stage7.xxperm002b
Description: Text document