On Fri, Jun 16, 2017 at 02:52:46PM -0500, Segher Boessenkool wrote:
> Hi Mike,
>
> On Thu, Jun 15, 2017 at 10:10:28PM -0400, Michael Meissner wrote:
> > +(define_insn_and_split "vsx_set_v4sf_p9"
> > + [(set (match_operand:V4SF 0 "gpc_reg_operand" "=wa")
> > + (unspec:V4SF
> > + [(match_operand:V4SF 1 "gpc_reg_operand" "0")
> > + (match_operand:SF 2 "gpc_reg_operand" "ww")
> > + (match_operand:QI 3 "const_0_to_3_operand" "n")]
> > + UNSPEC_VSX_SET))
> > + (clobber (match_scratch:SI 4 "=&wJwK"))]
> > + "VECTOR_MEM_VSX_P (V4SFmode) && TARGET_P9_VECTOR"
> > + "#"
> > + "&& reload_completed"
>
> I still don't think it is such a good idea to do all of this not until
> after reload. It does of course allow you to play tricks with changing
> register mode at will, like you do ;-)
The problem is MODES_TIEABLE_P. V4S{I,F}mode and SImode cannot be tied
together (i.e. use gen_lowpart to change the mode and use a SUBREG). So after
reload, we can just use gen_rtx_REG (...) to change the register type, but
before reload, by creating the SUBREG, it can lead to various aborts if rtl
checking is turned on.
> All these unspecs are a similar problem: the RTL optimisers cannot do
> much at all with it.
I don't think there is a good way to represent a vec_insert. And vec_extract
can't represent a variable extract either.
> > + [(set_attr "type" "vecperm")
I generally use the type of the last insn. I am open to other suggestions.
> Is that a good type for this? I think the convert is more expensive
> than the permutes? If so, that would be better (of course it only
> matters for sched1, not super important).
>
> > --- gcc/testsuite/gcc.target/powerpc/pr79799-1.c (nonexistent)
> > +++ gcc/testsuite/gcc.target/powerpc/pr79799-1.c (working copy)
> > @@ -0,0 +1,43 @@
> > +/* { dg-do compile { target { powerpc64*-*-* && lp64 } } } */
>
> Why not powerpc*-*-*?
Well as it turns out, it aborts in 32-bit, because -mvsx-small-integer is not
enabled, and we can't have SImode in vector registers. I'll have to add some
additional tests and resubmit the patch.
>
> > +/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } {
> > "-mcpu=power9" } } */
> > +/* { dg-require-effective-target powerpc_p9vector_ok } */
> > +/* { dg-options "-mcpu=power9 -O2" } */
> > +
> > +#include <altivec.h>
> > +
> > +/* GCC 7.1 did not have a specialized method for inserting 32-bit floating
> > point on
> > + ISA 3.0 (power9) systems. */
>
> That first line is a bit long.
Ok.
> The patch is okay for trunk and 7 with the testsuite nits taken care of.
--
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: [email protected], phone: +1 (978) 899-4797
