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

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

On Fri, Jun 09, 2017 at 04:12:25PM -0700, Carl E. Love wrote:
> GCC Maintainers:
> 
> On Fri, 2017-06-09 at 16:05 -0500, Segher Boessenkool wrote:
> 
> Fixed the various formatting (spaces) issues.  Been toying with how to
> write a space checker for patches.  Have to take some time to really
> think about how to do that....
> 
> > > +
> > > +  /* The vector merge instruction vmrgew swaps the 2nd and 3rd words,
> > > +     compensate by swapping the 64-bit elements around to negate the vmrgew
> > > +     swap. */
> > 
> > This comment isn't very clear to me...  Could you expand it a bit?
> 
> Reworked it, hopefully it explains things better
> 
> > > +;; 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 ;-)
> > 

Probably not, particularly since we've been adding new Altivec encoded
instructions.  Back in the ISA 2.06 (power7) days, it was much clearer, that
altivec.md was the musty old instructions and vsx.md were the new ones.

> 
> 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.

And doing large changes to 'simplify' things can lead to other problems.

> 
> > > +(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.  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.

Well in the power7 days, it wasn't clear whether we wanted to reduce the
register set, so I added the general "wa", and then added the more specific
changes ("ws", "wf", "wd").  In hindsight it probably wasn't a good idea.  But
the trouble is we can't delete the old constraints, or we would break user asm
code.

Over time, I have been deleting things where you have the specific constraint
and the general one where I'm modifying code:

	(match_operand:V2DF 0 "=wd,?wa")

to

	(match_operand:V2DF 0 "=wa")

Now the second round of constraints are needed because of the
-mupper-regs-<xxx> debug switches.  You might/might not allow DFmode into the
Altivec registers, and so you need several constraints:

	d	Just the traditional FPRs
	ws	Any FPR/Altivec register DFmode can go in for ISA 2.06 insns
	wk	Like ws, but only if 64-bit direct moves are supported
	wv	Only altivec registers (used for 64-bit load/stores)

Note, you have to be careful not to allow a register constraint that the
current type cannot go into.  This is due to a 'feature' in the LRA register
allocator that it will trap if such a case occurs.  For example, for ISA 2.06,
we do not have 32-bit floating point instructions in the Altivec registers.
This means you can't use "v" (just the Altivec registers) on any code where
-mcpu=power7 (or -mno-upper-regs-sf) is allowed.

-- 
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