Re: [Qemu-devel] [PATCH 6/7] tcg/mips: Support full movcond select operation

[James Hogan]

Hi Richard,

On Thu, Oct 01, 2015 at 06:56:39AM +1000, Richard Henderson wrote:
> On 10/01/2015 01:30 AM, James Hogan wrote:
> > Adapt the MIPS movcond implementation to handle the full select
> > operation using a pair of MOVN/MOVZ instructions.
> >
> > This allows the register alias constraint to be removed (which is what
> > ensured v2 == dest), and allows it to be more easily extended to support
> > the MIPS r6 instructions SELNEZ/SELEQZ which replace MOVN/MOVZ and
> > require similar logic.
> >
> > For example, previously we only supported:
> >   movcond_i32 dest, c1, c2, v1, v2=dest, cond
> >
> > With the host code:
> >   MOV[ZN] dest, v1, [!](c1 cond c2)
> >
> > Meaning:
> >   if (c1 cond c2)
> >       dest = v1;
> >
> > But now v2 doesn't have to equal dest, so we can support:
> >   movcond_i32 dest, c1, c2, v1, v2, cond
> >
> > With the host code:
> >   #if dest != v1
> >       MOV[ZN] dest, v1, [!](c1 cond c2)
> >   #endif
> >   #if dest != v2
> >       MOV[NZ] dest, v1, ![!](c1 cond c2)
> >   #endif
> >
> > Meaning:
> >   #if dest != v1
> >       if ([!](c1 cond c2))
> >           dest = v1;
> >   #endif
> >   #if dest != v2
> >       if (![!](c1 cond c2))
> >           dest = v2;
> >   #endif
> 
> I don't think this is a good change.  In the case of dest != v1 && dest != 
> v2, 
> we wind up with two conditional instructions.  On most targets that I'm 
> familiar with, this is more expensive than a plain move.
> 
> If r6 was conditional, as opposed to something that we must perforce know 
> about 
> at compilation time, then I'd say go ahead but split this into a normal move 
> followed by a conditional move.  But since that's not the case, I don't see 
> the 
> point in changing anything at all.

Yeh, I think you're right. This is certainly the change I was struggling
to convince myself about it being worthwhile, since it was unclear to me
just what net effect the operand aliasing constraint would have on
register allocation and the generated code.

I'll resubmit my original version without this patch. R2/R6 will just
have different constraints via an ifdef so that the R2 code doesn't have
to deal with that case (since dest will always be v2).

Thanks for the review!

Cheers
James

signature.asc
Description: Digital signature