Re: [PATCH] Add ISA 3.0 PowerPC support for VEXTU{B,H,W}{L,R}X instructions

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

On Tue, Dec 13, 2016 at 06:17:17PM -0500, Michael Meissner wrote:
> > > +	  else if (mode == V8HImode)
> > > +	    {
> > > +	      rtx tmp_gpr_si = (GET_CODE (tmp_gpr) == SCRATCH
> > > +				? dest_si
> > > +				: gen_rtx_REG (SImode, REGNO (tmp_gpr)));
> > 
> > I think you have these the wrong way around?
> 
> The rs6000_split_vec_extract_var function is called from several places in
> vsx.md to do a variable vector extract.  In looking at each of the cases, there
> is a GPR tmp register for each of the calls, so I could modify it to move:
> 
> 	gcc_assert (REG_P (tmp_gpr));
> 
> before the support for VEXTU{B,H,W}{L,R}X instructions, and leave the
> 
> 	gcc_assert (REG_P (tmp_altivec));
> 
> and remove the test for SCRATCH.  In the original version of the code, the
> non-variable case also called rs6000_split_vec_extract_var, and it did not have
> a scratch register.

What I am asking is: in your code, if there is a scratch you don't use it,
while if you get a reg you generate a new reg.  It looks like you have the
? and : the wrong way around.


> > You didn't address the reload_completed on all the splitters yet; is there
> > a reason for it?

[ ... ]

> Basically, until we know all of the details (i.e. after register allocator), we
> can't do the split, because the code is different.

Right.

> There is also the practical case that due to the funky way the scalar parts are
> not in the bottom part of the register, that SUBREG's really don't work between
> 64-bit and 128-bit items that go in vector registers.  After register
> allocation, we can do gen_rtx_REG (<mode>, <regno>) to change the types, but
> that really doesn't work before register allocator.

Hrm.  Nastiness.

Thanks for the explanation,


Segher