Re: [PATCH v2, rs6000] gcc mainline, add builtin support for vec_float, vec_float2, vec_floate, vec_floate, builtins

[Segher Boessenkool <segher at kernel dot crashing dot org>]

Hi again,

On Fri, Jun 09, 2017 at 04:12:25PM -0700, Carl E. Love wrote:
> On Fri, 2017-06-09 at 16:05 -0500, Segher Boessenkool wrote:
> > > +;; Mode iterator and attribute for vector floate and floato conversions
> > > +(define_mode_iterator VFC [V2DI V2DF])
> > > +(define_mode_attr VFC_inst [(V2DI "sxd") (V2DF "dp")])
> > 
> > .._sxddp, like VS_sxswp in altivec.md?  The iterator is just VSX_D.
> > 
> > Maybe some or all of these iterators/attrs should live in vector.md?
> > 
> > Is it really useful to have separate files altivec.md and vsx.md anymore?
> > Or should some things be moved?  This is a general question, not really
> > something to be handled in this patch ;-)
> > 
> 
> Yea, I find searching through all the files to rather hard.  Perhaps
> putting all the definitions into a single "header" file?  That way it
> could span across all of the .md files.  If you combined vsx.md and
> altivec.md, you would have a really large file.  Big files can be
> problematic in their own right.

All md files already *are* included into rs6000.md; see the very end of
rs6000.md for these.  vector.md is included before vsx.md and altivec.md,
so you can define all iterators etc. there.

Big files are problematic; arbitrary splits are worse.  Originally it
made sense to have vsx.md and altivec.md separate (and separate from
the integer stuff in rs6000.md), but now it is less clear.  Maybe we
should split differently?  vector, vector-int, vector-float, something
like that?

> > > +(define_insn "vsx_xvcvsxwsp"
> > > +  [(set (match_operand:V4SF 0 "vsx_register_operand" "=v")
> > > +	(unspec:V4SF[(match_operand:V4SI 1 "vsx_register_operand" "v")]
> > > +		    UNSPEC_VSX_CVSXWSP))]
> > > +  "VECTOR_UNIT_VSX_P (V4SFmode)"
> > > +  "xvcvsxwsp %x0,%x1"
> > > +  [(set_attr "type" "vecdouble")])
> > 
> > "v" is only the VRs...  Do you want "wa" or similar instead?
> 
> I went back and re-studied the Power register constrains.  I find them a
> bit confusing, I am sure they are perfectly clear to everyone else.

Heh, good joke :-)

> So
> the instructions all take VSX registers so "wa" should be fine if I
> understand it correctly.  Not sure there is any need to further
> constrain with "vs" for doubles or "ww" but I think you could.

"ws" is all VSRs if TARGET_UPPER_REGS_DF but just FP regs otherwise.
"ww" is all VSRs if TARGET_P8_VECTOR and TARGET_UPPER_REGS_SF, just
the FP regs if only TARGET_VSX, and nothing otherwise.

I don't know what to use when.  Mike does.

> Please let me know if the updated patch is OK for gcc mainline?

Let's hear what Mike thinks about the constraints.  I *think* that
the VECTOR_UNIT_VSX_P (V4SFmode) makes "wa" just work.

> +;; Generate float2
> +;; convert two long long signed ints to float
> +(define_expand "float2_v2di"
> +  [(match_operand:V4SF 0 "register_operand" "=wa")
> +   (unspec:V4SI [(match_operand:V2DI 1 "register_operand" "wa")
> +		 (match_operand:V2DI 2 "register_operand" "wa")]
> +  UNSPEC_VSX_FLOAT2)]
> +
> +  "TARGET_VSX"

... but TARGET_VSX is probably not good enough for "wa".

Will this work at all?  The insns the expander expands to have a different
condition than the insns themselves...  VECTOR_UNIT_VSX_P (V4SFmode) is
actually the same thing as TARGET_VSX, but will it stay that way, and
more importantly, that isn't clear at all.


Segher