[PATCH] rs6000: Fix PR92923, __builtin_vec_xor() causes subregs to be used when not using V4SImode vectors

[Segher Boessenkool]

Hi!

On Tue, Dec 17, 2019 at 07:06:19PM -0600, Peter Bergner wrote:
> PR92923 shows a problem where builtin function usage is causing performance
> problems due to unneeded subreg usage.  These are being caused by the front-
> end emitting unneeded VIEW_CONVERT_EXPR's on the builtin functions operands.
> These in tern where caused by a lack of overloaded builtin entries in the
> rs6000 backend.  The following patch adds just enough new definitions to
> match what our vector documentation says we must support.  I have also
> added new test cases so that we will catch any regressions in this area.

Something automated to verify what we implement is what we have documented
would be neat to have.  Maybe this becomes feasible with the rewrite of
the builtin stuff :-)

> This passed bootstrap and regression testing with no errors.  Ok for trunk?

On what kind of system did you test?

I'd like to see this tested on both BE and LE, and various processor
generations -- but we'll see if it regresses anyway, and it is still
stage 3.  So, okay for trunk, just please keep an eye out for
regressions (in January that is :-) )

> This is a bug on the release branches too, but given how big this patch
> ended up being, I don't think we want to backport this.

Yes, and no user hit problems here anyway.

> 	* config/rs6000/rs6000-builtin.def (VAND, VANDC, VNOR, VOR,
> 	VXOR): Delete.

You can end a changelog line in a colon just fine, fwiw.

> --- gcc/testsuite/gcc.target/powerpc/pr92923-1.c	(nonexistent)
> +++ gcc/testsuite/gcc.target/powerpc/pr92923-1.c	(working copy)
> @@ -0,0 +1,454 @@
> +/* { dg-do compile { target { powerpc*-*-* } } } */

You don't need this target clause, everything in gc.target/powerpc has it
already.

> +/* { dg-skip-if "" { powerpc*-*-darwin* } } */

I'm not sure if this is necessary, or just cargo culting :-)

> +/* { dg-final { scan-tree-dump-times "VIEW_CONVERT_EXPR" 0 "gimple" } } */

That's scan-tree-dump-not.

Same comments for the p8 test of course.  Okay with or without those
adjusted (they aren't wrong, just weird style).

Thanks,


Segher